Skip to content

Commit

Permalink
fixes URL validation method for cross-browser support
Browse files Browse the repository at this point in the history
In Firefox this will throw an Error:

```
new URL("https://www. example .com ")
```

In Chrome the same action will result in:

```
https://www.%20example%20.com
```

As a result the try/catch pattern used for validation rarely flagged
links on Chrome.

The approach in this commit uses a lenient regular expression to
validate URLs. A URL just needs a protocol, a domain, and optionally a
path. The domain and protocol have a limited character set.
  • Loading branch information
mattoberle committed Jan 13, 2021
1 parent 6c30b07 commit 77b60ae
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 15 deletions.
23 changes: 21 additions & 2 deletions lib/validators/built-in/invalid-links.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import _ from 'lodash';
import * as validUrl from 'valid-url';
import {
forEachComponent, forEachField, labelComponent, getPreviewText
} from '../helpers';
Expand All @@ -20,6 +19,26 @@ const COMPONENTS = [
*/
const isHash = (url) => (/^#/u).test(url);

/**
* Confirms that a URL loosely matches one of the following formats:
* - {protocol}://{domain}/*
* - mailto:{address}
* - #{anchor}
*
* Note: We cannot use a try/catch on 'new URL' here because some browsers
* will automatically encode spaces. Kiln prepends values with "https://" when
* an explicit protocol is not provided. These two behaviors combined result
* in almost *any* value getting coerced to something that will pass validation.
*
* @param {string} url: The href value of a link.
* @returns {boolean} true if link is valid URL, else false.
*/
const isValid = (url) => (
(/^\w+:\/\/[^\s/$.?#].[^\s]*$/u).test(url) // {protocol}://{domain}/*
|| (/^mailto:[^\s]+$/u).test(url) // mailto:{address}
|| (/^#[^\s+]/u).test(url) // #{anchor}
);

/**
* Collects all invalid links from the page state and returns
* an array of validation errors.
Expand All @@ -37,7 +56,7 @@ export function validate(state) {
[...val.matchAll(/href="([^"]+)"[^>]*>([^<]*)/gu)]
.map(match => [match[1], match[2]])
.filter(([linkUrl]) => !isHash(linkUrl))
.filter(([linkUrl]) => !validUrl.isUri(linkUrl))
.filter(([linkUrl]) => !isValid(linkUrl))
.map(([, linkText]) => ({
uri,
field: _.head(field.path.split('.')),
Expand Down
50 changes: 50 additions & 0 deletions lib/validators/built-in/invalid-links.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,56 @@ describe('validators: invalid-links', () => {
})).toEqual([]);
});

test('passes if field with mailto link', () => {
expect(fn({
components: {
[uri]: { a: '<a href="mailto:[email protected]">Lorem ipsum dolor sit amet</a>, consectetur adipisicing elit' }
},
schemas: {
[name]: {
a: {
_has: ['text']
}
}
}
})).toEqual([]);
});

test('passes if field with ftp link', () => {
expect(fn({
components: {
[uri]: { a: '<a href="ftp://1.1.1.1:21/test">Lorem ipsum dolor sit amet</a>, consectetur adipisicing elit' }
},
schemas: {
[name]: {
a: {
_has: ['text']
}
}
}
})).toEqual([]);
});

test('fails if field relative link', () => {
expect(fn({
components: {
[uri]: { a: '<a href="/subscribe">Lorem ipsum dolor sit amet</a>, consectetur adipisicing elit' }
},
schemas: {
[name]: {
a: {
_has: ['text']
}
}
}
})).toEqual([{
uri,
field: 'a',
location: 'Paragraph',
preview: 'Lorem ipsum dolor sit amet…'
}]);
});

test('fails if field with invalid link', () => {
expect(fn({
components: {
Expand Down
8 changes: 1 addition & 7 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 3 additions & 5 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "clay-kiln",
"version": "8.18.0",
"version": "8.17.0",
"description": "Editor tools for Clay",
"template": "template.handlebars",
"scripts": {
Expand All @@ -13,9 +13,8 @@
"test-local": "BABEL_ENV=test npm run jest -- --watch",
"build": "BABEL_ENV=build webpack",
"watch": "BABEL_ENV=build webpack -w",
"prepublishOnly": "BABEL_ENV=build webpack -p",
"generate-actions": "jsdoc2md --files lib/**/actions.js > docs/vuex-actions.md",
"release": "./.circleci/scripts/release.sh"
"prepublishOnly": "BABEL_ENV=build webpack -p && npm run generate-actions",
"generate-actions": "jsdoc2md --files lib/**/actions.js > docs/vuex-actions.md"
},
"repository": {
"type": "git",
Expand Down Expand Up @@ -151,7 +150,6 @@
"striptags": "^3.1.0",
"style-loader": "^0.19.0",
"url-parse": "^1.4.4",
"valid-url": "^1.0.9",
"velocity-animate": "^1.5.0",
"vue": "^2.5.9",
"vue-async-computed": "^3.3.1",
Expand Down
2 changes: 1 addition & 1 deletion webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ module.exports = {
module: {
rules: [{
// todo: remove vue-unit (and update vue-unit dep) once vue-unit hits 0.3.0
test: /node_modules\/(@tom-kitchin\/vue-unit|keen-ui|striptags|clayutils|valid-url)\//,
test: /node_modules\/(@tom-kitchin\/vue-unit|keen-ui|striptags|clayutils)\//,
loader: 'babel-loader'
}, {
test: /\.js$/,
Expand Down

0 comments on commit 77b60ae

Please sign in to comment.