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

Clarify if escaped space is allowed in link destination #493

Closed
robinst opened this issue Aug 24, 2017 · 5 comments
Closed

Clarify if escaped space is allowed in link destination #493

robinst opened this issue Aug 24, 2017 · 5 comments

Comments

@robinst
Copy link
Contributor

robinst commented Aug 24, 2017

The spec for link destinations (without angle brackets) says:

a nonempty sequence of characters that does not include ASCII space or control characters, and includes parentheses only if (a) they are backslash-escaped or (b) they are part of a balanced pair of unescaped parentheses. (Implementations may impose limits on parentheses nesting to avoid performance issues, but at least three levels of nesting should be supported.)

It's not clear whether an escaped space is valid or not, as in:

[foo](\ )

Currently, the dingus produces:

<p><a href="%5C%20">foo</a></p>

Thoughts?

@mgeier
Copy link
Contributor

mgeier commented Aug 24, 2017

Spaces cannot be backslash-escaped, only punctuation.

The space in your example is not part of the "link destination", because this would not be allowed ("... does not include ASCII space ...").

The space is still allowed, because it's part of the inline link:

An inline link consists of a link text followed immediately by a left parenthesis (, optional whitespace, an optional link destination, an optional link title separated from the link destination by whitespace, optional whitespace, and a right parenthesis ).

IMHO, this is a bug in commonmark.js, other CommonMark parsers get it right: http://johnmacfarlane.net/babelmark2/?text=%5Bfoo%5D(%5C+).

@robinst
Copy link
Contributor Author

robinst commented Aug 24, 2017

Good point about being allowed in an inline link, so this is a slightly different test:

[foo](a\ b)

@mgeier
Copy link
Contributor

mgeier commented Aug 24, 2017

Interesting, now there are two implementations with a bug: commonmark.js and markdown-it: http://johnmacfarlane.net/babelmark2/?text=%5Bfoo%5D(a%5C+b).

I remember some discussions about allowing spaces in link destinations (which would un-break many links on Github, for example), but this here is another issue.

@jgm
Copy link
Member

jgm commented Aug 24, 2017 via email

robinst added a commit to commonmark/commonmark-java that referenced this issue Mar 26, 2018
There were two failing tests, related to:

* Backslash in link destinations (see commonmark/commonmark-spec#493)
* Tabs in ATX/Setext headers

Change the implementation to match the reference implementation.
@robinst
Copy link
Contributor Author

robinst commented Mar 26, 2018

Makes sense, thanks John. Updated commonmark-java.

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

Successfully merging a pull request may close this issue.

3 participants