Author: kake Date: 2012-03-18 09:48:44 +0000 (Sun, 18 Mar 2012) New Revision: 1315

Added: trunk/t/92_node_name_from_cgi_obj.t trunk/t/93_redirect_without_spaces.t Modified: trunk/Changes trunk/MANIFEST trunk/lib/OpenGuides/CGI.pm trunk/wiki.cgi Log: Added code to deal with node name parameters in URLs having spaces instead of underscores, to take account of people typing manually (#114).

Modified: trunk/Changes =================================================================== --- trunk/Changes 2012-03-16 16:08:13 UTC (rev 1314) +++ trunk/Changes 2012-03-18 09:48:44 UTC (rev 1315) @@ -5,6 +5,9 @@

0.66 ? All templates now have access to the "username" TT variable. + Node name parameters in URLs are now accepted with spaces instead of + underscores, to take account of people typing manually - these will + redirect (303) to the proper URLs with underscores (#114). Add CSS classes for each category and locale to the "content" div (and hence we now require version 2.24 of Template). Switch to using Geo::Coordinates::OSGB/ITM instead of

Modified: trunk/MANIFEST =================================================================== --- trunk/MANIFEST 2012-03-16 16:08:13 UTC (rev 1314) +++ trunk/MANIFEST 2012-03-18 09:48:44 UTC (rev 1315) @@ -156,6 +156,8 @@ t/86_recent_changes.t t/90_css_category_locale_classes.t t/91_username_in_templates.t +t/92_node_name_from_cgi_obj.t +t/93_redirect_without_spaces.t t/templates/15_test.tt wiki.cgi META.json

Modified: trunk/lib/OpenGuides/CGI.pm =================================================================== --- trunk/lib/OpenGuides/CGI.pm 2012-03-16 16:08:13 UTC (rev 1314) +++ trunk/lib/OpenGuides/CGI.pm 2012-03-18 09:48:44 UTC (rev 1315) @@ -71,6 +71,142 @@

=over 4

+=item B<extract_node_param> + + my $config_file = $ENV{OPENGUIDES_CONFIG_FILE} || "wiki.conf"; + my $config = OpenGuides::Config->new( file => $config_file ); + my $guide = OpenGuides->new( config => $config ); + my $wiki = $guide->wiki; + + my $q = CGI->new; + + my $node_param = OpenGuides::CGI->extract_node_param( + wiki => $wiki, cgi_obj => $q ); + +Returns the title, id, or keywords parameter from the URL. Normally +this will be something like "British_Museum", i.e. with underscores +instead of spaces. However if the URL does contain spaces (encoded as +%20 or +), the return value will be e.g. "British Museum" instead. + +Croaks unless a LWiki::Toolkit object is supplied as C<wiki> and a +L<CGI> object is supplied as C<cgi_obj>. + +=cut + +sub extract_node_param { + my ($class, %args) = @_; + my $wiki = $args{wiki} or croak "No wiki supplied"; + croak "wiki not a Wiki::Toolkit object" + unless UNIVERSAL::isa( $wiki, "Wiki::Toolkit" ); + my $q = $args{cgi_obj} or croak "No cgi_obj supplied"; + croak "cgi_obj not a CGI object" + unless UNIVERSAL::isa( $q, "CGI" ); + + # Note $q->param( "keywords" ) gives you the entire param string. + # We need this to do URLs like foo.com/wiki.cgi?This_Page + my $param = $q->param( "id" ) + || $q->param( "title" ) + || join( " ", $q->param( "keywords" ) ) + || ""; + $param =~ s/%20/ /g; + $param =~ s/+/ /g; + return $param; +} + +=item B<extract_node_name> + + my $config_file = $ENV{OPENGUIDES_CONFIG_FILE} || "wiki.conf"; + my $config = OpenGuides::Config->new( file => $config_file ); + my $guide = OpenGuides->new( config => $config ); + my $wiki = $guide->wiki; + + my $q = CGI->new; + + my $node_name = OpenGuides::CGI->extract_node_name( + wiki => $wiki, cgi_obj => $q ); + +Returns the name of the node the user wishes to display/manipulate, as +we expect it to be stored in the database. Normally this will be +something like "British Museum", i.e. with spaces in. Croaks unless a +LWiki::Toolkit object is supplied as C<wiki> and a L<CGI> object is +supplied as C<cgi_obj>. + +=cut + +sub extract_node_name { + my ($class, %args) = @_; + # The next call will validate our args for us and croak if necessary. + my $param = $class->extract_node_param( %args ); + + # Sometimes people type spaces instead of underscores. + $param =~ s/ /_/g; + $param =~ s/%20/_/g; + $param =~ s/+/_/g; + + my $formatter = $args{wiki}->formatter; + return $formatter->node_param_to_node_name( $param ); +} + +=item B<check_spaces_redirect> + + my $config_file = $ENV{OPENGUIDES_CONFIG_FILE} || "wiki.conf"; + my $config = OpenGuides::Config->new( file => $config_file ); + my $guide = OpenGuides->new( config => $config ); + + my $q = CGI->new; + + my $url = OpenGuides::CGI->check_spaces_redirect( + wiki => $wiki, cgi_obj => $q ); + +If the user seems to have typed a URL with spaces in the node param +instead of underscores, this method will return the URL with the +underscores put in. Otherwise, it returns false. + +=cut + +sub check_spaces_redirect { + my ($class, %args) = @_; + my $wiki = $args{wiki}; + my $q = $args{cgi_obj}; + + my $name = $class->extract_node_name( wiki => $wiki, cgi_obj => $q ); + my $param = $class->extract_node_param( wiki => $wiki, cgi_obj => $q ); + + # If we can't figure out the name or param, it's safest to do nothing. + if ( !$name || !$param ) { + return 0; + } + + # If the name has no spaces in, or the name and param differ, we're + # probably OK. + if ( ( $name !~ / / ) || ( $name ne $param ) ) { + return 0; + } + + # Make a new CGI object to manipulate, to avoid action-at-a-distance. + my $new_q = CGI->new( $q ); + my $formatter = $wiki->formatter; + my $real_param = $formatter->node_name_to_node_param( $name ); + + if ( $q->param( "id" ) ) { + $new_q->param( -name => "id", -value => $real_param ); + } elsif ( $q->param( "title" ) ) { + $new_q->param( -name => "title", -value => $real_param ); + } else { + # OK, we have the keywords case; the entire param string is the + # node param. So just delete all existing parameters and stick + # the node param back in. + $new_q->delete_all(); + $new_q->param( -name => "id", -value => $real_param ); + } + + my $url = $new_q->self_url; + + # Escaped commas are ugly. + $url =~ s/%2C/,/g; + return $url; +} + =item B<make_prefs_cookie>

my $cookie = OpenGuides::CGI->make_prefs_cookie(

Added: trunk/t/92_node_name_from_cgi_obj.t =================================================================== --- trunk/t/92_node_name_from_cgi_obj.t (rev 0) +++ trunk/t/92_node_name_from_cgi_obj.t 2012-03-18 09:48:44 UTC (rev 1315) @@ -0,0 +1,101 @@ +use strict; +use OpenGuides; +use OpenGuides::CGI; +use OpenGuides::Test; +use Test::More; + +eval { require DBD::SQLite; }; +if ( $@ ) { + my ($error) = $@ =~ /^(.*?)\n/; + plan skip_all => + "DBD::SQLite could not be used - no database to test with. ($error)"; +} + +plan tests => 18; + +my $config = OpenGuides::Test->make_basic_config; +my $guide = OpenGuides->new( config => $config ); +my $wiki = $guide->wiki; + +# Clear out the database from any previous runs. +OpenGuides::Test::refresh_db(); + +# Write a node. +OpenGuides::Test->write_data( + guide => $guide, + node => "Ship Of Fools", + return_output => 1, + ); + +my ( $q, $node, $param ); + +# Test we get the right name/param with various CGI objects. Make sure to +# always start with an empty one by passing the empty string as arg. + +$q = CGI->new( "" ); +$q->param( -name => "id", -value => "Ship_Of_Fools" ); +$node = OpenGuides::CGI->extract_node_name( cgi_obj => $q, wiki => $wiki ); +is( $node, "Ship Of Fools", + "extract_node_name gives correct name with id param" ); +$param = OpenGuides::CGI->extract_node_param( cgi_obj => $q, wiki => $wiki ); +is( $param, "Ship_Of_Fools", "...as does extract_node_param" ); + +$q = CGI->new( "" ); +$q->param( -name => "title", -value => "Ship_Of_Fools" ); +$node = OpenGuides::CGI->extract_node_name( cgi_obj => $q, wiki => $wiki ); +is( $node, "Ship Of Fools", "title param works for node name" ); +$param = OpenGuides::CGI->extract_node_param( cgi_obj => $q, wiki => $wiki ); +is( $param, "Ship_Of_Fools", "...and for node param" ); + +$q = CGI->new( "Ship_Of_Fools" ); +$node = OpenGuides::CGI->extract_node_name( cgi_obj => $q, wiki => $wiki ); +is( $node, "Ship Of Fools", "whole-string node param works for node name" ); +$param = OpenGuides::CGI->extract_node_param( cgi_obj => $q, wiki => $wiki ); +is( $param, "Ship_Of_Fools", "...and for node param" ); + +# Now try it with encoded spaces instead of underscores. +$q = CGI->new( "" ); +$q->param( -name => "id", -value => "Ship%20Of%20Fools" ); +$node = OpenGuides::CGI->extract_node_name( cgi_obj => $q, wiki => $wiki ); +is( $node, "Ship Of Fools", + "id param works for node name with encoded spaces" ); +$param = OpenGuides::CGI->extract_node_param( cgi_obj => $q, wiki => $wiki ); +is( $param, "Ship Of Fools", "...as does node param" ); + +$q = CGI->new( "" ); +$q->param( -name => "title", -value => "Ship%20Of%20Fools" ); +$node = OpenGuides::CGI->extract_node_name( cgi_obj => $q, wiki => $wiki ); +is( $node, "Ship Of Fools", + "title param works for node name with encoded spaces" ); +$param = OpenGuides::CGI->extract_node_param( cgi_obj => $q, wiki => $wiki ); +is( $param, "Ship Of Fools", "...as does node param" ); + +$q = CGI->new( "Ship%20Of%20Fools" ); +$node = OpenGuides::CGI->extract_node_name( cgi_obj => $q, wiki => $wiki ); +is( $node, "Ship Of Fools", + "whole-string node param works for node name with encoded spaces" ); +$param = OpenGuides::CGI->extract_node_param( cgi_obj => $q, wiki => $wiki ); +is( $param, "Ship Of Fools", "...as does node param" ); + +# Finally try it with plus signs. +$q = CGI->new( "" ); +$q->param( -name => "id", -value => "Ship+Of+Fools" ); +$node = OpenGuides::CGI->extract_node_name( cgi_obj => $q, wiki => $wiki ); +is( $node, "Ship Of Fools", "id param works for node name with plus signs" ); +$param = OpenGuides::CGI->extract_node_param( cgi_obj => $q, wiki => $wiki ); +is( $param, "Ship Of Fools", "...as does node param" ); + +$q = CGI->new( "" ); +$q->param( -name => "title", -value => "Ship+Of+Fools" ); +$node = OpenGuides::CGI->extract_node_name( cgi_obj => $q, wiki => $wiki ); +is( $node, "Ship Of Fools", + "title param works for node name with plus signs" ); +$param = OpenGuides::CGI->extract_node_param( cgi_obj => $q, wiki => $wiki ); +is( $param, "Ship Of Fools", "...as does node param" ); + +$q = CGI->new( "Ship+Of+Fools" ); +$node = OpenGuides::CGI->extract_node_name( cgi_obj => $q, wiki => $wiki ); +is( $node, "Ship Of Fools", + "whole-string node param works for node name with plus signs" ); +$param = OpenGuides::CGI->extract_node_param( cgi_obj => $q, wiki => $wiki ); +is( $param, "Ship Of Fools", "...as does node param" );

Added: trunk/t/93_redirect_without_spaces.t =================================================================== --- trunk/t/93_redirect_without_spaces.t (rev 0) +++ trunk/t/93_redirect_without_spaces.t 2012-03-18 09:48:44 UTC (rev 1315) @@ -0,0 +1,139 @@ +use strict; +use OpenGuides; +use OpenGuides::CGI; +use OpenGuides::Test; +use Test::More; + +eval { require DBD::SQLite; }; +if ( $@ ) { + my ($error) = $@ =~ /^(.*?)\n/; + plan skip_all => + "DBD::SQLite could not be used - no database to test with. ($error)"; +} + +plan tests => 27; + +my $config = OpenGuides::Test->make_basic_config; +my $guide = OpenGuides->new( config => $config ); +my $wiki = $guide->wiki; + +# Clear out the database from any previous runs. +OpenGuides::Test::refresh_db(); + +# Write a couple of nodes, one with a single-word name, one with multiple. +OpenGuides::Test->write_data( + guide => $guide, + node => "Croydon", + return_output => 1, + ); +OpenGuides::Test->write_data( + guide => $guide, + node => "Ship Of Fools", + return_output => 1, + ); + +my ( $q, $url ); + +# Check we don't get redirects with the single-word node. + +$q = CGI->new( "" ); +$q->param( -name => "id", -value => "Croydon" ); +$url = OpenGuides::CGI->check_spaces_redirect( cgi_obj => $q, wiki => $wiki ); +ok( !$url, "No URL redirect for id param with single-word node" ); + +$q = CGI->new( "" ); +$q->param( -name => "title", -value => "Croydon" ); +$url = OpenGuides::CGI->check_spaces_redirect( cgi_obj => $q, wiki => $wiki ); +ok( !$url, "...nor for title param" ); + +$q = CGI->new( "Croydon" ); +$url = OpenGuides::CGI->check_spaces_redirect( cgi_obj => $q, wiki => $wiki ); +ok( !$url, "...nor for whole-string param" ); + +# Nor with the "proper" URLs with underscores. + +$q = CGI->new( "" ); +$q->param( -name => "id", -value => "Ship_Of_Fools" ); +$url = OpenGuides::CGI->check_spaces_redirect( cgi_obj => $q, wiki => $wiki ); +ok( !$url, "No URL redirect for id param with underscores" ); + +$q = CGI->new( "" ); +$q->param( -name => "title", -value => "Ship_Of_Fools" ); +$url = OpenGuides::CGI->check_spaces_redirect( cgi_obj => $q, wiki => $wiki ); +ok( !$url, "...nor for title param with underscores" ); + +$q = CGI->new( "Ship_Of_Fools" ); +$url = OpenGuides::CGI->check_spaces_redirect( cgi_obj => $q, wiki => $wiki ); +ok( !$url, "...nor for whole-string node param with underscores" ); + +# Now check that we get redirects when supplying CGI objects with spaces +# in the node parameter. + +# First encoded spaces. + +$q = CGI->new( "" ); +$q->param( -name => "id", -value => "Ship%20Of%20Fools" ); +$url = OpenGuides::CGI->check_spaces_redirect( cgi_obj => $q, wiki => $wiki ); +ok( $url, "We do get a redirect for id param with encoded spaces" ); +is( $url, "http://localhost?id=Ship_Of_Fools", "...the right one" ); +$q->param( -name => "action", -value => "edit" ); +$url = OpenGuides::CGI->check_spaces_redirect( cgi_obj => $q, wiki => $wiki ); +ok( $url, "...also get redirect with edit param" ); +is( $url, "http://localhost?id=Ship_Of_Fools;action=edit", + "...the right one" ); + +$q = CGI->new( "" ); +$q->param( -name => "title", -value => "Ship%20Of%20Fools" ); +$url = OpenGuides::CGI->check_spaces_redirect( cgi_obj => $q, wiki => $wiki ); +ok( $url, "...also get redirect for title param with encoded spaces" ); +is( $url, "http://localhost?title=Ship_Of_Fools", "...the right one" ); +$q->param( -name => "action", -value => "edit" ); +$url = OpenGuides::CGI->check_spaces_redirect( cgi_obj => $q, wiki => $wiki ); +ok( $url, "...also get redirect with edit param" ); +is( $url, "http://localhost?title=Ship_Of_Fools;action=edit", + "...the right one" ); + +$q = CGI->new( "Ship%20Of%20Fools" ); +$url = OpenGuides::CGI->check_spaces_redirect( cgi_obj => $q, wiki => $wiki ); +ok( $url, + "...also get redirect for whole-string node param with encoded spaces" ); +is( $url, "http://localhost?id=Ship_Of_Fools", "...the right one" ); + +# Try it with plus signs. + +$q = CGI->new( "" ); +$q->param( -name => "id", -value => "Ship+Of+Fools" ); +$url = OpenGuides::CGI->check_spaces_redirect( cgi_obj => $q, wiki => $wiki ); +ok( $url, "We do get a redirect for id param with plus signs" ); +is( $url, "http://localhost?id=Ship_Of_Fools", "...the right one" ); +$q->param( -name => "action", -value => "edit" ); +$url = OpenGuides::CGI->check_spaces_redirect( cgi_obj => $q, wiki => $wiki ); +ok( $url, "...also get redirect with edit param" ); +is( $url, "http://localhost?id=Ship_Of_Fools;action=edit", + "...the right one" ); + +$q = CGI->new( "" ); +$q->param( -name => "title", -value => "Ship+Of+Fools" ); +$url = OpenGuides::CGI->check_spaces_redirect( cgi_obj => $q, wiki => $wiki ); +ok( $url, "...and for title param with plus signs" ); +is( $url, "http://localhost?title=Ship_Of_Fools", "...the right one" ); +$q->param( -name => "action", -value => "edit" ); +$url = OpenGuides::CGI->check_spaces_redirect( cgi_obj => $q, wiki => $wiki ); +ok( $url, "...also get redirect with edit param" ); +is( $url, "http://localhost?title=Ship_Of_Fools;action=edit", + "...the right one" ); + +$q = CGI->new( "Ship+Of+Fools" ); +$url = OpenGuides::CGI->check_spaces_redirect( cgi_obj => $q, wiki => $wiki ); +ok( $url, "...and for whole-string node param with plus signs" ); +is( $url, "http://localhost?id=Ship_Of_Fools", "...the right one" ); + +# Make sure commas don't get escaped, for it is unnecessary and ugly. +OpenGuides::Test->write_data( + guide => $guide, + node => "Londis, Pitlake", + return_output => 1, + ); +$q = CGI->new( "Londis, Pitlake" ); +$url = OpenGuides::CGI->check_spaces_redirect( cgi_obj => $q, wiki => $wiki ); +is( $url, "http://localhost?id=Londis,_Pitlake", "Commas don't get escaped." );

Modified: trunk/wiki.cgi =================================================================== --- trunk/wiki.cgi 2012-03-16 16:08:13 UTC (rev 1314) +++ trunk/wiki.cgi 2012-03-18 09:48:44 UTC (rev 1315) @@ -30,15 +30,21 @@ $guide = OpenGuides->new( config => $config ); $wiki = $guide->wiki; $formatter = $wiki->formatter; - - # Get CGI object, find out what to do. $q = CGI->new;

- # Note $q->param('keywords') gives you the entire param string. - # We need this to do URLs like foo.com/wiki.cgi?This_Page - my $node = $q->param('id') || $q->param('title') || $q->param('keywords') || ''; - $node = $formatter->node_param_to_node_name( $node ); + # See if we need to redirect due to spaces in URL. + my $redirect = OpenGuides::CGI->check_spaces_redirect( + cgi_obj => $q, wiki => $wiki );

+ if ( $redirect ) { + print $q->redirect( -uri => $redirect, -status => 303 ); + exit 0; + } + + # No redirect - carry on. + my $node = OpenGuides::CGI->extract_node_name( + cgi_obj => $q, wiki => $wiki ); + # If we did a post, then CGI->param probably hasn't fully de-escaped, # in the same way as a get would've done my $request_method = $q->request_method() || '';