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

Item13880: TML rendered in head and script zones causes malformed html in WebCreateNewTopic

pencil
Priority: Urgent
Current State: Closed
Released In: 2.1.0
Target Release: minor
Applies To: Engine
Component: FoswikiRender
Branches: Release02x00 master Item13897
Reported By: LynnwoodBrown
Waiting For:
Last Change By: GeorgeClark
The "js" section of System. contains this code:
%ADDTOZONE{"head" 
   tag="WebCreateNewTopicTemplate:META" 
   text="<noautolink><meta name='foswiki.webTopicCreator.nameFeedback' content='%MAKETEXT{"Topic will be named: "}%' />
<meta name='foswiki.webTopicCreator.errorFeedbackNoWikiName' content='<p class=\"foswikiGrayText\">%MAKETEXT{"Enter the topic name as WikiWord or check the allow non-Wiki Word box."}%</p>' />"
   requires=""
}%
..which is rendering as: (notice &lt; and &gt; )
<meta name='foswiki.webTopicCreator.errorFeedbackNoWikiName' content='&lt;p class="foswikiGrayText">Enter the topic name as WikiWord or check the allow non-Wiki Word box.</p>' /&gt;<!--WebCreateNewTopicTemplate:META-->

It is fixed by adding literal tag. (Also, I think the noautolink tag should be closed.)
%ADDTOZONE{"head" 
   tag="WebCreateNewTopicTemplate:META" 
   text="<noautolink><literal><meta name='foswiki.webTopicCreator.nameFeedback' content='%MAKETEXT{"Topic will be named: "}%' />
<meta name='foswiki.webTopicCreator.errorFeedbackNoWikiName' content='<p class=\"foswikiGrayText\">%MAKETEXT{"Enter the topic name as WikiWord or check the allow non-Wiki Word box."}%</p>' /></literal></noautolink>"
   requires=""
}%

Perhaps there's a better and simpler correction.

-- LynnwoodBrown - 02 Dec 2015

Actually found that UserRegistrationParts has a similar issue. The ID's in the meta are autolinking, in addition the <link> tag was malformed.

-- GeorgeClark - 12 Dec 2015

The underlying cause of this is that Foswiki::Render issues a takeOutBlocks for "script" tags, so that there is no TML expansion within the tags, but does not do the same for "meta" or "link" tags. The end result is that any "WikiWord" styled "id" or "class" will be expanded as a broken link. It is addressed piecemeal by addition of <noautolink> and/or <literal> into the ADDTOZONE for meta zones.

Note that this has a possibility for subtle breakage as well. Some Class ID's are all uppercase. In which case they only link if the corresponding Acronym topic exists. If someone were to create a topic that matched an existing class, you would end up with unexplained breakage.

I think that the fix to this is to treat meta, link and script blocks in similar fashion.

diff --git a/core/lib/Foswiki/Render.pm b/core/lib/Foswiki/Render.pm
index 1872268..cf7a644 100644
--- a/core/lib/Foswiki/Render.pm
+++ b/core/lib/Foswiki/Render.pm
@@ -247,6 +247,12 @@ sub getRenderedVersion {
       $this->_takeOutProtected( $text, qr/<script\b.*?<\/script>/si, 'script',
         $removed );
     $text =
+      $this->_takeOutProtected( $text, qr/<meta\b.*?\/?>/si, 'meta',
+        $removed );
+    $text =
+      $this->_takeOutProtected( $text, qr/<link\b.*?\/?>/si, 'link',
+        $removed );
+    $text =
       $this->_takeOutProtected( $text, qr/<style\b.*?<\/style>/si, 'style',
         $removed );
 
@@ -554,6 +560,8 @@ sub getRenderedVersion {
     $text =~ s|\n?<nop>\n$||;    # clean up clutch
 
     $this->_putBackProtected( \$text, 'style',  $removed );
+    $this->_putBackProtected( \$text, 'link', $removed );
+    $this->_putBackProtected( \$text, 'meta', $removed );
     $this->_putBackProtected( \$text, 'script', $removed );
     Foswiki::putBackBlocks( \$text, $removed, 'literal', '' );
     $this->_putBackProtected( \$text, 'literal', $removed );

-- GeorgeClark - 12 Dec 2015

And these case insensitive regexes are much much slower than using case sensitive with character classes: eg [Mm][Ee][Tt][Aa]

Perl 5.8.8:
     Rate      a      b      c
a  49.8/s     --  -100%  -100%          - Case insensitive match
b 13949/s 27932%     --    -1%          - Match for (:?script|SCRIPT)
c 14037/s 28109%     1%     --          - Match for [Ss][Cc][[Rr][Ii][Pp][Tt]
Perl 5.23.3, running the same 3 tests. Eliminating the /i modifier is still well worth it.
    Rate     a     c     b
a 88.7/s    --  -98%  -99%
c 3873/s 4264%    --  -45%
b 7028/s 7819%   81%    --

-- GeorgeClark - 12 Dec 2015

JanKrueger pointed out that my proposed regexes to protect <meta> tags were not going to work if the content contained html.

In looking at this further, meta and link tags are part of the head zone, and all of head is protected when the topic is rendered. So no TML expansion should be happening. The root cause is that Foswiki::Render::Zones::_renderZone() actually calls Foswiki::Render::renderTML() which makes the head exclusion in render completely moot.

What I don't know is why the code does this. Removing the call doesn't seem to break anything, in my limited testing. All that seems to be accomplished, is that renderTML removes pseudo-tags like <literal> and <noautolink>

-- GeorgeClark - 17 Dec 2015

Proposed patch
diff --git a/core/lib/Foswiki/Render/Zones.pm b/core/lib/Foswiki/Render/Zones.pm
index 485733a..8ffe210 100644
--- a/core/lib/Foswiki/Render/Zones.pm
+++ b/core/lib/Foswiki/Render/Zones.pm
@@ -277,7 +277,15 @@ sub _renderZone {
 
     # delay rendering the zone until now
     $result = $topicObject->expandMacros($result);
-    $result = $topicObject->renderTML($result);
+
+    # TML should not be rendered in the HEAD, which includes the head & script zones.
+    # Other zones will be rendered normally.
+    if ( $zone eq 'head' || $zone eq 'script' ) {
+        $result =~ s#</?(:?literal|noautolink|nop|verbatim)>##g;    # Clean up TML pseudo tags
+    }
+    else {
+        $result = $topicObject->renderTML($result);
+    }
 
     return $result;
 }

-- GeorgeClark - 17 Dec 2015

Using

<meta name='foswiki.webTopicCreator.nameFeedback' content='%MAKETEXT{"Topic will be named: "}%' />

is the wrong way to communicate translations to javascript. Instead use HTML5 data attributes as in

<div class="webTopicCreator" data-name-feedback="%MAKETEXT{"Topic will be named:"}%">
...
</div>

And in javascript someting like

<script>
jQuery(function($) {
  var defaults = {
      nameFeedback: "Topic will be named:"
  };

   $(".webTopicCreator").each(function() {
      var $this = $(this),
            opts = $.extend({}, defaults, $this.data());

...
   });
);
</script>

This seems to render the ADDTOZONE discussion bogus more or less ... as far as I was able to follow.

-- MichaelDaum - 18 Dec 2015

Right, but still. As RENDERZONE is written, it explicitly calls renderTML against the content of the head zone. Render is written to exclude the <head>...&lt/head> block. .... except that RENDERZONE doesn't include the tags. So it gets rendered. and breaks links that include TML, autolinking wikiwords, etc.

Anyway, I'm going to check in my fixes. head and script zone should not have TML markup expanded. That is consistent with the takeOutProtected() calls for head and script in Render.pm

-- GeorgeClark - 18 Dec 2015

The complete fix - to not render TML in the head or script zones, will only be in Foswiki 2.1. A workaround is included in Foswiki 2.0.4 (and Foswiki 1.1.11 if ever released) by surrounding the broken elements with literal and/or noautolink tags.

-- Main.GeorgeClark - 19 Dec 2015 - 15:21
 
Topic revision: r13 - 03 Feb 2016, GeorgeClark - This page was cached on 19 May 2017 - 17:17.

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