Quote ampersend chars in XML text. Closes #263. #265

Merged
merged 2 commits into from Mar 29, 2013

Conversation

Projects
None yet
2 participants
Contributor

strk commented Mar 22, 2013

Includes testcase.

Sandro Santilli Quote ampersend chars in XML text. Closes #263.
Includes testcase.
acf94e5
Contributor

strk commented Mar 22, 2013

Note that the Travis failures are unrelated with this change. REF: #264

Sandro Santilli Quote all needed XML chars. See #263.
Includes testcase.
d6585d3
Contributor

strk commented Mar 27, 2013

@tmcw I've pushed a new commit quoting all chars, and extending the testcase accordingly

@tmcw tmcw commented on the diff Mar 27, 2013

lib/carto/tree/quoted.js
@@ -8,7 +8,11 @@ tree.Quoted.prototype = {
is: 'string',
toString: function(quotes) {
- var xmlvalue = this.value.replace(/\'/g, ''');
+ var xmlvalue = this.value.replace(/&/g, '&');
+ xmlvalue = xmlvalue.replace(/\'/g, ''');
+ xmlvalue = xmlvalue.replace(/\"/g, '"');
+ xmlvalue = xmlvalue.replace(/\</g, '&lt;');
+ xmlvalue = xmlvalue.replace(/\>/g, '&gt;');
@tmcw

tmcw Mar 27, 2013

Contributor

These should be chained:

var xmlvalue = this.value.replace(/&/g, '&amp;')
    .replace(/\"/g, '&quot;');

etc instead of creating 4 new temp strings.

@strk

strk Mar 27, 2013

Contributor

I belive Temporary strings would be created anyway, just unnamed.
If you want to avoid that I guess you can list all characters
in the pattern and provide a function that does a lookup to determine
the replacement. This should be done after the first pass escaping
the ampersend itself.

@tmcw tmcw added a commit that referenced this pull request Mar 29, 2013

@tmcw tmcw Merge pull request #265 from strk/master-quote-amp
Quote ampersend chars in XML text. Closes #263.
435452b

@tmcw tmcw merged commit 435452b into mapbox:master Mar 29, 2013

1 check failed

default The Travis build failed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment