From 9d1e5fadd7d18f4c96ab0509d673040e34225a04 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sun, 14 Aug 2016 10:21:09 +0000 Subject: www: do not unecessarily escape some chars in paths Based on reading RFC 3986, it seems '@', ':', '!', '$', '&', "'", '; '(', ')', '*', '+', ',', ';', '=' are all allowed in path-absolute where we have the Message-ID. In any case, it seems '@' is fairly common in path components nowadays and too common in Message-IDs. --- t/cgi.t | 14 +++++++------- t/check-www-inbox.perl | 4 ---- t/mid.t | 11 +++++++++++ t/nntp.t | 4 ++-- t/plack.t | 28 ++++++++++++++-------------- t/psgi_mount.t | 2 +- 6 files changed, 35 insertions(+), 28 deletions(-) create mode 100644 t/mid.t (limited to 't') diff --git a/t/cgi.t b/t/cgi.t index a0f09c59..092ad8c7 100644 --- a/t/cgi.t +++ b/t/cgi.t @@ -118,7 +118,7 @@ EOF like($res->{head}, qr/Status:\s*206/i, "info/refs partial past end OK"); is($res->{body}, substr($orig, 5), 'partial body OK past end'); } - +use Data::Dumper; # atom feeds { local $ENV{HOME} = $home; @@ -126,7 +126,7 @@ EOF like($res->{body}, qr/test for public-inbox/, "set title in XML feed"); like($res->{body}, - qr!http://test\.example\.com/test/blah%40example\.com/!, + qr!http://test\.example\.com/test/blah\@example\.com/!, "link id set"); like($res->{body}, qr/what\?/, "reply included"); } @@ -148,7 +148,7 @@ EOF $im->add($reply); $im->done; - my $res = cgi_run("/test/slashy%2fasdf%40example.com/raw"); + my $res = cgi_run("/test/slashy%2fasdf\@example.com/raw"); like($res->{body}, qr/Message-Id: <\Q$slashy_mid\E>/, "slashy mid raw hit"); @@ -167,20 +167,20 @@ EOF $res = cgi_run("/test/blahblah\@example.com/f/"); like($res->{head}, qr/Status: 301 Moved/, "301 response"); like($res->{head}, - qr!^Location: http://[^/]+/test/blahblah%40example\.com/\r\n!ms, + qr!^Location: http://[^/]+/test/blahblah\@example\.com/\r\n!ms, '301 redirect location'); $res = cgi_run("/test/blahblah\@example.con/"); like($res->{head}, qr/Status: 300 Multiple Choices/, "mid html miss"); $res = cgi_run("/test/new.html"); - like($res->{body}, qr/slashy%2Fasdf%40example\.com/, + like($res->{body}, qr/slashy%2Fasdf\@example\.com/, "slashy URL generated correctly"); } # retrieve thread as an mbox { local $ENV{HOME} = $home; - my $path = "/test/blahblah%40example.com/t.mbox.gz"; + my $path = "/test/blahblah\@example.com/t.mbox.gz"; my $res = cgi_run($path); like($res->{head}, qr/^Status: 501 /, "search not-yet-enabled"); my $indexed = system($index, $maindir) == 0; @@ -200,7 +200,7 @@ EOF my $have_xml_feed = eval { require XML::Feed; 1 } if $indexed; if ($have_xml_feed) { - $path = "/test/blahblah%40example.com/t.atom"; + $path = "/test/blahblah\@example.com/t.atom"; $res = cgi_run($path); like($res->{head}, qr/^Status: 200 /, "atom returned 200"); like($res->{head}, qr!^Content-Type: application/atom\+xml!m, diff --git a/t/check-www-inbox.perl b/t/check-www-inbox.perl index 6be631e9..4319049c 100644 --- a/t/check-www-inbox.perl +++ b/t/check-www-inbox.perl @@ -131,10 +131,6 @@ sub worker_loop { warn "W: ".$r->code . " $u\n" } - # check bad links - my @at = grep(/@/, @links); - print "BAD: $u ", join("\n", @at), "\n" if @at; - my $s; # blocking foreach my $l (@links, "DONE\t$u") { diff --git a/t/mid.t b/t/mid.t new file mode 100644 index 00000000..b0af8386 --- /dev/null +++ b/t/mid.t @@ -0,0 +1,11 @@ +# Copyright (C) 2016 all contributors <meta@public-inbox.org> +# License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt> +use Test::More; +use PublicInbox::MID qw(mid_escape); + +is(mid_escape('foo!@(bar)'), 'foo!@(bar)'); +is(mid_escape('foo%!@(bar)'), 'foo%25!@(bar)'); +is(mid_escape('foo%!@(bar)'), 'foo%25!@(bar)'); + +done_testing(); +1; diff --git a/t/nntp.t b/t/nntp.t index de07abb0..7500d6b9 100644 --- a/t/nntp.t +++ b/t/nntp.t @@ -112,7 +112,7 @@ use_ok 'PublicInbox::Inbox'; PublicInbox::NNTP::set_nntp_headers($mime->header_obj, $ng, 1, $mid); is_deeply([ $mime->header('Message-ID') ], [ "<$mid>" ], 'Message-ID unchanged'); - is_deeply([ $mime->header('Archived-At') ], [ "<${u}a%40b/>" ], + is_deeply([ $mime->header('Archived-At') ], [ "<${u}a\@b/>" ], 'Archived-At: set'); is_deeply([ $mime->header('List-Archive') ], [ "<$u>" ], 'List-Archive: set'); @@ -128,7 +128,7 @@ use_ok 'PublicInbox::Inbox'; is_deeply([ $mime->header('Message-ID') ], [ "<$mid>" ], 'Message-ID unchanged'); is_deeply([ $mime->header('Archived-At') ], - [ "<${u}a%40b/>", '<http://mirror.example.com/m/a%40b/>' ], + [ "<${u}a\@b/>", '<http://mirror.example.com/m/a@b/>' ], 'Archived-At: appended'); is_deeply([ $mime->header('Xref') ], [ 'example.com test:2' ], 'Old Xref: clobbered'); diff --git a/t/plack.t b/t/plack.t index db3a9b23..608afb9e 100644 --- a/t/plack.t +++ b/t/plack.t @@ -97,7 +97,7 @@ EOF foreach my $t (qw(t T)) { test_psgi($app, sub { my ($cb) = @_; - my $u = $pfx . "/blah%40example.com/$t"; + my $u = $pfx . "/blah\@example.com/$t"; my $res = $cb->(GET($u)); is(301, $res->code, "redirect for missing /"); my $location = $res->header('Location'); @@ -108,11 +108,11 @@ EOF foreach my $t (qw(f)) { test_psgi($app, sub { my ($cb) = @_; - my $u = $pfx . "/blah%40example.com/$t"; + my $u = $pfx . "/blah\@example.com/$t"; my $res = $cb->(GET($u)); is(301, $res->code, "redirect for legacy /f"); my $location = $res->header('Location'); - like($location, qr!/blah%40example\.com/\z!, + like($location, qr!/blah\@example\.com/\z!, 'redirected with missing /'); }); } @@ -124,7 +124,7 @@ EOF is(200, $res->code, 'success response received'); like($res->content, qr!href="new\.atom"!, 'atom URL generated'); - like($res->content, qr!href="blah%40example\.com/"!, + like($res->content, qr!href="blah\@example\.com/"!, 'index generated'); }); @@ -133,13 +133,13 @@ EOF my $res = $cb->(GET($pfx . '/atom.xml')); is(200, $res->code, 'success response received for atom'); like($res->content, - qr!link\s+href="\Q$pfx\E/blah%40example\.com/"!s, + qr!link\s+href="\Q$pfx\E/blah\@example\.com/"!s, 'atom feed generated correct URL'); }); test_psgi($app, sub { my ($cb) = @_; - my $path = '/blah%40example.com/'; + my $path = '/blah@example.com/'; my $res = $cb->(GET($pfx . $path)); is(200, $res->code, "success for $path"); like($res->content, qr!<title>hihi - Me!, @@ -149,13 +149,13 @@ EOF $res = $cb->(GET($pfx . $path)); is(301, $res->code, "redirect for $path"); my $location = $res->header('Location'); - like($location, qr!/blah%40example\.com/\z!, + like($location, qr!/blah\@example\.com/\z!, '/$MESSAGE_ID/f/ redirected to /$MESSAGE_ID/'); }); test_psgi($app, sub { my ($cb) = @_; - my $res = $cb->(GET($pfx . '/blah%40example.com/raw')); + my $res = $cb->(GET($pfx . '/blah@example.com/raw')); is(200, $res->code, 'success response received for /*/raw'); like($res->content, qr!^From !sm, "mbox returned"); }); @@ -164,10 +164,10 @@ EOF foreach my $t (qw(m f)) { test_psgi($app, sub { my ($cb) = @_; - my $res = $cb->(GET($pfx . "/$t/blah%40example.com.txt")); + my $res = $cb->(GET($pfx . "/$t/blah\@example.com.txt")); is(301, $res->code, "redirect for old $t .txt link"); my $location = $res->header('Location'); - like($location, qr!/blah%40example\.com/raw\z!, + like($location, qr!/blah\@example\.com/raw\z!, ".txt redirected to /raw"); }); } @@ -180,22 +180,22 @@ EOF while (my ($t, $e) = each %umap) { test_psgi($app, sub { my ($cb) = @_; - my $res = $cb->(GET($pfx . "/$t/blah%40example.com.html")); + my $res = $cb->(GET($pfx . "/$t/blah\@example.com.html")); is(301, $res->code, "redirect for old $t .html link"); my $location = $res->header('Location'); like($location, - qr!/blah%40example\.com/$e(?:#u)?\z!, + qr!/blah\@example\.com/$e(?:#u)?\z!, ".html redirected to new location"); }); } foreach my $sfx (qw(mbox mbox.gz)) { test_psgi($app, sub { my ($cb) = @_; - my $res = $cb->(GET($pfx . "/t/blah%40example.com.$sfx")); + my $res = $cb->(GET($pfx . "/t/blah\@example.com.$sfx")); is(301, $res->code, 'redirect for old thread link'); my $location = $res->header('Location'); like($location, - qr!/blah%40example\.com/t\.mbox(?:\.gz)?\z!, + qr!/blah\@example\.com/t\.mbox(?:\.gz)?\z!, "$sfx redirected to /mbox.gz"); }); } diff --git a/t/psgi_mount.t b/t/psgi_mount.t index dae45baf..4a515c6a 100644 --- a/t/psgi_mount.t +++ b/t/psgi_mount.t @@ -67,7 +67,7 @@ test_psgi($app, sub { is($res->code, 200, 'OK with URLMap mount'); $res = $cb->(GET('/a/test/m/blah%40example.com.html')); is($res->header('Location'), - 'http://localhost/a/test/blah%40example.com/', + 'http://localhost/a/test/blah@example.com/', 'redirect functions properly under mount'); $res = $cb->(GET('/test/blah%40example.com/')); -- cgit v1.2.3-24-ge0c7