libwww-perl / HTTP-Message

The HTTP-Message distribution contains classes useful for representing the messages passed in HTTP style communication.
https://metacpan.org/pod/HTTP::Message
Other
28 stars 61 forks source link

Spec change or configuration Problem? #154

Closed robertII closed 3 years ago

robertII commented 3 years ago

I'm moving a venerable Perl system to new infrastructure -- from Perl 5.12 on CentOS6 to 5.16 on CentOS 7.

In particular, I'm trying to get its tests working, and I've almost immediately hit a rather surprising problem.

I believe the relevant bits of the test are...

use Test::More;
use Catalyst::Test 'APPNAME';
use URI;
   ;
   ;
my $res = request('/clubs');
ok(my $uri = $res->header('Location'), "Location field");
is($uri->path, '/clubs/list', "Redirected to list");

The test used to pass, but is now failing with

Can't locate object method "path" via package "http://localhost/clubs/list" 

That is because request is behaving differently. It is still returning an instance of HTTP::Response (as Catalyst::Test says it should), and that HTTP::Response has an HTTP::Headers which includes a plausible-looking location.

But on the old infrastructure, that location was an object of type URI::http, and it is now a plain string.

This is surprising. Has the behaviour of HTTP::Response and/or HTTP::Headers (or, just possibly Catalyst::Test) actually changed like this? To hold/return a string instead of a URI::http? In which case I'm going to have to change many tests.

Or is something somewhere misconfigured?

Version are Module Old Now
HTTP::Response 6.01 6.04
HTTP::Headers 6.00 6.05
URI::http - 1.76
Catalyst 5.90019 5.90126
oalders commented 3 years ago

What might be helpful here is to run the same test with all of the versions of HTTP-Message(6.00-6.05) installed, so you can see if the behaviour changes at any given version. From a quick scan of the code I don't see a change in behaviour, but this would be an easy way to see if we can take this module out of the equation.

robertII commented 3 years ago

Thanks for the very prompt response, and the good suggestion -- didn't think of that.

It was much less painful than I thought it would be. But the results are, well, annoying...

My aged C6 machines, where the test is passing, have HTTP::Message v 6.02. So on my C7 machine, which has HTTP::Message 6.06 installed "host wide", I did

cpanm -l /opt/ttscore-elttl HTTP::Message@6.02

so that now that user's "private" copy is hiding 6.06, and so that user is using version 6.02

perl -e 'use HTTP::Message; print $HTTP::Message::VERSION . "\n";'
6.02

And at this point, my test still fails in the same way -- the HTTP::Response returned by the request(...) has a HTTP::Headers where the location is a string, not a URI::http as it is/was on the C6 servers.

So the change in behaviour that is breaking the tests is not down to HTTP::Message itself changing from 6.02 to 6.05, but either in one of its dependencies, or to a change in how Catalyst::Test is using it that occurred between Catalyst 5.90019 and 5.90126...

There are no obviously relevant changes mentioned since 2008 (!)...

I'll try asking the maintainer if he can shed any light, but I'm thinking that I'll have to just go with the flow and modify my tests to expect a string not a URI::http

Are we having fun yet?

oalders commented 3 years ago

You could try doing the same with Catalyst and seeing at which version things break.

robertII commented 3 years ago

I did try that. But first, there are "many" version (0019 -> 0126) to check, so even binary chopping is going to take eight tries. And also, when I tried to install the earlier version of Catalyst, it failed because of unsatisfied dependencies. I didn't go into too much detail, but it certainly didn't look like it was going to be a quick/easy task...

oalders commented 3 years ago

Looking at your initial code snippet, it's not complete. What is providing request()?

oalders commented 3 years ago

Oh, I see it's Catalyst::Test. The docs for the latest version of that module have:

my $res = request('/'); # redirects to /y
warn $res->header('location');
use URI;
my $uri = URI->new($res->header('location'));
is ( $uri->path , '/y');
my $content = get($uri->path);

So, there a new URI object is being constructed from the location header, which looks to be in line with the behaviour you're seing.

robertII commented 3 years ago

Whereas the URI used to be created inside the request(....).

So I'm just converting the tests that were is($uri->path, 'blah/blah;', ... ) to like($uri, qr%blah/blah%, ...)

Now, I just have to work out a clean way to change the logging behaviour of catalyst environment hidden inside the request so that the $c->log->debug("blah blah") statements stop messing up the `is($stderr, "", "No error messages") tests!!

oalders commented 3 years ago

😄 It sounds like you're on your way and I think we've established that this isn't an issue with URI itself. Good luck porting your app! I'll close this issue for now, but feel free to open a new issue if you do find a problem with this code.