Mailing List Archive: Catalyst: Dev

Proposed patch - Re: May (can) I fix Catalyst/Engine/CGI.pm REDIRECT_URL + PATH_INFO bug?

Index | Next | Previous | View Threaded

apv at sedition

Oct 22, 2008, 3:27 PM

Post #1 of 3 (737 views)
Permalink

Proposed patch - Re: May (can) I fix Catalyst/Engine/CGI.pm REDIRECT_URL + PATH_INFO bug?

On Oct 21, 2008, at 8:38 PM, Ashley wrote:
> This is an old bug I reported the first time about two years ago,
> IIRC. Short version REDIRECT_URL + PATH_INFO can cause paths to get
> squirrelly if there are regex chars, which URIs can legally have.
>
>> - $base_path =~ s/$ENV{PATH_INFO}$//;
>> + $base_path =~ s/\Q$ENV{PATH_INFO}\E$//;

Attached is an svn diff -- redirect_url_substitution.patch -- against
Catalyst-Runtime/5.70/trunk, r8571. It updates t/
live_engine_request_uri.t, lib/Catalyst/Engine/CGI.pm, and Changes.

I didn't try to apply it but I'd be glad to (try at least) if a core
dev reviews it, or modify it if anyone directs it. If someone would
rather 'patch -p0', tweak, and commit yourself (as it were), that's
fine too. :)

There is a related bug report from Chris Dolan outstanding -- http://
rt.cpan.org/Ticket/Display.html?id=24951 -- which can be closed if
the patch is applied.

-Ashley
--
By the way, if you are going to work with this, please run the full
tests first. Some are failing (for me) in r8571 so you should see
that before trying my patch and then seeing it and thinking the patch
is to blame.

Attachments:

redirect_url_substitution.patch (2.54 KB)

marcus at nordaaker

Oct 23, 2008, 12:50 AM

Post #2 of 3 (689 views)
Permalink

Re: Proposed patch - Re: May (can) I fix Catalyst/Engine/CGI.pm REDIRECT_URL + PATH_INFO bug? [In reply to]

I don't think this merits a minor release on it's own. Can you
recreate it as a patch to the 5.8 branch?

Marcus

On 23. okt.. 2008, at 00.27, Ashley wrote:

> On Oct 21, 2008, at 8:38 PM, Ashley wrote:
>> This is an old bug I reported the first time about two years ago,
>> IIRC. Short version REDIRECT_URL + PATH_INFO can cause paths to get
>> squirrelly if there are regex chars, which URIs can legally have.
>>
>>> - $base_path =~ s/$ENV{PATH_INFO}$//;
>>> + $base_path =~ s/\Q$ENV{PATH_INFO}\E$//;
>
> Attached is an svn diff -- redirect_url_substitution.patch --
> against Catalyst-Runtime/5.70/trunk, r8571. It updates t/
> live_engine_request_uri.t, lib/Catalyst/Engine/CGI.pm, and Changes.
>
> I didn't try to apply it but I'd be glad to (try at least) if a core
> dev reviews it, or modify it if anyone directs it. If someone would
> rather 'patch -p0', tweak, and commit yourself (as it were), that's
> fine too. :)
>
> There is a related bug report from Chris Dolan outstanding -- http://rt.cpan.org/Ticket/Display.html?id=24951
> -- which can be closed if the patch is applied.
>
> -Ashley
> --
> By the way, if you are going to work with this, please run the full
> tests first. Some are failing (for me) in r8571 so you should see
> that before trying my patch and then seeing it and thinking the
> patch is to blame.
>
> <
> redirect_url_substitution
> .patch>_______________________________________________
> Catalyst-dev mailing list
> Catalyst-dev[at]lists.scsys.co.uk
> http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst-dev

_______________________________________________
Catalyst-dev mailing list
Catalyst-dev[at]lists.scsys.co.uk
http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst-dev

apv at sedition

Oct 23, 2008, 8:02 AM

Post #3 of 3 (681 views)
Permalink

Re: Proposed patch - Re: May (can) I fix Catalyst/Engine/CGI.pm REDIRECT_URL + PATH_INFO bug? [In reply to]

On Oct 23, 2008, at 12:50 AM, Marcus Ramberg wrote:
> I don't think this merits a minor release on it's own. Can you
> recreate it as a patch to the 5.8 branch?

Of course. Thanks!

-Ashley

>
>
> Marcus
>
> On 23. okt.. 2008, at 00.27, Ashley wrote:
>
>> On Oct 21, 2008, at 8:38 PM, Ashley wrote:
>>> This is an old bug I reported the first time about two years ago,
>>> IIRC. Short version REDIRECT_URL + PATH_INFO can cause paths to
>>> get squirrelly if there are regex chars, which URIs can legally
>>> have.
>>>
>>>> - $base_path =~ s/$ENV{PATH_INFO}$//;
>>>> + $base_path =~ s/\Q$ENV{PATH_INFO}\E$//;
>>
>> Attached is an svn diff -- redirect_url_substitution.patch --
>> against Catalyst-Runtime/5.70/trunk, r8571. It updates t/
>> live_engine_request_uri.t, lib/Catalyst/Engine/CGI.pm, and Changes.
>>
>> I didn't try to apply it but I'd be glad to (try at least) if a
>> core dev reviews it, or modify it if anyone directs it. If someone
>> would rather 'patch -p0', tweak, and commit yourself (as it were),
>> that's fine too. :)
>>
>> There is a related bug report from Chris Dolan outstanding -- http://rt.cpan.org/Ticket/Display.html?id=24951
>> -- which can be closed if the patch is applied.
>>
>> -Ashley
>> --
>> By the way, if you are going to work with this, please run the full
>> tests first. Some are failing (for me) in r8571 so you should see
>> that before trying my patch and then seeing it and thinking the
>> patch is to blame.
>>
>> <
>> redirect_url_substitution
>> .patch>_______________________________________________
>> Catalyst-dev mailing list
>> Catalyst-dev[at]lists.scsys.co.uk
>> http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst-dev
>
>
> _______________________________________________
> Catalyst-dev mailing list
> Catalyst-dev[at]lists.scsys.co.uk
> http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst-dev

_______________________________________________
Catalyst-dev mailing list
Catalyst-dev[at]lists.scsys.co.uk
http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst-dev

Index | Next | Previous | View Threaded

Interested in having your list archived? Contact lists@gossamer-threads.com