You are here: Foswiki>Tasks Web>Item12560 (03 Feb 2016, GeorgeClark)Edit Attach

Item12560: NameFilter should reject colon, conflicts with Interwiki links. Restructure filters to improve flexibility.

pencil
Priority: Enhancement
Current State: Closed
Released In: 2.1.0
Target Release: minor
Applies To: Engine
Component: InterwikiPlugin, FoswikiUIAttach
Branches: Item12560 master
Reported By: RichMorin
Waiting For:
Last Change By: GeorgeClark
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 explicitly coded.

  • 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.
  1. INCLUDE doesn't check that the topic name is valid before adding the web prefix.
  2. 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
 
Topic revision: r22 - 03 Feb 2016, GeorgeClark - This page was cached on 25 May 2017 - 11:06.

The copyright of the content on this website is held by the contributing authors, except where stated elsewhere. See Copyright Statement. Creative Commons License