Skip to content

Commit

Permalink
Item12560: Merge SplitTopicAttachmentNameFilters
Browse files Browse the repository at this point in the history
This feature makes several changes
 - Adds the AttachmentNameFilter
 - Adds a colon to the NameFilter
 - Removes colon and space from the AttachmentNameFilter
 - Adds new setting flag {AttachmentReplaceSpaces}, which restores the
   old behavior of replacing spaces with underscores in attachment
   names.
  • Loading branch information
gac410 committed Dec 2, 2015
2 parents 356dc8d + d9744f1 commit cc396ef
Show file tree
Hide file tree
Showing 9 changed files with 178 additions and 30 deletions.
42 changes: 42 additions & 0 deletions InterwikiPlugin/test/unit/InterwikiPlugin/InterwikiPluginTests.pm
Expand Up @@ -19,10 +19,15 @@ sub set_up {

$this->SUPER::set_up();
$this->{test_user} = 'scum';

$this->{other_web} = "$this->{test_web}other";
my $webObject = $this->populateNewWeb( $this->{other_web} );
$webObject->finish();
}

sub tear_down {
my $this = shift;
$this->removeWebFixture( $this->{session}, $this->{other_web} );
$this->SUPER::tear_down();
}

Expand Down Expand Up @@ -483,4 +488,41 @@ sub test_link_in_parentheses {
);
}

sub test_link_from_include {
my $this = shift;
my $otherTopic = "IncludedTopic";

# This test will fail if colon is removed from $Foswiki::cfg{NameFilter}

Foswiki::Func::saveTopic( $this->{other_web}, $otherTopic, undef, <<'HERE');
---+!! InterwikiInclBase
%STARTSECTION{"Exponential growth"}%
<blockquote>
Exponential growth occurs when the growth rate
of the value of a mathematical function
is proportional to the function's current value.
-- [[Wikipedia:Exponential_growth][Exponential growth]] (WP)
</blockquote>
%ENDSECTION{"Exponential growth"}%
HERE

Foswiki::Plugins::InterwikiPlugin::initPlugin(
$this->{test_web}, $this->{test_topic},
$this->{test_user}, $Foswiki::cfg{SystemWebName}
);

$this->assert_html_matches(
qq{<a class="interwikiLink" href="http://en.wikipedia.org/wiki/Exponential_growth" title="'Exponential_growth' on 'Wikipedia'">},
Foswiki::Func::renderText(
Foswiki::Func::expandCommonVariables(
"%INCLUDE{\"$this->{other_web}.$otherTopic\" section=\"Exponential growth\"}%"
),
$this->{test_web}
)
);
}

1;
6 changes: 6 additions & 0 deletions JQueryPlugin/lib/Foswiki/Plugins/JQueryPlugin/FOSWIKI.pm
Expand Up @@ -76,6 +76,12 @@ sub init {
$Foswiki::cfg{NameFilter} );
}

# init ATTACHMENTNAMEFILTER
unless ( Foswiki::Func::getPreferencesValue('ATTACHMENTNAMEFILTER') ) {
Foswiki::Func::setPreferencesValue( 'ATTACHMENTNAMEFILTER',
$Foswiki::cfg{AttachmentNameFilter} );
}

# add exported preferences to head
my %prefs = ();
foreach my $pref ( split( /\s*,\s*/, $prefs ) ) {
Expand Down
2 changes: 1 addition & 1 deletion UnitTestContrib/test/unit/FormattingTests.pm
Expand Up @@ -1379,7 +1379,7 @@ sub test_internalLinkSpacedText_Item8713 {
my $editURI = $this->{session}->getScriptUrl( 0, 'edit' );

my $expected = <<EXPECTED;
<a class="foswikiNewLink" href="$editURI/DiscussWiki:PhilosophyVs/Technology?topicparent=TemporaryFormattingTestWebFormatting.TestTopicFormatting" rel="nofollow" title="Create this topic">discuss 'wiki': philosophy vs. technology</a>
<a class="foswikiNewLink" href="$editURI/DiscussWikiPhilosophyVs/Technology?topicparent=TemporaryFormattingTestWebFormatting.TestTopicFormatting" rel="nofollow" title="Create this topic">discuss 'wiki': philosophy vs. technology</a>
EXPECTED

my $actual = <<ACTUAL;
Expand Down
50 changes: 35 additions & 15 deletions UnitTestContrib/test/unit/RobustnessTests.pm
Expand Up @@ -134,7 +134,7 @@ sub test_sanitizeAttachmentName {
my $crap = '';
for ( 0 .. 255 ) {
my $c = chr($_);
$crap .= $c if $c =~ m/$Foswiki::cfg{NameFilter}/;
$crap .= $c if $c =~ m/$Foswiki::cfg{AttachmentNameFilter}/;
}

my $hex = '';
Expand All @@ -144,12 +144,13 @@ sub test_sanitizeAttachmentName {
}

my $expecthex =
'\x00\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f "#$%&\'*;<>?@[\]^`|~';
'\x00\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f"#$%&\'*;<>?@[\]^`|~';
$this->assert_str_equals( $expecthex, $hex,
"Expected: ($expecthex)\n Got: ($hex)\nHas {NameFilter} changed?" );
$this->assert_num_equals( 52, length($crap) );
"Expected: ($expecthex)\n Got: ($hex)\nHas {AttachmentNameFilter} changed?"
);
$this->assert_num_equals( 51, length($crap) );
my $x = $crap =~ m/ / ? '_' : '';
$this->assert_str_equals( "pick_me${x}pick_me",
$this->assert_str_equals( "pick me${x}pick me",
_sanitize("pick me${crap}pick me") );
my %junkset = (
'<script>' => 'script',
Expand All @@ -162,7 +163,7 @@ sub test_sanitizeAttachmentName {
"foo\x1ffoo" => 'foofoo', # C0 Control
"\xe2cret\xe9" => "\xe2cret\xe9", # cf. acrete - 'âcreté'
'片仮名' => '片仮名',
'var a = { b : !(1 - 2 + 3) };' => 'var_a_=_{_b_:_!(1_-_2_+_3)_}',
'var a = { b : !(1 - 2 + 3) };' => 'var a = { b : !(1 - 2 + 3) }',

#'var a = { b : !(1 - 2 + 3) };' => 'var_a___b_:_1__2__3_',
#"foo\x7ffoo" => 'foofoo', # C1 Control
Expand All @@ -174,30 +175,49 @@ sub test_sanitizeAttachmentName {
}

# Check that the upload filter is applied.
# SMELL: Keep this regex in sync with the default regex in Foswiki.spec
$Foswiki::cfg{UploadFilter} = qr(^(
\.htaccess
| .*\.(?i)(?:php[0-9s]?(\..*)?
| [sp]htm[l]?(\..*)?
| pl
| py
| cgi ))$)x;
(?i)\.htaccess # .htaccess needs to be case insensitive
| .*\.(?i) # Case insensitive
(?:php[0-9s]?(\..*)? # PHP files can have a suffix
| [sp]?htm[l]?(\..*)? # html, shtml, phtml, htm, shtm, phtm
| pl # Perl
| py # Python
| cgi # CGI Scripts
)?)$)x;

# Case sensitive, typical on Linux
$this->assert_str_equals( ".htaccess.txt", _sanitize(".htaccess") );
for my $i (qw(php shtm phtml pl py cgi PHP SHTM PHTML PL PY CGI)) {

# Case insensitive (windows)
$this->assert_str_equals( ".HTacceSS.txt", _sanitize(".HTacceSS") );

# Trailing dot ignored on windows
$this->assert_str_equals( ".HTacceSS..txt", _sanitize(".HTacceSS.") );

for my $i (qw(php shtm phtml html pl py cgi PHP SHTM PHTML PL PY CGI)) {
my $j = "bog.$i";
my $y = "$j.txt";
$this->assert_str_equals( $y, _sanitize($j) );
}
for my $i (qw(php phtm shtml PHP PHTM SHTML)) {
for my $i (qw(php php0 phtm shtml html PHP PHP0 PHTM SHTML)) {
my $j = "bog.$i.s";
my $y = "$j.txt";
$this->assert_str_equals( $y, _sanitize($j) );
}

# Trailing dots also get renamed
for my $i (qw(php phtm shtml html PHP py pl cgi PHTM SHTML)) {
my $j = "bog.$i.";
my $y = "bog.$i..txt";
$this->assert_str_equals( $y, _sanitize($j) );
}

return;
}

# Item11185 - see also: FuncTests::test_unicode_attachment
sub test_sanitizeAttachmentNama_unicode {
sub test_sanitizeAttachmentName_unicode {
my ($this) = shift;

# The second word in the string below consists only of two _graphemes_
Expand Down
67 changes: 64 additions & 3 deletions UnitTestContrib/test/unit/UploadScriptTests.pm
Expand Up @@ -127,6 +127,67 @@ sub test_simple_upload {
return;
}

sub test_space_filename {
my $this = shift;
local $/ = undef;
my $result;

# Try the upload with ReplaceSpaces enabled (old Foswiki 1.x/2.0 behaviour)
# It should oops. but will still attach the file.
$Foswiki::cfg{AttachmentReplaceSpaces} = 1;
try {
$result = $this->do_upload(
'Flappa doodle.txt',
"BLAH",
undef,
hidefile => 0,
filecomment => 'Elucidate the goose',
createlink => 0,
changeproperties => 0,
);
}
catch Foswiki::OopsException with {
my $e = shift;
$this->assert_str_equals( "upload_name_changed", $e->{def} );
};

# Try the upload again, without ReplaceSpaces enabled, (new behaviour)
# It should work, without any oops.
$Foswiki::cfg{AttachmentReplaceSpaces} = 0;
$result = $this->do_upload(
'Flappa doodle.txt',
"BLAH",
undef,
hidefile => 0,
filecomment => 'Stuff the goose',
createlink => 0,
changeproperties => 0,
);

$this->assert_matches( qr/^Status: 302/ms, $result );
$this->assert(
open(
my $F,
'<',
"$Foswiki::cfg{PubDir}/$this->{test_web}/$this->{test_topic}/Flappa doodle.txt"
)
);
$this->assert_str_equals( "BLAH", <$F> );
$this->assert( close($F) );
my ( $meta, $text ) =
Foswiki::Func::readTopic( $this->{test_web}, $this->{test_topic} );

# Check the meta
my $at = $meta->get( 'FILEATTACHMENT', 'Flappa doodle.txt' );
$this->assert($at);
$this->assert_str_equals( 'Stuff the goose', $at->{comment} );

$at = $meta->get( 'FILEATTACHMENT', 'Flappa_doodle.txt' );
$this->assert($at);
$this->assert_str_equals( 'Elucidate the goose', $at->{comment} );
return;
}

sub test_noredirect_param {
my $this = shift;
local $/ = undef;
Expand Down Expand Up @@ -563,7 +624,7 @@ sub test_imagelink {
$this->assert( open( my $FILE, '<', $imageFile ) );
my $data = do { local $/ = undef; <$FILE> };
$this->assert( close($FILE) );
my $filename = 'bomb.png';
my $filename = 'bo:mb.png';
$filename = Assert::TAINT($filename);
my $result = $this->do_upload(
$filename,
Expand Down Expand Up @@ -591,12 +652,12 @@ sub test_imagelink {

# Check the link was created
$this->assert_matches(
qr/<img src=\"%ATTACHURLPATH%\/bomb.png\" alt=\"bomb.png\" width=\'16\' height=\'16\' \/>/,
qr/<img src=\"%ATTACHURLPATH%\/bo:mb.png\" alt=\"bo:mb.png\" width=\'16\' height=\'16\' \/>/,
$text
);

# Check the meta
my $at = $meta->get( 'FILEATTACHMENT', 'bomb.png' );
my $at = $meta->get( 'FILEATTACHMENT', 'bo:mb.png' );
$this->assert($at);
$this->assert_matches( qr/h/i, $at->{attr} );
$this->assert_str_equals( 'Educate the hedgehog', $at->{comment} );
Expand Down
2 changes: 1 addition & 1 deletion core/lib/Foswiki.pm
Expand Up @@ -559,7 +559,7 @@ qr(AERO|ARPA|ASIA|BIZ|CAT|COM|COOP|EDU|GOV|INFO|INT|JOBS|MIL|MOBI|MUSEUM|NAME|NE
# See RobustnessTests::test_sanitizeAttachmentName
#
# Actually, this is used in GenPDFPrincePlugin; let's copy NameFilter
$regex{filenameInvalidCharRegex} = qr/$Foswiki::cfg{NameFilter}/;
$regex{filenameInvalidCharRegex} = qr/$Foswiki::cfg{AttachmentNameFilter}/;

# Multi-character alpha-based regexes
$regex{mixedAlphaNumRegex} = qr/[[:alnum:]]*/;
Expand Down
18 changes: 15 additions & 3 deletions core/lib/Foswiki.spec
Expand Up @@ -870,6 +870,7 @@ $Foswiki::cfg{AccessibleCFG} = [
'{AntiSpam}{EntityEncode}',
'{AntiSpam}{HideUserDetails}',
'{AntiSpam}{RobotsAreWelcome}',
'{AttachmentNameFilter}',
'{AuthRealm}',
'{AuthScripts}',
'{Cache}{Enabled}',
Expand Down Expand Up @@ -973,14 +974,25 @@ $Foswiki::cfg{DenyDotDotInclude} = $TRUE;
# files that were already uploaded, or files that were created directly
# on the server.
#
$Foswiki::cfg{UploadFilter} = '^((?i)\.htaccess|.*\.(?i)(?:php[0-9s]?(\..*)?|[sp]htm[l]?(\..*)?|pl|py|cgi)?)$';
$Foswiki::cfg{UploadFilter} = '^((?i)\.htaccess|.*\.(?i)(?:php[0-9s]?(\..*)?|[sp]?htm[l]?(\..*)?|pl|py|cgi)?)$';

# **REGEX LABEL="Name Filter" EXPERT**
# Filter-out regex for webnames, topic names, file attachment names, usernames,
# Filter-out regex for web names, topic names, usernames,
# include paths and skin names. This is a filter *out*, so if any of the
# characters matched by this expression are seen in names, they will be
# removed.
$Foswiki::cfg{NameFilter} = '[\\\\\\s\\*?~^\\$@%`"\'\\x26;|\\x3c>\\[\\]#\\x00-\\x1f]';
$Foswiki::cfg{NameFilter} = '[\\\\\\s\\*?~^\\$@%`"\'\\x26;:|\\x3c>\\[\\]#\\x00-\\x1f]';

# **REGEX LABEL="Attachment Name Filter" EXPERT**
# Filter-out regex file attachment names. This is a filter *out*, so if any of the
# characters matched by this expression are seen in an attachment name, they will be
# removed.
$Foswiki::cfg{AttachmentNameFilter} = '[\\\\\\*?~^\\$@%`"\'\\x26;|\\x3c>\\[\\]#\\x00-\\x1f]';

# **BOOLEAN LABEL="Replace Attachment Spaces" EXPERT**
# Enable this parameter if you want the old behavior of replacing spaces in an attachment filename
# with underscores.
$Foswiki::cfg{AttachmentReplaceSpaces} = $FALSE;

# **BOOLEAN LABEL="Force unsafe Regular Expressions" EXPERT**
# If this is set, then the search module will use more relaxed
Expand Down
9 changes: 8 additions & 1 deletion core/lib/Foswiki/Macros/INCLUDE.pm
Expand Up @@ -89,7 +89,14 @@ m#^($Foswiki::regex{webNameRegex}\.|$Foswiki::regex{defaultWebNameRegex}\.|$Fosw

# If link is only an anchor, leave it as is (Foswikitask:Item771)
return "[[$link][$label]]" if $link =~ m/^#/;
return "[[$web.$link][$label]]";

if ( Foswiki::isValidTopicName( $link, 1 ) ) {
return "[[$web.$link][$label]]";
}
else {
# Just leave it alone, not a valid topic, might be a Interwiki link.
return "[[$link][$label]]";
}
}

# generate an include warning
Expand Down
12 changes: 6 additions & 6 deletions core/lib/Foswiki/Sandbox.pm
Expand Up @@ -182,7 +182,7 @@ standard Web/Topic/attachment level e.g
Web/Topic/attachmentdir/subdir/attachment.gif. While such attachments cannot
be created via the UI, they *can* be created manually on the server.
The individual path components are filtered by $Foswiki::cfg{NameFilter}
The individual path components are filtered by $Foswiki::cfg{AttachmentNameFilter}
=cut

Expand Down Expand Up @@ -216,7 +216,7 @@ sub validateAttachmentName {
else {

# Filter nasty characters
$component =~ s/$Foswiki::cfg{NameFilter}//g;
$component =~ s/$Foswiki::cfg{AttachmentNameFilter}//g;
push( @result, $component );
}
}
Expand All @@ -240,7 +240,7 @@ sub _cleanUpFilePath {
if ( $component eq '..' ) {
throw Error::Simple( 'relative path in filename ' . $string );
}
elsif ( $component =~ m/$Foswiki::cfg{NameFilter}/ ) {
elsif ( $component =~ m/$Foswiki::cfg{AttachmentNameFilter}/ ) {
throw Error::Simple( 'illegal characters in file name component "'
. $component
. '" of filename '
Expand All @@ -266,7 +266,7 @@ sub _cleanUpFilePath {
---++ StaticMethod normalizeFileName( $string ) -> $filename
Throws an exception if =$string= contains filtered characters, as
defined by =$Foswiki::cfg{NameFilter}=
defined by =$Foswiki::cfg{AttachmentNameFilter}=
The returned string is not tainted, but it may contain shell
metacharacters and even control characters.
Expand Down Expand Up @@ -337,11 +337,11 @@ sub sanitizeAttachmentName {
}

# Change spaces to underscore
$fileName =~ s/ /_/g;
$fileName =~ s/ /_/g if ( $Foswiki::cfg{AttachmentReplaceSpaces} );

# See Foswiki.pm filenameInvalidCharRegex definition and/or Item11185
#$fileName =~ s/$Foswiki::regex{filenameInvalidCharRegex}//g;
$fileName =~ s/$Foswiki::cfg{NameFilter}//g;
$fileName =~ s/$Foswiki::cfg{AttachmentNameFilter}//g;

# Append .txt to some files
$fileName =~ s/$Foswiki::cfg{UploadFilter}/$1\.txt/g;
Expand Down

0 comments on commit cc396ef

Please sign in to comment.