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

attribute values containing '0' not correctly parsed #17

Closed
adamtwo opened this issue Feb 3, 2014 · 5 comments
Closed

attribute values containing '0' not correctly parsed #17

adamtwo opened this issue Feb 3, 2014 · 5 comments
Assignees
Labels

Comments

@adamtwo
Copy link

adamtwo commented Feb 3, 2014

Line 9339 of asciidoctor.js seems to replace blanks with 0's in attributes string. Then line 93340 does the inverse using 0's to split into an array. The issue arises when attribute name or value contain 0. In such a case the code below incorrectly splits attribute name or value and inserts partial key or value in the target array.

If we have to do such an ugly hack I recommend using some more unique string than only '0' character.

Adam

attrs = attrs.$gsub($scope.REGEXP['$[]']("space_delim"), "\\10").$gsub($scope.REGEXP['$[]']("escaped_space"), "1");
          attrs = options['$[]=']("attributes", ($a = ($c = attrs.$split("0")).$inject, $a._p = (TMP_4 = function(accum, entry){var self = TMP_4._s || this, $a, k = nil, v = nil;if (accum == null) accum = nil;if (entry == null) entry = nil;
@mojavelinux
Copy link
Member

Aha! This is happening because Opal is interpreting the null character as a
0. I have a fix I've used for other control characters. I'll apply it to
the null character as well for the upcoming 1.5.0.preview.2 release of
Asciidoctor.

I believe Opal has corrected this parse issue in master, but until the
0.6.0 release, we'll use the workaround.

@adamtwo
Copy link
Author

adamtwo commented Feb 3, 2014

Great, thanks for looking into it.

@mojavelinux
Copy link
Member

I filed this issue as #883. I've also resolved it. Once I release preview2,
I'll update the Asciidoctor.js build, since some other changes are
necessary as well.

asciidoctor/asciidoctor#883

@mojavelinux
Copy link
Member

Resolved upstream by asciidoctor/asciidoctor#884. This will be reflected in the 1.5.0.preview.2 build of Asciidoctor.js.

@mojavelinux mojavelinux self-assigned this Feb 4, 2014
@ggrossetie
Copy link
Member

Fix in master (upgrade to 1.5.0.preview.5)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants