http-config.t failures (caused by URI 1.75)

[eserte]

I observe the following kind of failures on some of my smokers:

#   Failed test at t/http-config.t line 59.
#          got: 'u:p|slash|.com|GET|not secure|always'
#     expected: '.com|GET|secure|always'

#   Failed test at t/http-config.t line 69.
#          got: 'u:p|slash|.com|success|GET|not secure|always'
#     expected: '.com|success|GET|secure|always'

#   Failed test at t/http-config.t line 73.
#          got: 'u:p|slash|success|GET|not secure|always'
#     expected: 'success|GET|always'
# Looks like you failed 3 tests of 28.
t/http-config.t ............. 
Dubious, test returned 3 (wstat 768, 0x300)
Failed 3/28 subtests 

This seems to happen also on other systems, for example: http://www.cpantesters.org/cpan/report/34a5f0ee-13ca-11e9-8c82-aba07c4588f0

[eserte]

Well, it's not random, it's caused by the newest URI.pm. Statistical analysis:

****************************************************************
Regression 'mod:URI'
****************************************************************
Name           	       Theta	      StdErr	 T-stat
[0='const']    	      1.0000	      0.0436	  22.96
[1='eq_1.64']  	      0.0000	      0.0533	   0.00
[2='eq_1.65']  	      0.0000	      0.0754	   0.00
[3='eq_1.67']  	      0.0000	      0.0754	   0.00
[4='eq_1.71']  	      0.0000	      0.0458	   0.00
[5='eq_1.72']  	      0.0000	      0.0459	   0.00
[6='eq_1.73']  	      0.0000	      0.0458	   0.00
[7='eq_1.74']  	     -0.0060	      0.0438	  -0.14
[8='eq_1.75']  	     -1.0000	      0.0446	 -22.41

R^2= 0.971, N= 271, K= 9
****************************************************************

[oalders]

Thanks. I can replicate this locally.

[oalders]

I just reverted the changes to URI and uploaded to CPAN. That should fix the immediate problem.

[doriantaylor]

Okay I think I see what's going on:

HTTP::Request::uri_canonical is cached, and only ever overwritten when a subsequent call to ->uri($new_uri) is made:

sub uri_canonical
{
    my $self = shift;
    return $self->{'_uri_canonical'} ||= $self->{'_uri'}->canonical;
}

The test rewrites the URI scheme which will not be reflected in the memoized _uri_canonical member, since URI issue 57 guarantees it is no longer the same object:

is(j($conf->matching_items($request)), "u:p|slash|.com|GET|not secure|always");

$request->method("HEAD");
$request->uri->scheme("https");

is(j($conf->matching_items($request)), ".com|GET|secure|always");

I have created a patch to HTTP::Request which passes the test, if you're interested.

[Grinnz]

I'd also consider this a bug fix, because in the case where the URI is not already canonical, the cached version would become out of sync when the original one is changed. (could this be tested for?)

[doriantaylor]

A latent bug perhaps. The ostensible reason why it never surfaced was likely because the URI would have been minted as canonical (or at least canonical-ish) in the test, which &URI::canonical prior to 1.75 would have just returned $self as-is. As such, any subsequent changes to the URI would have implicitly operated on the same object.

Indeed, however, prior to patch #123, the raw URI and the canonical URI could fall out of sync if for some reason the URI in the HTTP::Request object was not canonical (e.g. /%7Euser instead of /~user or something), and then was subsequently changed (e.g. $req->uri->query('lol=wut')).