Skip to content

Commit

Permalink
Item13185: Rework the URL path parser
Browse files Browse the repository at this point in the history 
Do better validations of the web path components and topic names.

Preserve the invalid names, so that they can be reported in oops
messages.

Add oops messages to Preview, Rename, Save and View when they are called
with an invalid web or topic name.  Previously they were often
sanitized to a valid default web or topic name.

Add an error message for invalid characters in a topic name.  Revise the
bad_web error to show the incorrect value.

Client tests were a test error.  It was setting a query parameter into
the query_path.

ViewScriptTests have 3 changes:
 - /Sandbox/WebIndex/?topic=WebChanges   Previously the WebIndex would
   be preserved in the web path,  Now it's detected as a valid topic
   name and overridden by the topic= parameter.
 - /Someweb/Subweb?topic=WebChanges   Prevously the Subweb was ignored
   because of no trailing slash,  and overridden by the topic name,
   returning /Someweb/WebChanges.   If the Subweb is a valid webname, it
   now returns  /Someweb/Subweb/WebChanges.
 - /A:B/WebHome  used to return an empty webName.  It now sets the
   default UsersWeb webname, but also sets invalidWeb,  so that an error
   will be thrown.
  • Loading branch information
@gac410
gac410 committed Mar 18, 2015
1 parent 0f0f263 commit 9838f11
Show file tree
Hide file tree
Showing 9 changed files with 296 additions and 82 deletions.
3 changes: 2 additions & 1 deletion UnitTestContrib/test/unit/ClientTests.pm
View file
Expand Up @@ -150,7 +150,8 @@ sub verify_edit {
};

$query = Unit::Request->new();
$query->path_info("/$this->{test_web}/$this->{test_topic}?breaklock=1");
$query->path_info("/$this->{test_web}/$this->{test_topic}");
$query->param( '-breaklock', 1 );

$this->createNewFoswikiSession( undef, $query );

Expand Down
11 changes: 7 additions & 4 deletions UnitTestContrib/test/unit/ViewScriptTests.pm
View file
Expand Up @@ -491,9 +491,9 @@ sub test_urlparsing {
$this->urltest( '/Sandbox//WebIndex?topic=WebChanges',
'Sandbox', 'WebChanges' );
$this->urltest( '/Sandbox/WebIndex/?topic=WebChanges',
'Sandbox/WebIndex', 'WebChanges' );
'Sandbox', 'WebChanges' );
$this->urltest( '/Sandbox/WebIndex//?topic=WebChanges',
'Sandbox/WebIndex', 'WebChanges' );
'Sandbox', 'WebChanges' );

$this->urltest( '/Sandbox?topic=WebChanges', 'Sandbox', 'WebChanges' );
$this->urltest( '/Sandbox/?topic=WebChanges', 'Sandbox', 'WebChanges' );
Expand Down Expand Up @@ -568,7 +568,7 @@ sub test_urlparsing {

#you may read 'subweb' but as its the last string in the url, foswiki treats it as a topicname
$this->urltest( '/' . $this->{test_subweb} . '?topic=WebChanges',
$this->{test_web}, 'WebChanges' );
$this->{test_subweb}, 'WebChanges' );

#trailing slashes make it a web again
$this->urltest( '/' . $this->{test_subweb} . '/?topic=WebChanges',
Expand Down Expand Up @@ -709,7 +709,10 @@ sub test_urlparsing {
#invalid..

# - Invalid web name - Tasks.Item8713
$this->urltest( '/A:B/WebPreferences', '', 'WebPreferences' );
$this->urltest( '/A:B/WebPreferences', $Foswiki::cfg{UsersWebName},
'WebPreferences' );
$this->urltest( '/A\'":B/WebPreferences', $Foswiki::cfg{UsersWebName},
'WebPreferences' );

#invalid topic name
$this->urltest( '/System/WebPre@ferences', 'System', 'WebHome' );
Expand Down
290 changes: 213 additions & 77 deletions core/lib/Foswiki.pm
View file
Expand Up @@ -1754,6 +1754,187 @@ sub load_package {

=begin TML
---++ Private _parsePath( $this, $webtopic, $defaultweb, $topicOverride )
Parses the Web/Topic path parameters to safely establish a valid web and topic,
or assign safe defaults.
* $webtopic - A "web/topic" path. It might have originated from the query path_info,
or from the topic= URL parameter. (only when the topic param contained a web component)
* $defaultweb - The default web to use if the web part of webtopic is missing or invalid.
This can be from the default UsersWebName, or from the url parameter defaultweb.
* $topicOverride - A topic name to use instead of any topic provided in the pathinfo.
Note if $webtopic ends with a trailing slash, it provides a hint that the last component should be
considered web. This allows disambiguation between a topic and subweb with the same name.
Trailing slash forces it to be recognized as a webname, otherwise the topic is shown.
Note. If the web doesn't exist, the force will be ignored. It's not possible to create a missing web
by referencing it in a URL.
This routine sets two variables when encountering invalid input:
* $this->{invalidWeb} contains original invalid web / pathinfo content when validation fails.
* $this->{invalidTopic} Same function but for topic name
When invalid / illegal characters are encountered, the session {webName} and {topicName} will be
defaulted to safe defaults. Scripts using those fields should also test if the corresponding
invalid* versions are defined, and should throw an oops exception rathern than allowing execution
to proceed with defaulted values.
The topic name will always have the first character converted to upper case, to prevent creation of
or access to invalid topics.
=cut

sub _parsePath {
my $this = shift;
my $webtopic = shift;
my $defaultweb = shift;
my $topicOverride = shift;

#print STDERR "_parsePath called WT ($webtopic) DEF ($defaultweb)\n";

my $trailingSlash = ( $webtopic =~ s/\/$// );

#print STDERR "TRAILING = $trailingSlash\n";

# Remove any leading slashes or dots.
$webtopic =~ s/^[\/.]+//;

my @parts = split /[\/.]+/, $webtopic;
my $cur = 0;
my @webs; # Collect valid webs from path
my @badpath; # Collect all webs, including illegal
my $temptopic; # Candidate topicname extracted from path, defaults.

foreach (@parts) {

# Lax check on name to elimnate evil characters.
my $p = Foswiki::Sandbox::untaint( $_,
\&Foswiki::Sandbox::validateTopicName );
unless ($p) {
push @badpath, $_;
next;
}

if ( \$_ == \$parts[-1] ) { # This is the last part of path

if ( $this->topicExists( join( '/', @webs ) || $defaultweb, $p )
&& !$trailingSlash )
{

#print STDERR "Exists and no trailing slash\n";

# It exists in Store as a topic and there is no trailing slash
$temptopic = $p || '';
}
elsif ( $this->webExists( join( '/', @webs, $p ) ) ) {

#print STDERR "Web Exists " . join( '/', @webs, $p ) . "\n";

# It exists in Store as a web
push @badpath, $p;
push @webs, $p;
}
elsif ($trailingSlash) {

#print STDERR "Web forced ...\n";
if ( !$this->webExists( join( '/', @webs, $p ) )
&& $this->topicExists( join( '/', @webs ) || $defaultweb,
$p ) )
{

#print STDERR "Forced, but no such web, and topic exists";
$temptopic = $p;
}
else {

#print STDERR "Append it to the webs\n";
$p = Foswiki::Sandbox::untaint( $p,
\&Foswiki::Sandbox::validateWebName );

unless ($p) {
push @badpath, $_;
next;
}
else {
push @badpath, $p;
push @webs, $p;
}
}
}
else {
#print STDERR "Just a topic. " . scalar @webs . "\n";
$temptopic = $p;
}
}
else {
$p = Foswiki::Sandbox::untaint( $p,
\&Foswiki::Sandbox::validateWebName );
unless ($p) {
push @badpath, $_;
next;
}
else {
push @badpath, $p;
push @webs, $p;
}
}
}

my $web = join( '/', @webs );
my $badweb = join( '/', @badpath );

# Set the requestedWebName before applying defaults - used by statistics
# generation. Note: This is validated using Topic name rules to permit
# names beginning with lower case.
$this->{requestedWebName} =
Foswiki::Sandbox::untaint( $badweb,
\&Foswiki::Sandbox::validateTopicName );

#print STDERR "Set requestedWebName to $this->{requestedWebName} \n"
# if $this->{requestedWebName};

if ( length($web) != length($badweb) ) {

#print STDERR "RESULTS:\nPATH: $web\nBAD: $badweb\n";
$this->{invalidWeb} = $badweb;
}

unless ($web) {
$web = Foswiki::Sandbox::untaint( $defaultweb,
\&Foswiki::Sandbox::validateWebName );
unless ($web) {
$this->{invalidWeb} = $defaultweb;
$web = $Foswiki::cfg{UsersWebName};
}
}

# Override topicname if urlparam $topic is provided.
$temptopic = $topicOverride if ($topicOverride);

# Provide a default topic if none specified
$temptopic = $Foswiki::cfg{HomeTopicName} unless defined($temptopic);

# Item3270 - here's the appropriate place to enforce spec
# http://develop.twiki.org/~twiki4/cgi-bin/view/Bugs/Item3270
my $topic =
Foswiki::Sandbox::untaint( ucfirst($temptopic),
\&Foswiki::Sandbox::validateTopicName );

unless ($topic) {
$this->{invalidTopic} = $temptopic;
$topic = $Foswiki::cfg{HomeTopicName};

#print STDERR "RESULTS:\nTOPIC $topic\nBAD: $temptopic\n";
$this->{invalidTopic} = $temptopic;
}

#print STDERR "PARSE returns web $web topic $topic\n";

return ( $web, $topic );
}

=begin TML
---++ ClassMethod new( $defaultUser, $query, \%initialContext )
Constructs a new Foswiki session object. A unique session object exists for
Expand Down Expand Up @@ -1996,105 +2177,54 @@ sub new {
$this->{scriptUrlPath} = $1;
}

my $web = '';
my $topic = '';
# The web/topic can be provided by either the query path_info, or by URL Parameters
# urlparams topic: Specifies web.topic or topic. Overrides the path given in the URL
# defaultweb: Overrides the default web, for use when topic= does not provide a web.
# path_info web/topic Always provdes a web, if topic not provided, defaults to the Home topic

# Set the default for web if specified
# Set the default for web
# Development.AddWebParamToAllCgiScripts: enables
# bin/script?topic=WebPreferences;defaultweb=Sandbox
$web = $query->param('defaultweb') || '';
my $defaultweb = $query->param('defaultweb') || $Foswiki::cfg{UsersWebName};

#if the user has only asked for the web portion, then we can make a better guess (view/One/Two/Three/ is just a web (trailing /))
my $topicSpecified = 0;

#look at the url path
my $pathInfo = $query->path_info();
my $webtopic = $query->path_info() || '';
my $topicOverride = '';
if ( $query->param('topic') ) {
if ( $query->param('topic') =~ m/[\/.]+/ ) {
$webtopic = $query->param('topic');

# Truncate the path_info at the first quote
if ( $pathInfo =~ m/['"]/g ) {
$pathInfo = substr( $pathInfo, 0, ( ( pos $pathInfo ) - 1 ) );
}
if ( $pathInfo && $pathInfo =~ m|^/+(.+)| ) {
$pathInfo = $1;
if ( $pathInfo =~ m|[./]| ) {
( $web, $topic ) = $this->normalizeWebTopicName( $web, $pathInfo );
$topicSpecified = ( $pathInfo =~ m/$topic$/ );
#print STDERR "candidate webtopic set to $webtopic by query param\n";
}
else {
$web = $pathInfo;
}
}
$topicOverride = $query->param('topic');

# Set the requestedWebName before applying defaults - used by statistics
# generation. Note: This is validated using Topic name rules to permit
# names beginning with lower case.
$this->{requestedWebName} =
Foswiki::Sandbox::untaint( $web, \&Foswiki::Sandbox::validateTopicName );

#non-'' ?topic
if ( $query->param('topic') ) {
$topic = $query->param('topic');
$topicSpecified = 1;
#print STDERR
# "candidate topic set to $topicOverride by query param\n";
}
}

( $web, $topic ) = $this->normalizeWebTopicName( $web, $topic );
$web =~ s|/*$||;
# SMELL Scripts like rest, jsonrpc, don't use web/topic path. so this ends up all bogus
# but doesn't do any harm.

my $topicNameTemp = $this->UTF82SiteCharSet($topic);
if ($topicNameTemp) {
$topic = $topicNameTemp;
}

# Item3270 - here's the appropriate place to enforce spec
# http://develop.twiki.org/~twiki4/cgi-bin/view/Bugs/Item3270
$topic = ucfirst($topic);

# Validate and untaint topic name from path info
$topic =
Foswiki::Sandbox::untaint( $topic, \&Foswiki::Sandbox::validateTopicName )
|| $Foswiki::cfg{HomeTopicName};

# Set the default for web
#
# SMELL: This appears to be useless. normalizeWebTopicName will always assign a default
# By now $web is always set.
#
$web ||= $Foswiki::cfg{UsersWebName};

# Validate web name from path info or default
$web =
Foswiki::Sandbox::untaint( $web, \&Foswiki::Sandbox::validateWebName )
|| '';
my ( $web, $topic ) =
$this->_parsePath( $webtopic, $defaultweb, $topicOverride );

# Convert UTF-8 web and topic name from URL into site charset if
# necessary
# SMELL: merge these two cases, browsers just don't mix two encodings
# in one URL - can also simplify into 2 lines by making function
# return unprocessed text if no conversion
#
my $topicNameTemp = $this->UTF82SiteCharSet($topic);
if ($topicNameTemp) {
$topic = $topicNameTemp;
}

my $webNameTemp = $this->UTF82SiteCharSet($web);
if ($webNameTemp) {
$web = $webNameTemp;
}

#if the parsed per spec web&topic don't exist, try to help the user
if ($topicSpecified) {
if ( !$this->topicExists( $web, $topic )
&& $this->webExists( $web . '/' . $topic ) )
{

#requested view/Web/Sub - when there is no Web.Sub topic, but there is such a web
$web = $web . '/' . $topic;
$topic = $Foswiki::cfg{HomeTopicName};
}
}
else {
if ( !$this->webExists($web) ) {

#requested view/Web/Sub/ - when there is no Web.Sub web, but there is such a topic
my ( $w, $t ) = $this->normalizeWebTopicName( '', $web );
( $web, $topic ) = ( $w, $t ) if ( $this->topicExists( $w, $t ) );
}
}
$this->{topicName} = $topic;
$this->{webName} = $web;

Expand All @@ -2109,6 +2239,10 @@ sub new {
# SMELL: what happens if we move this into the Foswiki::Users::new?
$this->{user} = $this->{users}->initialiseUser( $this->{remoteUser} );

# SMELL: Initializing the user overrides the web/topic for some reason
$this->{topicName} = $topic;
$this->{webName} = $web;

#Monitor::MARK("Initialised user");

# Static session variables that can be expanded in topics when they
Expand Down Expand Up @@ -2384,6 +2518,8 @@ sub finish {
undef $this->{topic};
undef $this->{webName};
undef $this->{topicName};
undef $this->{invalidWeb};
undef $this->{invalidTopic};
undef $this->{_ICONSPACE};
undef $this->{_EXT2ICON};
undef $this->{_KNOWNICON};
Expand Down

0 comments on commit 9838f11

Please sign in to comment.