Skip Menu |
 

Preferred bug tracker

Please visit the preferred bug tracker to report your issue.

This queue is for tickets about the CGI CPAN distribution.

Report information
The Basics
Id: 87790
Status: resolved
Priority: 0/
Queue: CGI

People
Owner: Nobody in particular
Requestors: lpsolit [...] gmail.com
Cc:
AdminCc:

Bug Information
Severity: (no value)
Broken in: (no value)
Fixed in: (no value)



Subject: url() shouldn't call query_string() unless needed
Date: Mon, 12 Aug 2013 13:27:52 +0200
To: bug-CGI.pm [...] rt.cpan.org
From: "Frédéric Buclin" <lpsolit [...] gmail.com>
Download (untitled) / with headers
text/plain 862b
Despite I call url() *without* query => 1, it calls query_string() anyway which is quite slow when the URL contains several hundreds of parameters (900 in my case): 70 ms per call! And I call this method several times in a single CGI script so that's quite annoying as the time penalty becomes very visible (0.3 second). The fix in url() is trivial: --- CGI.pm 2013-08-12 13:18:56.388940276 +0200 +++ CGI.pm 2013-08-12 13:19:06.518713206 +0200 @@ -2817,7 +2817,7 @@ sub url { my $path = $self->path_info; my $script_name = $self->script_name; my $request_uri = unescape($self->request_uri) || ''; - my $query_str = $self->query_string; + my $query_str = $query ? $self->query_string : ''; my $rewrite_in_use = $request_uri && $request_uri !~ /^\Q$script_name/; Could you please include this fix for CGI 3.64? :)
Subject: Re: [rt.cpan.org #87790] url() shouldn't call query_string() unless needed
Date: Mon, 12 Aug 2013 05:54:02 -0700
To: Frédéric Buclin via RT <bug-CGI.pm [...] rt.cpan.org>
From: Mark Stosberg <mark [...] stosberg.com>
Download (untitled) / with headers
text/plain 1.4k
Can you confirm that: 1. There is sufficient test coverage for the related cases. 2. The full test suite passes after the change? Mark On Mon, Aug 12, 2013, at 04:28 AM, Frédéric Buclin via RT wrote: Show quoted text
> Mon Aug 12 07:28:09 2013: Request 87790 was acted upon. > Transaction: Ticket created by lpsolit@gmail.com > Queue: CGI.pm > Subject: url() shouldn't call query_string() unless needed > Broken in: (no value) > Severity: (no value) > Owner: Nobody > Requestors: lpsolit@gmail.com > Status: new > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=87790 > > > > Despite I call url() *without* query => 1, it calls query_string() > anyway which is quite slow when the URL contains several hundreds of > parameters (900 in my case): 70 ms per call! And I call this method > several times in a single CGI script so that's quite annoying as the > time penalty becomes very visible (0.3 second). The fix in url() is > trivial: > > > --- CGI.pm 2013-08-12 13:18:56.388940276 +0200 > +++ CGI.pm 2013-08-12 13:19:06.518713206 +0200 > @@ -2817,7 +2817,7 @@ sub url { > my $path = $self->path_info; > my $script_name = $self->script_name; > my $request_uri = unescape($self->request_uri) || ''; > - my $query_str = $self->query_string; > + my $query_str = $query ? $self->query_string : ''; > > my $rewrite_in_use = $request_uri && $request_uri !~ > /^\Q$script_name/; > > > Could you please include this fix for CGI 3.64? :) > >
Subject: Re: [rt.cpan.org #87790] url() shouldn't call query_string() unless needed
Date: Mon, 12 Aug 2013 14:58:36 +0200
To: bug-CGI.pm [...] rt.cpan.org
From: "Frédéric Buclin" <lpsolit [...] gmail.com>
Download (untitled) / with headers
text/plain 571b
Le 12. 08. 13 14:54, Mark Stosberg via RT a écrit : Show quoted text
> Can you confirm that: > > 1. There is sufficient test coverage for the related cases. > 2. The full test suite passes after the change?
I really don't see how this could break stuff. Just look at the code itself: sub url { ... my $query_str = $self->query_string; ... $url .= "?$query_str" if $query and $query_str ne ''; } That's the only 2 lines which use $query_str. All my patch does is to skip a useless and time consuming call to query_string() when it's ignored a few lines later.
Download (untitled) / with headers
text/plain 241b
This issue has been copied to: https://github.com/leejo/CGI.pm/issues/121 please take all future correspondence there. This ticket will remain open but please do not reply here. This ticket will be closed when the github issue is dealt with.
Download (untitled) / with headers
text/plain 405b
commit e960d905c33009585ac594d4a88a91057b84c8b1 Author: Lee Johnson <lee@givengain.ch> Date: Sat Sep 20 16:54:18 2014 +0100 resolve #121 [rt.cpan.org #87790] - optimise url by not calling ->query_string if -query is not passed to the method since we *don't* append the query_string anyway in this case Changes | 2 ++ lib/CGI.pm | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-)


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.