Skip to content
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

Allow custom components in jsx-no-target-blank #2116

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions README.md
Expand Up @@ -48,6 +48,11 @@ You should also specify settings that will be shared across all the plugin rules
"forbidExtraProps",
{"property": "freeze", "object": "Object"},
{"property": "myFavoriteWrapper"}
],
"linkComponents": [
// Components used as alternatives to <a> for linking, eg. <Link to={ url } />
"Hyperlink",
{"name": "Link", "linkAttribute": "to"}
]
}
}
Expand Down
41 changes: 32 additions & 9 deletions docs/rules/jsx-no-target-blank.md
Expand Up @@ -21,36 +21,59 @@ This rule aims to prevent user generated links from creating security vulnerabil
* enabled: for enabling the rule. 0=off, 1=warn, 2=error. Defaults to 0.
* enforce: optional string, 'always' or 'never'

### always (default)
### `enforceDynamicLinks`

#### always

`{"enforceDynamicLinks": "always"}` enforces the rule if the href is a dynamic link (default)

When {"enforceDynamicLinks": "always"} is set, the following patterns are considered errors:

```jsx
var Hello = <a target='_blank' href="http://example.com/"></a>
var Hello = <a target='_blank' href={ dynamicLink }></a>
var Hello = <a target='_blank' href={dynamicLink}></a>
```

The following patterns are **not** considered errors:

```jsx
var Hello = <p target='_blank'></p>
var Hello = <a target='_blank' rel='noopener noreferrer' href="http://example.com"></a>
var Hello = <a target='_blank' href="relative/path/in/the/host"></a>
var Hello = <a target='_blank' href="/absolute/path/in/the/host"></a>
var Hello = <p target="_blank"></p>
EvHaus marked this conversation as resolved.
Show resolved Hide resolved
var Hello = <a target="_blank" rel="noopener noreferrer" href="http://example.com"></a>
var Hello = <a target="_blank" href="relative/path/in/the/host"></a>
var Hello = <a target="_blank" href="/absolute/path/in/the/host"></a>
var Hello = <a></a>
```

### never
#### never

`{"enforceDynamicLinks": "never"}` does not enforce the rule if the href is a dynamic link

When {"enforceDynamicLinks": "never"} is set, the following patterns are **not** considered errors:

```jsx
var Hello = <a target='_blank' href={ dynamicLink }></a>
var Hello = <a target='_blank' href={dynamicLink}></a>
```

### Custom link components

This rule supports the ability to use custom components for links, such as `<Link />` which is popular in libraries like `react-router`, `next.js` and `gatsby`. To enable this, define your custom link components in the global [shared settings](https://github.com/yannickcr/eslint-plugin-react/blob/master/README.md#configuration) under the `linkComponents` configuration area. Once configured, this rule will check those components as if they were `<a />` elements.

The following patterns are considered errors:

```jsx
var Hello = <Link target="_blank" to="http://example.com/"></Link>
var Hello = <Link target="_blank" to={dynamicLink}></Link>
```

The following patterns are **not** considered errors:

```jsx
var Hello = <Link target="_blank" rel="noopener noreferrer" to="http://example.com"></Link>
var Hello = <Link target="_blank" to="relative/path/in/the/host"></Link>
var Hello = <Link target="_blank" to="/absolute/path/in/the/host"></Link>
var Hello = <Link />
```

## When Not To Use It

If you do not have any external links, you can disable this rule
If you do not have any external links, you can disable this rule
16 changes: 10 additions & 6 deletions lib/rules/jsx-no-target-blank.js
Expand Up @@ -5,6 +5,7 @@
'use strict';

const docsUrl = require('../util/docsUrl');
const linkComponentsUtil = require('../util/linkComponents');

// ------------------------------------------------------------------------------
// Rule Definition
Expand All @@ -18,16 +19,16 @@ function isTargetBlank(attr) {
attr.value.value.toLowerCase() === '_blank';
}

function hasExternalLink(element) {
function hasExternalLink(element, linkAttribute) {
return element.attributes.some(attr => attr.name &&
attr.name.name === 'href' &&
attr.name.name === linkAttribute &&
attr.value.type === 'Literal' &&
/^(?:\w+:|\/\/)/.test(attr.value.value));
}

function hasDynamicLink(element) {
function hasDynamicLink(element, linkAttribute) {
return element.attributes.some(attr => attr.name &&
attr.name.name === 'href' &&
attr.name.name === linkAttribute &&
attr.value.type === 'JSXExpressionContainer');
}

Expand Down Expand Up @@ -63,14 +64,17 @@ module.exports = {
create: function(context) {
const configuration = context.options[0] || {};
const enforceDynamicLinks = configuration.enforceDynamicLinks || 'always';
const components = linkComponentsUtil.getLinkComponents(context);

return {
JSXAttribute: function(node) {
if (node.parent.name.name !== 'a' || !isTargetBlank(node) || hasSecureRel(node.parent)) {
if (!components.has(node.parent.name.name) || !isTargetBlank(node) || hasSecureRel(node.parent)) {
return;
}

if (hasExternalLink(node.parent) || (enforceDynamicLinks === 'always' && hasDynamicLink(node.parent))) {
const linkAttribute = components.get(node.parent.name.name);

if (hasExternalLink(node.parent, linkAttribute) || (enforceDynamicLinks === 'always' && hasDynamicLink(node.parent, linkAttribute))) {
context.report(node, 'Using target="_blank" without rel="noopener noreferrer" ' +
'is a security risk: see https://mathiasbynens.github.io/rel-noopener');
}
Expand Down
21 changes: 21 additions & 0 deletions lib/util/linkComponents.js
@@ -0,0 +1,21 @@
/**
* @fileoverview Utility functions for propWrapperFunctions setting
*/
'use strict';

const DEFAULT_LINK_COMPONENTS = ['a'];
const DEFAULT_LINK_ATTRIBUTE = 'href';

function getLinkComponents(context) {
const settings = context.settings || {};
return new Map(DEFAULT_LINK_COMPONENTS.concat(settings.linkComponents || []).map(value => {
if (typeof value === 'string') {
return [value, DEFAULT_LINK_ATTRIBUTE];
}
return [value.name, value.linkAttribute];
}));
}

module.exports = {
getLinkComponents: getLinkComponents
};
20 changes: 20 additions & 0 deletions tests/lib/rules/jsx-no-target-blank.js
Expand Up @@ -47,6 +47,16 @@ ruleTester.run('jsx-no-target-blank', rule, {
{
code: '<a target="_blank" href={ dynamicLink }></a>',
options: [{enforceDynamicLinks: 'never'}]
},
{
code: '<Link target="_blank" href={ dynamicLink }></Link>',
options: [{enforceDynamicLinks: 'never'}],
settings: {linkComponents: ['Link']}
},
{
code: '<Link target="_blank" to={ dynamicLink }></Link>',
options: [{enforceDynamicLinks: 'never'}],
settings: {linkComponents: {name: 'Link', linkAttribute: 'to'}}
}
],
invalid: [{
Expand Down Expand Up @@ -83,5 +93,15 @@ ruleTester.run('jsx-no-target-blank', rule, {
code: '<a target="_blank" href={ dynamicLink }></a>',
options: [{enforceDynamicLinks: 'always'}],
errors: defaultErrors
}, {
code: '<Link target="_blank" href={ dynamicLink }></Link>',
options: [{enforceDynamicLinks: 'always'}],
settings: {linkComponents: ['Link']},
errors: defaultErrors
}, {
code: '<Link target="_blank" to={ dynamicLink }></Link>',
options: [{enforceDynamicLinks: 'always'}],
settings: {linkComponents: {name: 'Link', linkAttribute: 'to'}},
errors: defaultErrors
}]
});
5 changes: 5 additions & 0 deletions tests/util/.eslintrc
@@ -0,0 +1,5 @@
{
"env": {
"mocha": true
}
}
35 changes: 35 additions & 0 deletions tests/util/linkComponents.js
@@ -0,0 +1,35 @@
'use strict';

const assert = require('assert');
const linkComponentsUtil = require('../../lib/util/linkComponents');

describe('linkComponentsFunctions', () => {
describe('getLinkComponents', () => {
it('returns a default map of components', () => {
const context = {};
assert.deepStrictEqual(linkComponentsUtil.getLinkComponents(context), new Map([
['a', 'href']
]));
});

it('returns a map of components', () => {
const linkComponents = [
'Hyperlink',
{
name: 'Link',
linkAttribute: 'to'
}
];
const context = {
settings: {
linkComponents: linkComponents
}
};
assert.deepStrictEqual(linkComponentsUtil.getLinkComponents(context), new Map([
['a', 'href'],
['Hyperlink', 'href'],
['Link', 'to']
]));
});
});
});
1 change: 0 additions & 1 deletion tests/util/propWrapper.js
@@ -1,4 +1,3 @@
/* eslint-env mocha */
'use strict';

const assert = require('assert');
Expand Down
1 change: 0 additions & 1 deletion tests/util/version.js
@@ -1,4 +1,3 @@
/* eslint-env mocha */
'use strict';

const path = require('path');
Expand Down