Extend safeHref #58
Extend safeHref #58
Conversation
…ls to be added, and to overwrite and allow protocol from component
| import {extractAnchorProps, getUseSafeHref, HrefData} from './utils/safeHref' | ||
|
|
||
| const Box: BoxComponent = ({ is = 'div', innerRef, children, allowUnsafeHref, ...props }) => { | ||
| const Box: BoxComponent = ({ is = 'div', innerRef, children, allowUnsafeHref, allowProtocols, ...props }) => { |
There was a problem hiding this comment.
Need to add these to the PropTypes as well ( allowUnsafeHref and allowProtocols)
Co-Authored-By: Matt Shwery <mshwery@gmail.com>
Co-Authored-By: Matt Shwery <mshwery@gmail.com>
ucarion
left a comment
ucarion
left a comment
There was a problem hiding this comment.
Just some thoughts! Overall LGTM. :)
| const safeHrefEnabled = (typeof allowUnsafeHref === 'boolean' ? !allowUnsafeHref : getUseSafeHref()) && is === 'a' && parsedProps.href | ||
| if (safeHrefEnabled) { | ||
| const {safeHref, safeRel} = extractAnchorProps(parsedProps.href, parsedProps.rel) | ||
| const hrefData:HrefData = { |
There was a problem hiding this comment.
Nit: What's the motivation for explicitly marking the type here? What happens if you remove :HrefData?
There was a problem hiding this comment.
If we opt to keep this as-is, I would suggest a space between hrefData: and HrefData.
| */ | ||
| const protocolResult = url.match(PROTOCOL_REGEX) | ||
| const originResult = url.match(ORIGIN_REGEX) | ||
| const cleanedUrl = url.trim() |
There was a problem hiding this comment.
Might we worth putting a comment motivating why this is a sufficient "cleanup" step? For instance a link to:
https://html.spec.whatwg.org/multipage/links.html#links-created-by-a-and-area-elements
I can confirm that this is a sufficient cleanup, at least in Node:
> ["\u0009", "\u000A", "\u000C", "\u000D", "\u0020"].map(s => s.trim())
[ '', '', '', '', '' ]
There was a problem hiding this comment.
You could also use url.parse (https://nodejs.org/api/url.html#url_url_parse_urlstring_parsequerystring_slashesdenotehost) and then grab the pieces that you need.
An update to the safeHref functionality to better support our users
data:protocol from safe protocols to further protect usersconfigureSafeHrefmethodallowProtocolssafeHref