Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EZP-27171: Fix escaping for custom tag attribute values #1289

Merged
merged 1 commit into from Jul 25, 2017

Conversation

7 participants
@pkamps
Copy link
Contributor

commented Mar 29, 2017

The ezxmltext datatype allows editors to embed custom tags. For example the 'factbox'. A custom tag may have input fields allowing the editor to type in text. For example 'Title' for the 'factbox'.

If you type in a " (double-quote) char into the custom tag field, it is getting encoded twice before the value gets stored in the database. That's problematic because the template that renders the custom tag is not receiving the " char but '"' instead.

Steps to reproduce the issue:

  • you have a xmltext attribute (like article body)
  • in this attribute you add a custom tag
  • the custom tag has an field and you add some HTML code into this field - something like "<iframe src="http://www.mugo.ca"></iframe>"
  • You save the content object (Send to publishing)
  • On the public site, the HTML for the custom tag is broken. It renders something like
    "<iframe src"http://www.mugo.ca&quot;></iframe>"

More details where the problem is:

When you hit 'Send for publishing', the HTML form will send all attribute values of the content object to the server. In case of the xmltext attributes, it sends an HTML string. In my test I added a custom tag 'factbox' - the HTML string looks similar to:

<div type="custom" class="ezoeItemCustomTag factbox" customattributes="title|&quot;page.html&quot;attribute_separationalign|right">
<p>factbox</p>
</div>

For the title field, I typed in "page.html" (with quotes). You can see that those double-quotes are escaped. That's good, otherwise it would break the HTML string (doube-quote in an HTML attribute value).

This HTML string is send to:
kernel/classes/datatypes/exzmltext/ezxmlinputparser.php

That class turns the HTML into an XML format. That XML string is getting stored in the database. In case of my example, that's what you get in the DB:

<paragraph xmlns:tmp="http://ez.no/namespaces/ezpublish3/temporary/">
    <custom name="factbox" custom:title="&amp;quot;page.html&amp;quot;" custom:align="right"/>
</paragraph>

Here you can see the problem: &amp;quot;

That " char gets encoded twice:

"         ->   &quot;
&quot;     -> $amp;quot;

There is no good reason for the double-encoding. In XML, you are allowed to have the string " as an attribute value.

The double encoding happens in
extension/ezoe/ezxmltext/handlers/input/ezoeinputparser.php Line: 274

$element->setAttribute( $qualifiedName, $value );

setAttribute is escaping the given $value - see http://stackoverflow.com/questions/7294134/php-xml-dom-unwanted-escaping-characters-i-cannot-write-qnot

I changed it to
$element->setAttribute( $qualifiedName, htmlspecialchars_decode( $value ) );

That will avoid the double-encoding and will render the correct value in the templates.

@brookinsconsulting

This comment has been minimized.

Copy link
Contributor

commented Mar 30, 2017

+1

@pkamps Excellent documentation. Do we have an issue ticket for this PR?

Cheers,
Brookins Consulting

@peterkeung

This comment has been minimized.

Copy link
Contributor

commented Mar 30, 2017

Issue has been created: https://jira.ez.no/browse/EZP-27171

@pkamps pkamps changed the title Fix escaping for custom tag attribute values EZP-27171: Fix escaping for custom tag attribute values Mar 30, 2017

@kmadejski
Copy link
Member

left a comment

Given example with iframe is maybe not really easy to understand, but proposed solution solves the problem.
The easiest example can be some title wrapped in quotation marks.

I've done few manual tests and everything works well.

@andrerom or @bdunogier please, confirm that solution is good.

@andrerom
Copy link
Member

left a comment

Review ping @glye

@glye

glye approved these changes May 23, 2017

@andrerom

This comment has been minimized.

Copy link
Member

commented May 23, 2017

@kmadejski Do you send this to QA? (github label here and move jira issue to ready for testing, ping me if your missing rights for any of that)

@kmadejski

This comment has been minimized.

Copy link
Member

commented May 23, 2017

@andrerom JIRA issue status changed, but I can not change label here on Github.

@adamwojs

This comment has been minimized.

@andrerom andrerom merged commit 57bf1d5 into ezsystems:master Jul 25, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.