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 2 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
42 changes: 33 additions & 9 deletions docs/rules/jsx-no-target-blank.md
Expand Up @@ -20,37 +20,61 @@ 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'
* Link components can be something other than an `<a>`, see [shared settings](https://github.com/yannickcr/eslint-plugin-react/blob/master/README.md#configuration) for `linkComponents` configuration)
gbakernet marked this conversation as resolved.
Show resolved Hide resolved

### `enforceDynamicLinks`

#### always

### always (default)
`{"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>
```

### Link components
gbakernet marked this conversation as resolved.
Show resolved Hide resolved

Link components can be something other than an `<a>`, see [shared settings](https://github.com/yannickcr/eslint-plugin-react/blob/master/README.md#configuration) for `linkComponents` configuration)

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
}]
});
36 changes: 36 additions & 0 deletions tests/util/linkComponents.js
@@ -0,0 +1,36 @@
/* eslint-env mocha */
gbakernet marked this conversation as resolved.
Show resolved Hide resolved
'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']
]));
});
});
});