-
Notifications
You must be signed in to change notification settings - Fork 56
Extend safeHref #58
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
base: master
Are you sure you want to change the base?
Extend safeHref #58
Changes from all commits
17c0926
0ce02d7
fa8f3e0
08cd418
c243da7
a390a73
59aa408
70f6dfa
df06920
21ebb43
1e05213
2e63d6b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,9 +3,9 @@ import PropTypes from 'prop-types' | |
import {BoxComponent} from './types/box-types' | ||
import {propTypes} from './enhancers' | ||
import enhanceProps from './enhance-props' | ||
import {extractAnchorProps, getUseSafeHref} from './utils/safeHref' | ||
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 }) => { | ||
// Convert the CSS props to class names (and inject the styles) | ||
const {className, enhancedProps: parsedProps} = enhanceProps(props) | ||
|
||
|
@@ -22,7 +22,16 @@ const Box: BoxComponent = ({ is = 'div', innerRef, children, allowUnsafeHref, .. | |
*/ | ||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: What's the motivation for explicitly marking the type here? What happens if you remove There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we opt to keep this as-is, I would suggest a space between |
||
href: parsedProps.href, | ||
rel: parsedProps.rel, | ||
} | ||
|
||
if (allowProtocols && allowProtocols.length > 0) { | ||
hrefData.allowProtocols = allowProtocols | ||
} | ||
|
||
const {safeHref, safeRel} = extractAnchorProps(hrefData) | ||
parsedProps.href = safeHref | ||
parsedProps.rel = safeRel | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,13 +3,22 @@ export interface URLInfo { | |
sameOrigin: boolean | ||
} | ||
|
||
export interface HrefData { | ||
href: string | ||
rel: string | ||
allowProtocols?: string[] | ||
} | ||
|
||
export interface SafeHrefConfigObj { | ||
enabled?: boolean | ||
origin?: string | ||
additionalProtocols?: string[] | ||
} | ||
|
||
const PROTOCOL_REGEX = /^[a-z]+:/ | ||
const ORIGIN_REGEX = /^(?:[a-z]+:?:)?(?:\/\/)?([^\/\?]+)/ | ||
const safeProtocols: string[] = ['http:', 'https:', 'mailto:', 'tel:'] | ||
let customProtocols:Array<string> = [] | ||
let useSafeHref = false | ||
let globalOrigin = typeof window !== 'undefined' ? window.location.origin : false | ||
|
||
|
@@ -21,40 +30,45 @@ export function configureSafeHref(configObject: SafeHrefConfigObj) { | |
if (configObject.origin) { | ||
globalOrigin = configObject.origin | ||
} | ||
|
||
if (configObject.additionalProtocols && configObject.additionalProtocols.length) { | ||
customProtocols.push(...configObject.additionalProtocols) | ||
} | ||
} | ||
|
||
export function getUseSafeHref(): boolean { | ||
return useSafeHref | ||
} | ||
|
||
export function getURLInfo(url: string): URLInfo { | ||
/** | ||
* An array of the safely allowed url protocols | ||
*/ | ||
const safeProtocols = ['http:', 'https:', 'mailto:', 'tel:', 'data:'] | ||
export function resetCustomProtocols() { | ||
customProtocols = [] | ||
} | ||
|
||
export function getURLInfo(url: string, allowProtocols: Array<string>): URLInfo { | ||
/** | ||
* - Find protocol of URL or set to 'relative' | ||
* - Find origin of URL | ||
* - Determine if sameOrigin | ||
* - Determine if protocol of URL is safe | ||
*/ | ||
const protocolResult = url.match(PROTOCOL_REGEX) | ||
const originResult = url.match(ORIGIN_REGEX) | ||
const cleanedUrl = url.trim() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You could also use |
||
const protocolResult = cleanedUrl.match(PROTOCOL_REGEX) | ||
const originResult = cleanedUrl.match(ORIGIN_REGEX) | ||
const urlProtocol = protocolResult ? protocolResult[0] : 'relative' | ||
let sameOrigin = urlProtocol === 'relative' | ||
if (!sameOrigin && globalOrigin) { | ||
sameOrigin = globalOrigin === (originResult && originResult[0]) | ||
} | ||
|
||
const isSafeProtocol = sameOrigin ? true : safeProtocols.includes(urlProtocol) | ||
const allowedProtocols = [...safeProtocols, ...customProtocols, ...allowProtocols] | ||
const isSafeProtocol = sameOrigin ? true : allowedProtocols.includes(urlProtocol) | ||
if (!isSafeProtocol) { | ||
/** | ||
* If the url is unsafe, put a error in the console, and return the URLInfo object | ||
* with the value of url being `undefined` | ||
*/ | ||
console.error( | ||
'📦 `href` passed to anchor tag is unsafe. Because of this, the `href` on the element was not set. Please review the safe href documentation if you have questions.', | ||
'📦 ui-box: `href` passed to anchor tag is unsafe. Because of this, the `href` on the element was not set. Please review the safe href documentation if you have questions.', | ||
'https://www.github.com/segmentio/ui-box' | ||
) | ||
return { | ||
|
@@ -67,16 +81,19 @@ export function getURLInfo(url: string): URLInfo { | |
* If the url is safe, return the url and origin | ||
*/ | ||
return { | ||
url, | ||
url: cleanedUrl, | ||
sameOrigin | ||
} | ||
} | ||
|
||
export function extractAnchorProps(href: string, rel: string) { | ||
export function extractAnchorProps(hrefData: HrefData) { | ||
const {href, rel} = hrefData | ||
const allowProtocols = hrefData.allowProtocols && hrefData.allowProtocols.length ? hrefData.allowProtocols : [] | ||
|
||
/** | ||
* Get url info and update href | ||
*/ | ||
const urlInfo = getURLInfo(href) | ||
const urlInfo = getURLInfo(href, allowProtocols) | ||
const safeHref = urlInfo.url | ||
|
||
/** | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,58 @@ | ||
import test from 'ava' | ||
import {extractAnchorProps, configureSafeHref, resetCustomProtocols} from '../../src/utils/safeHref' | ||
|
||
test('Allows safe protocols', t => { | ||
configureSafeHref({ | ||
enabled: true | ||
}) | ||
|
||
const {safeHref} = extractAnchorProps({ | ||
href: 'https://www.apple.com', | ||
rel: '' | ||
}) | ||
|
||
t.assert(safeHref === 'https://www.apple.com') | ||
}) | ||
|
||
test('Rejects unsafe protocols', t => { | ||
const {safeHref} = extractAnchorProps({ | ||
href: 'javascript:alert("hi")', | ||
rel: '' | ||
}) | ||
|
||
t.assert(safeHref === undefined) | ||
}) | ||
|
||
test('Rejects unsafe protocols with whitespace', t => { | ||
const {safeHref} = extractAnchorProps({ | ||
href: ' javascript:alert("hi")', | ||
rel: '' | ||
}) | ||
|
||
t.assert(safeHref === undefined) | ||
}) | ||
|
||
test('Allows custom protocol', t => { | ||
configureSafeHref({ | ||
additionalProtocols: ['data:'] | ||
}) | ||
|
||
const {safeHref} = extractAnchorProps({ | ||
href: 'data:text/html,<html><h1>Hi</h1></html>', | ||
rel: '' | ||
}) | ||
|
||
resetCustomProtocols() | ||
|
||
t.assert(safeHref === 'data:text/html,<html><h1>Hi</h1></html>') | ||
}) | ||
|
||
test('Allows individual level custom protocol', t => { | ||
const {safeHref} = extractAnchorProps({ | ||
href: 'data:text/html,<html><h1>Hi</h1></html>', | ||
rel: '', | ||
allowProtocols: ['data:'] | ||
}) | ||
|
||
t.assert(safeHref === 'data:text/html,<html><h1>Hi</h1></html>') | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to add these to the PropTypes as well (
allowUnsafeHref
andallowProtocols
)