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 L<Wiki::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
+L<Wiki::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() || '';