Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Template::Filters - uri_filter/url_filter regex needs updating #13

Closed
mar-kolya opened this issue Nov 22, 2012 · 5 comments
Closed

Template::Filters - uri_filter/url_filter regex needs updating #13

mar-kolya opened this issue Nov 22, 2012 · 5 comments
Assignees

Comments

@mar-kolya
Copy link

URI::Escape now uses the RFC3986 regex as below (lines from URI::Escape)

my %Unsafe = (
RFC2732 => qr/[^A-Za-z0-9-_.!~*'()]/,
RFC3986 => qr/[^A-Za-z0-9-._~"]/
);

lines 282 and 307 in Template::Filters should be updated accordingly to match the new RFC spec. (or allow config setting of RFC version)

Or maybe it's worth just using URI::Escape instead.

This is basically copy of issue reported on cpan: https://rt.cpan.org/Public/Bug/Display.html?id=57590

Thanks!

@abw
Copy link
Owner

abw commented Jan 8, 2014

Now updated. Thanks for bringing it to my attention.

4dfa101

@ghost ghost assigned abw Jan 8, 2014
@abw abw closed this as completed Jan 8, 2014
@ghost
Copy link

ghost commented Feb 10, 2014

The regexp reported by mar-kolya for RFC 3986 is incorrect. Double quotes are not part of the regexp and must still be escaped, see:

http://tools.ietf.org/html/rfc3986#section-2.3
http://search.cpan.org/~gaas/URI/URI/Escape.pm

my %Unsafe = (
RFC2732 => qr/[^A-Za-z0-9-_.!~*'()]/,
RFC3986 => qr/[^A-Za-z0-9-._~]/,
);

Please fix that (and this would be a huge regression anyway as <a href="http://www.foo.com/id=[% foo FILTER uri %]&name=[% bar FILTER uri %]" would be broken if one of the variables contains a double quote in its value as it would prematurely close the URL).

Looks like I cannot reopen this bug... :(

@ghost
Copy link

ghost commented Feb 10, 2014

Also note that the regexps from my previous comment are mangled by github as it removed backslashes and underscores. So make sure to not copy and paste from here.

@mar-kolya
Copy link
Author

LpSolit, you are absolutely correct, you may want to open a new issue for that.

In my original comment I've just copy-pased a bug from rt.cpan.org without complete verification. And the bug on rt.cpan.org was created before URI::Escape had that quote remove. My apologies for not verifying that bug report against RFC.

@ghost
Copy link

ghost commented Feb 10, 2014

I filed issue #35 about the regression.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants