Bug #51562 for CGI: url() should use virtual_port(), not server

Mon Nov 16 00:58:08 2009 mkanat [...] cpan.org - Ticket created

Subject:

url() should use virtual_port(), not server_port()

Right now, url() does this: my $port = $self->server_port; $url .= ":" . $port unless (lc($protocol) eq 'http' && $port == 80) || (lc($protocol) eq 'https' && $port == 443); However, in an environment where the CGI application is behind a proxy that is doing some sort of port translation (for example, the frontend handles SSL, so traffic comes in on 443, and the backend is in HTTP, so traffic is on some other port, like 81) this isn't the right behavior--the url comes out with all the information of the front-end proxy but the back-end port number.

Mon Nov 16 09:51:58 2009 mark [...] summersault.com - Correspondence added

Subject:	Re: [rt.cpan.org #51562] url() should use virtual_port(), not server_port()
Date:	Mon, 16 Nov 2009 09:51:41 -0500
To:	bug-CGI.pm [...] rt.cpan.org
From:	Mark Stosberg <mark [...] summersault.com>

Show quoted text

> > Right now, url() does this: > > my $port = $self->server_port; > $url .= ":" . $port > unless (lc($protocol) eq 'http' && $port == 80) > || (lc($protocol) eq 'https' && $port == 443); > > However, in an environment where the CGI application is behind a proxy > that is doing some sort of port translation (for example, the frontend > handles SSL, so traffic comes in on 443, and the backend is in HTTP, so > traffic is on some other port, like 81) this isn't the right > behavior--the url comes out with all the information of the front-end > proxy but the back-end port number.

Thanks for the feedback. What do you think the fix is? Do you think we should use virtual_port() if it is available, and server_port if it is not()? Are other other projects you can point as examples of a "correct" behavior? For example, it would be interesting to review what CGI::Simple does here. Mark

Mon Nov 16 09:51:59 2009 The RT System itself - Status changed from 'new' to 'open'

Mon Nov 16 15:24:37 2009 mkanat [...] cpan.org - Correspondence added

On Mon Nov 16 09:51:58 2009, mark@summersault.com wrote: Show quoted text

> What do you think the fix is? Do you think we should use virtual_port() > if it is available, and server_port if it is not()?

Well, virtual_port already returns server_port if it doesn't have a virtual_port. And url() already uses the virtual host if there's a virtual host. So it seems like this is the logical solution. User-Agents send the port as part of the Host header, so if there is a Host header (which url() already uses for the host name), there will be a valid port, and it will be the port that the UA made the original request on. Show quoted text

> Are other other projects you can point as examples of a "correct" > behavior? For example, it would be interesting to review what > CGI::Simple does here.

CGI::Simple currently appears to have identical code to CGI.pm in this area.

Sat Nov 28 19:49:00 2009 mkanat [...] cpan.org - Correspondence added

Hey Mark. This problem continues to affect Bugzilla users who have Bugzilla behind a proxy (which is quite a few users, particularly large installations) pretty severely. Do you think there's any chance of getting a fix for it?

Sat Nov 28 22:34:36 2009 yanick [...] babyl.dyndns.org - Correspondence added

Subject:	Re: [rt.cpan.org #51562] url() should use virtual_port(), not server_port()
Date:	Sat, 28 Nov 2009 22:36:40 -0500
To:	bug-CGI.pm [...] rt.cpan.org
From:	Yanick Champoux <yanick [...] babyl.dyndns.org>

Max Kanat-Alexander via RT wrote: Show quoted text

> Hey Mark. This problem continues to affect Bugzilla users who have > Bugzilla behind a proxy (which is quite a few users, particularly large > installations) pretty severely. Do you think there's any chance of > getting a fix for it?

I have some hacking time planned for tomorrow. I'll try to have something done for this particular bug along the lines of your previous feedback. `/anick

Sun Nov 29 12:12:00 2009 yanick [...] babyl.dyndns.org - Correspondence added

Subject:	Re: [rt.cpan.org #51562] url() should use virtual_port(), not server_port()
Date:	Sun, 29 Nov 2009 12:14:11 -0500
To:	bug-CGI.pm [...] rt.cpan.org
From:	Yanick Champoux <yanick [...] babyl.dyndns.org>

yanick@babyl.dyndns.org via RT wrote: Show quoted text

> I'll try to have > something done for this particular bug along the lines of your previous > feedback.

I've created a branch with the potential fix at http://github.com/yanick/CGI.pm/commits/rt-51562 The fix itself was very simple and in line with what was discussed earler: change the call to server_port() for virtual_port(). I also added a small test file, url.t, to test the behavior change.

Wed Dec 02 22:10:34 2009 MARKSTOS [...] cpan.org - Taken

Wed Dec 02 22:24:29 2009 MARKSTOS [...] cpan.org - Correspondence added

Thanks to you both-- I've reviewed the changes for this and have merged them into my git repo.

Wed Dec 02 22:24:30 2009 MARKSTOS [...] cpan.org - Status changed from 'open' to 'patched'

Fri Feb 05 22:26:34 2010 MARKSTOS [...] cpan.org - Correspondence added

I believe a fix for this was released today in the 3.49 release.

Fri Feb 05 22:26:35 2010 The RT System itself - Status changed from 'patched' to 'open'

Fri Feb 05 22:26:35 2010 MARKSTOS [...] cpan.org - Status changed from 'open' to 'resolved'

Fri May 23 14:29:34 2014 The RT System itself - Queue changed from CGI.pm to CGI

Bug #51562 for CGI: url() should use virtual_port(), not server_port()

Preferred bug tracker