Item12560: NameFilter should reject colon, conflicts with Interwiki links. Restructure filters to improve flexibility.
Priority: Enhancement
Current State: Closed
Released In: 2.1.0
Target Release: minor
I appear to have found a bug in the interaction between INCLUDE, Interwiki, and subwebs.
Consider the following pages
The InterwikiInclTest page contains three instances of a blockquote which used an Interwiki link:
- One is INCLUDEd from the InterwikiInclBase page in the current directory.
- One is INCLUDEd from the InterwikiInclBase page in the parent directory.
The first two instances work fine, but the third one fails.
--
RichMorin - 02 Aug 2013 [examples EDITED 03 Aug]
Try to simplify the set of installed plugins:
start from 1 plugin enabled (InterwikiPlugin) and work your way upwards.
--
MichaelDaum - 03 Aug 2013
That's certainly a reasonable approach to tracking down the problem.
However, as I have a workaround (and pages to write),
I can't guarantee when or if I'll follow this approach.
In the meanwhile, I discovered that I could add a subweb to the Sandbox (!),
so I was able to replicate the problem there.
Given that foswiki.org doesn't support massive numbers of plugins,
this should be a good start on
MichaelDaum's suggested approach.
--
RichMorin - 03 Aug 2013
The issue is caused by rendering order of
INCLUDE and the
InterwikiPlugin pre-rendering handler. During include processing,
[[Topic]]
links are changed to
[[Web.topic]]
links. This happens before the
InterwikiPlugin processes the links.
The problem is twofold.
- INCLUDE doesn't check that the topic name is valid before adding the web prefix.
- Even if it did, since the colon is valid in topic names, it would still prefix the Interwiki link with the web.
I'm not sure of a good solution. The following patch, combined with addition of the colon to the
$Foswiki::cfg{NameFilter}
fixes the issue. But since it changes
NameFilter
, that will potentially break linking for other users.
Even though the
InterwikiPlugin is a default plugin, we can't add checking for Interwiki links into the
INCLUDE handler.
diff --git a/core/lib/Foswiki/Macros/INCLUDE.pm b/core/lib/Foswiki/Macros/INCLUDE.pm
index 329bf66..8c64eff 100644
--- a/core/lib/Foswiki/Macros/INCLUDE.pm
+++ b/core/lib/Foswiki/Macros/INCLUDE.pm
@@ -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 =~ /^#/;
- return "[[$web.$link][$label]]";
+
+ if ( Foswiki::isValidTopicName($link, 1) ) {
+ print STDERR "INCLUDE is rewriting $link to $web.$link \n";
+ return "[[$web.$link][$label]]";
+ }
+ else {
+ return "[[$link][$label]]";
+ }
}
# generate an include warning
Any other ideas on how to fix this?
--
GeorgeClark - 31 Dec 2014
In experimenting a bit more, it's possible to create topics that are unreachable through normal linking. For example, it's possible to create a topic named "Wikipedia:SomeTopic" which can be accessed through a manual URL, but the Interwiki will always rewrite in-topic references to the Wikipedia site.
Given that, it might be cleaner to recommend adding : to the
NameFilter
when InterikPlugin is in use. and include it by default in 1.2. With that change, the above fix would be useful for new sites, and it would prevent creation of unreachable topics.
--
GeorgeClark - 31 Dec 2014
And has another side effect. NameFilter would also sanitize the : from attachment names.
--
GeorgeClark - 01 Jan 2015
The colon should be added to
NameFilter, IMHP.
--
CrawfordCurrie - 22 Jan 2015
I wonder if it's time to split the
NameFilter into an "AttachmentNameFilter" And if
Item2865 is to be believed, change to a filterIN instead of a filterOUT.
There is also some justification to support spaces in attachment names, which probably won't be supported in topic or web names. Just splitting to a separate filter could be done for 1.2.0, but changing to a filter OUT is probably too complex this late in the process.
--
GeorgeClark - 23 Jan 2015
Hm... I was sure we had a stalled request to support spaces in attachment names. I couldn't find it, but did find
RejectDontRenameBadFileUploads I guess if this is worth doing, it's worth doing twice. I completed an alternate implementation before discovering the proposal.
--
GeorgeClark - 23 Jan 2015
My proposed fix is almost a
FeatureProposal, but definitely not to the level of the 'RejectDontRename' proposal.
- Apply the proposed fix (above) to Include processing.
- Move Attachment filtering into
{AttachmentNameFilter}
. Remains a Filter-Out
- Add
{AttachmentNameSpaces}
config key to enable/disable the "rename spaces to underscore" behaviour.
- Add colon to the
{NameFilter}
and remove \s
from the {AttachmentNameFilter}
- Add some additional scrubbing for windows platforms.
- Announce deprecation of
Sandbox::sanitizeAttachmentName()
, replaced by equivalent Func
API. This positions us for larger changes in 2.0.
--
GeorgeClark - 23 Jan 2015
Completed except for deprecation of sanitizeAttachmentName.
--
GeorgeClark - 17 Dec 2015