mode / alamode

A community-maintained library of visualizations for Mode reports
MIT License
70 stars 67 forks source link

Handle url specials from cell data in addLinksToTables #32

Open pwildani opened 6 years ago

pwildani commented 6 years ago

This probably addresses https://github.com/mode/alamode/issues/31, but I encountered it independently.

When the cell contents includes URL-special characters, they should probably not be interpreted as multiple query parameters or other such errors.

For example, with a addLinksToTables({link_columns: ["col"], link_urls: ["http://modeanalytics?param_foo={{col}}"]}):

So this avoids data that is potentially sourced from external users getting injected into the super dangerous "securityhole" parameter on another report unintentionally.

Or more practically, when the data contains a +, it doesn't get re-interpreted as a space by the browser.

IMO, interpreting data from a single cell in a table as multiple parameters should be a special case that needs more coding. The simple case of expecting the cell contents to be a single scalar value should be the default.

leqilong commented 6 years ago

Hi @pwildani! Thanks for the PR. I think this code change assumes that the cell content will be the value of a query string. However, there are cases where the cell content are a URLs, like this report: https://modeanalytics.com/modeanalytics/reports/0bc75fadb03e/runs/d94183dc4088 In this case, the line encodeURIComponent(content) would encode the whole URL, which would make the link broken.

pwildani commented 6 years ago

Tricky. Handling that difference might need a real behavior controlling flag or a separate top level function.