diff options
| -rw-r--r-- | notes/catalyst-master-merge-todos.txt | 1 | ||||
| -rw-r--r-- | perllib/FixMyStreet/App/Controller/Report.pm | 14 | ||||
| -rw-r--r-- | t/app/controller/report_display.t | 5 |
3 files changed, 15 insertions, 5 deletions
diff --git a/notes/catalyst-master-merge-todos.txt b/notes/catalyst-master-merge-todos.txt index c3e93ae48..200fda0a5 100644 --- a/notes/catalyst-master-merge-todos.txt +++ b/notes/catalyst-master-merge-todos.txt @@ -6,3 +6,4 @@ should we ditch flickr import? (does not seem to be getting huge usage and those move all FAQ over to TT2 +check that using specific 404 and 410 pages is ok for reports diff --git a/perllib/FixMyStreet/App/Controller/Report.pm b/perllib/FixMyStreet/App/Controller/Report.pm index fe6310bbf..b457faa50 100644 --- a/perllib/FixMyStreet/App/Controller/Report.pm +++ b/perllib/FixMyStreet/App/Controller/Report.pm @@ -54,17 +54,21 @@ sub display : Path('') : Args(1) { # my %input_h = map { $_ => $q->param($_) ? ent($q->param($_)) : '' } @vars; # my $base = Cobrand::base_url($cobrand); - # Some council with bad email software - if ( $id =~ m{ ^ 3D (\d+) $ }x ) { + if ( + $id =~ m{ ^ 3D (\d+) $ }x # Some council with bad email software + || $id =~ m{ ^(\d+) \D .* $ }x # trailing garbage + ) + { return $c->res->redirect( $c->uri_for($1), 301 ); } # try to load a report if the id is a number - my $problem = - $id =~ m{\D} - ? undef + my $problem # + = $id =~ m{\D} # is id non-numeric? + ? undef # ...don't even search : $c->model('DB::Problem')->find( { id => $id } ); + # check that the problem is suitable to show. if ( !$problem || $problem->state eq 'unconfirmed' ) { $c->detach( '/page_error_404_not_found', [ _('Unknown problem ID') ] ); } diff --git a/t/app/controller/report_display.t b/t/app/controller/report_display.t index f979cc4ed..f0f1bdcba 100644 --- a/t/app/controller/report_display.t +++ b/t/app/controller/report_display.t @@ -55,6 +55,11 @@ subtest "test bad council email clients web links" => sub { is $mech->uri->path, "/report/$report_id", "at /report/$report_id"; }; +subtest "test tailing non-ints get stripped" => sub { + $mech->get_ok("/report/${report_id}xx "); + is $mech->uri->path, "/report/$report_id", "at /report/$report_id"; +}; + subtest "test bad ids get dealt with (404)" => sub { foreach my $id ( 'XXX', 99999999 ) { ok $mech->get("/report/$id"), "get '/report/$id'"; |