Skip to content

text annotations: < in href params breaks href tag #2239

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

Closed
endymonium opened this issue Jan 9, 2018 · 1 comment
Closed

text annotations: < in href params breaks href tag #2239

endymonium opened this issue Jan 9, 2018 · 1 comment
Labels
bug something broken

Comments

@endymonium
Copy link

Hi,

I would like to show a link in chart, unfortunately that links query contains a '<', which leads to plotly not recognizing it as a link: https://jsfiddle.net/2krwamLv/1/

If I encode the '<' manually prior, plotly encodes it again. I looked at the code https://github.com/plotly/plotly.js/blob/master/src/lib/svg_text_utils.js#L214-L221 but I found no easy way around this.

Any ideas?

Thanks,
Jan

@alexcjohnson
Copy link
Collaborator

Thanks for the report @endymonium - you're correct, I don't see any workaround without altering the code. This is a tricky section of code, due to the risk of XSS, but seems to me there are two things we can do, either one of which would give you a workaround:

  1. Allow already-escaped href attributes to be used verbatim - perhaps this would be as easy as replacing this line nodeSpec.href = encodeURI(href); with nodeSpec.href = encodeURI(decodeURI(href)); since the decodeURI is a noop if the URI is already decoded, but would prevent double encoding.
  2. Be smarter about SPLIT_TAGS and ONE_TAG to allow < and > inside attribute strings, which does seem like it would match the permissiveness of browsers, at least as far as href is concerned. This we would need to be very careful about though, security-wise, as these characters could be dangerous in other attributes that we don't explicitly sanitize later.

Option 1 seems fairly straightforward, and would just need to be paired with a test or two in svg_text_utils_test.js of the kind of links this allows us to support. Feel like giving it a try?

@alexcjohnson alexcjohnson added the bug something broken label Jan 10, 2018
rmoestl added a commit that referenced this issue Mar 13, 2018
- Bug: an already escaped URI of an HTML link was encoded a second time
  by Plotly which led to a wrong URI.
rmoestl added a commit that referenced this issue Mar 13, 2018
- Bug: an already escaped URI of an HTML link was encoded a second time
  by Plotly which led to a wrong URI.
rmoestl added a commit that referenced this issue Mar 13, 2018
Handle HTML links with encoded URIs correctly in svg text labels #2239
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something broken
Projects
None yet
Development

No branches or pull requests

2 participants