Skip to content
This repository has been archived by the owner on Jul 15, 2023. It is now read-only.

Commit

Permalink
[Issue #210] new rule: react-anchor-blank-noopener
Browse files Browse the repository at this point in the history
closes #210
  • Loading branch information
HamletDRC committed Sep 15, 2016
1 parent 27f4d07 commit b15bd1c
Show file tree
Hide file tree
Showing 5 changed files with 288 additions and 5 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ Rule Name | Description | Since
`react-a11y-role` | For accessibility of your website, elements with aria roles must use a **valid**, **non-abstract** aria role. A reference to role defintions can be found at [WAI-ARIA roles](https://www.w3.org/TR/wai-aria/roles#role_definitions). References:<br/>* [WCAG Rule 92: Role value must be valid](http://oaa-accessibility.org/wcag20/rule/92/)| 2.0.11
`react-a11y-tabindex-no-positive` | For accessibility of your website, enforce tabindex value is **not greater than zero**. Avoid positive tabindex attribute values to synchronize the flow of the page with keyboard tab order.<br/>References:<br/>[WCAG 2.4.3 - Focus Order](https://www.w3.org/TR/2008/REC-WCAG20-20081211/#navigation-mechanisms-focus-order)<br/>[Audit Rules - tabindex-usage](https://github.com/GoogleChrome/accessibility-developer-tools/wiki/Audit-Rules#tabindex-usage)<br/>[Avoid positive integer values for tabIndex](https://github.com/GoogleChrome/accessibility-developer-tools/wiki/Audit-Rules#ax_focus_03) | 2.0.11
`react-a11y-titles` | For accessibility of your website, HTML title elements must not be empty, must be more than one word, and must not be more than 60 characters long.<br/>References:<br/>* [WCAG 2.0 - Requirement 2.4.2 Page Titled (Level A)](http://www.w3.org/TR/WCAG20/#navigation-mechanisms-title)<br/>* [OAA-Accessibility Rule 13: Title element should not be empty](http://oaa-accessibility.org/wcag20/rule/13/)<br/>* [OAA-Accessibility Rule 24: Title content should be concise](http://oaa-accessibility.org/wcag20/rule/24/)<br/>* [OAA-Accessibility Rule 25: Title text must contain more than one word](http://oaa-accessibility.org/wcag20/rule/25/)<br/> | 2.0.11
`react-anchor-blank-noopener` | For security reasons, anchor tags with target="_blank" should also include rel="noopener noreferrer". In order to restrict the behavior window.opener access, the original page needs to add a rel="noopener" attribute to any link that has target="_blank". However, Firefox does not support that tag, so you should actually use rel="noopener noreferrer" for full coverage. For more info see: [The target="_blank" vulnerability by example](https://dev.to/ben/the-targetblank-vulnerability-by-example)| 2.0.11
`react-no-dangerous-html` | Do not use React's dangerouslySetInnerHTML API. This rule finds usages of the dangerouslySetInnerHTML API (but not any JSX references). For more info see the [react-no-dangerous-html Rule wiki page](https://github.com/Microsoft/tslint-microsoft-contrib/wiki/react-no-dangerous-html-Rule). | 0.0.2
`react-this-binding-issue` | Several errors can occur when using React and React.Component subclasses. When using React components you must be careful to correctly bind the 'this' reference on any methods that you pass off to child components as callbacks. For example, it is common to define a private method called 'onClick' and then specify `onClick={this.onClick}` as a JSX attribute. If you do this then the 'this' reference will be undefined when your private method is invoked. The React documentation suggests that you bind the 'this' reference on all of your methods within the constructor: `this.onClick = this.onClick.bind(this);`. This rule will create a violation if 1) a method reference is passed to a JSX attribute without being bound in the constructor. And 2) a method is bound multiple times in the constructor. Another issue that can occur is binding the 'this' reference to a function within the render() method. For example, many people will create an anonymous lambda within the JSX attribute to avoid the 'this' binding issue: `onClick={() => { this.onClick(); }}`. The problem with this is that a new instance of an anonymous function is created every time render() is invoked. When React compares virutal DOM properties within shouldComponentUpdate() then the onClick property will look like a new property and force a re-render. You should avoid this pattern because creating function instances within render methods breaks any logic within shouldComponentUpdate() methods. This rule creates violations if 1) an anonymous function is passed as a JSX attribute. And 2) if a function instantiated in local scope is passed as a JSX attribute. This rule can be configured via the "allow-anonymous-listeners" parameter. If you want to suppress violations for the anonymous listener scenarios then configure that rule like this: `"react-this-binding-issue": [ true, { 'allow-anonymous-listeners': true } ]` | 2.0.8, 2.0.9
`react-unused-props-and-state` | Remove unneeded properties defined in React Props and State interfaces. Any interface named Props or State is defined as a React interface. All fields in these interfaces must be referenced. | 2.0.10
Expand Down
89 changes: 89 additions & 0 deletions src/reactAnchorBlankNoopenerRule.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
import * as ts from 'typescript';
import * as Lint from 'tslint/lib/lint';

import {ErrorTolerantWalker} from './utils/ErrorTolerantWalker';
import {ExtendedMetadata} from './utils/ExtendedMetadata';
import {SyntaxKind} from './utils/SyntaxKind';
import {Utils} from './utils/Utils';

import { getJsxAttributesFromJsxElement,
getStringLiteral,
isEmpty } from './utils/JsxAttribute';

const FAILURE_STRING: string = 'Anchor tags with target="_blank" should also include rel="noopener noreferrer"';

/**
* Implementation of the react-anchor-blank-noopener rule.
*/
export class Rule extends Lint.Rules.AbstractRule {

public static metadata: ExtendedMetadata = {
ruleName: 'react-anchor-blank-noopener',
type: 'functionality',
description: 'Anchor tags with target="_blank" should also include rel="noopener noreferrer"',
options: null,
issueClass: 'SDL',
issueType: 'Error',
severity: 'Critical',
level: 'Mandatory',
group: 'Security',
commonWeaknessEnumeration: '242,676'
};

public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
if (sourceFile.languageVariant === ts.LanguageVariant.JSX) {
return this.applyWithWalker(new ReactAnchorBlankNoopenerRuleWalker(sourceFile, this.getOptions()));
} else {
return [];
}
}
}

class ReactAnchorBlankNoopenerRuleWalker extends ErrorTolerantWalker {

protected visitJsxElement(node: ts.JsxElement): void {
const openingElement: ts.JsxOpeningElement = node.openingElement;
this.validateOpeningElement(openingElement);
super.visitJsxElement(node);
}

protected visitJsxSelfClosingElement(node: ts.JsxSelfClosingElement): void {
this.validateOpeningElement(node);
super.visitJsxSelfClosingElement(node);
}

private validateOpeningElement(openingElement: ts.JsxOpeningElement): void {
if (openingElement.tagName.getText() === 'a') {

const allAttributes: { [propName: string]: ts.JsxAttribute } = getJsxAttributesFromJsxElement(openingElement);
/* tslint:disable:no-string-literal */
const target: ts.JsxAttribute = allAttributes['target'];
const rel: ts.JsxAttribute = allAttributes['rel'];
/* tslint:enable:no-string-literal */
if (getStringLiteral(target) === '_blank' && !isRelAttributeValue(rel)) {
this.addFailure(this.createFailure(openingElement.getStart(), openingElement.getWidth(), FAILURE_STRING));
}
}
}
}

function isRelAttributeValue(attribute: ts.JsxAttribute): boolean {
if (isEmpty(attribute)) {
return false;
}

if (attribute.initializer.kind === SyntaxKind.current().JsxExpression) {
const expression: ts.JsxExpression = <ts.JsxExpression>attribute.initializer;
if (expression.expression != null && expression.expression.kind !== SyntaxKind.current().StringLiteral) {
return true; // attribute value is not a string literal, so do not validate
}
}

const stringValue = getStringLiteral(attribute);
if (stringValue == null || stringValue.length === 0) {
return false;
}

const relValues: string[] = stringValue.split(/\s+/);
return Utils.contains(relValues, 'noreferrer') && Utils.contains(relValues, 'noopener');
}
10 changes: 6 additions & 4 deletions src/utils/JsxAttribute.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export function getStringLiteral(node: ts.JsxAttribute): string {
throw new Error('The node must be a JsxAttribute collected by the AST parser.');
}

const initializer: ts.Expression = node.initializer;
const initializer: ts.Expression = node == null ? null : node.initializer;

if (!initializer) { // <tag attribute/>
return '';
Expand All @@ -52,7 +52,7 @@ export function getStringLiteral(node: ts.JsxAttribute): string {
}

export function isEmpty(node: ts.JsxAttribute): boolean {
const initializer: ts.Expression = node.initializer;
const initializer: ts.Expression = node == null ? null : node.initializer;

if (initializer == null) {
return true;
Expand Down Expand Up @@ -83,7 +83,7 @@ export function getNumericLiteral(node: ts.JsxAttribute): string {
throw new Error('The node must be a JsxAttribute collected by the AST parser.');
}

const initializer: ts.Expression = node.initializer;
const initializer: ts.Expression = node == null ? null : node.initializer;

return isJsxExpression(initializer) && isNumericLiteral(initializer.expression)
? (<ts.LiteralExpression>initializer.expression).text
Expand All @@ -97,7 +97,9 @@ export function getNumericLiteral(node: ts.JsxAttribute): string {
export function getAllAttributesFromJsxElement(node: ts.Node): (ts.JsxAttribute | ts.JsxSpreadAttribute)[] {
let attributes: (ts.JsxAttribute | ts.JsxSpreadAttribute)[];

if (isJsxElement(node)) {
if (node == null) {
return [];
} else if (isJsxElement(node)) {
attributes = node.openingElement.attributes;
} else if (isJsxSelfClosingElement(node)) {
attributes = node.attributes;
Expand Down
190 changes: 190 additions & 0 deletions tests/ReactAnchorBlankNoopenerRuleTests.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,190 @@
/// <reference path="../typings/mocha.d.ts" />
/// <reference path="../typings/chai.d.ts" />

import {TestHelper} from './TestHelper';

/**
* Unit tests.
*/
describe('reactAnchorBlankNoopenerRule', () : void => {

const ruleName : string = 'react-anchor-blank-noopener';

it('should pass on blank anchor with noopener and noreferrer', () : void => {
const script : string = `
import React = require('react');
const a = <a target="_blank" rel="noopener noreferrer">link</a>;
const b = <a target="_blank" rel="noreferrer noopener">link</a>;
const c = <a target="_blank" rel="whatever noopener noreferrer">link</a>;
const d = <a target="_blank" rel="noreferrer whatever noopener">link</a>;
const e = <a target="_blank" rel="noreferrer noopener whatever">link</a>;
const f = <a target="_blank" rel="noopener noreferrer"/>;
const g = <a target="_blank" rel="noreferrer noopener"/>;
const h = <a target="_blank" rel="whatever noopener noreferrer"/>;
const i = <a target="_blank" rel="noreferrer whatever noopener"/>;
const j = <a target="_blank" rel="noreferrer noopener whatever"/>;
const k = <a target={ something() } rel="noreferrer noopener whatever"/>;
const l = <a target="_blank" rel={ something() }/>;
`;

TestHelper.assertViolations(ruleName, script, [ ]);
});

it('should pass on anchors without blank', () : void => {
const script : string = `
import React = require('react');
const a = <a target="_self" >link</a>;
const b = <a target="_whatever" >link</a>;
const c = <a target="_self" />;
const d = <a target="_whatever" />;
`;

TestHelper.assertViolations(ruleName, script, [ ]);
});

it('should fail on missing rel', () : void => {
const script : string = `
import React = require('react');
const a = <a target="_blank">link</a>;
const b = <a target={"_blank"}>link</a>;
`;

TestHelper.assertViolations(ruleName, script, [ {
"failure": "Anchor tags with target=\"_blank\" should also include rel=\"noopener noreferrer\"",
"name": "file.tsx",
"ruleName": "react-anchor-blank-noopener",
"startPosition": { "character": 23, "line": 4 }
},
{
"failure": "Anchor tags with target=\"_blank\" should also include rel=\"noopener noreferrer\"",
"name": "file.tsx",
"ruleName": "react-anchor-blank-noopener",
"startPosition": { "character": 23, "line": 5 }
}
]);
});

it('should fail on missing rel - self-closing', () : void => {
const script : string = `
import React = require('react');
const a = <a target="_blank" />;
const b = <a target={"_blank"} />;
`;

TestHelper.assertViolations(ruleName, script, [
{
"failure": "Anchor tags with target=\"_blank\" should also include rel=\"noopener noreferrer\"",
"name": "file.tsx",
"ruleName": "react-anchor-blank-noopener",
"startPosition": { "character": 23, "line": 4 }
},
{
"failure": "Anchor tags with target=\"_blank\" should also include rel=\"noopener noreferrer\"",
"name": "file.tsx",
"ruleName": "react-anchor-blank-noopener",
"startPosition": { "character": 23, "line": 5 }
}
]);
});

it('should fail on empty rel', () : void => {
const script : string = `
import React = require('react');
const a = <a target="_blank" rel="" >link</a>;
const b = <a target={"_blank"} rel={""} >link</a>;
`;

TestHelper.assertViolations(ruleName, script, [
{
"failure": "Anchor tags with target=\"_blank\" should also include rel=\"noopener noreferrer\"",
"name": "file.tsx",
"ruleName": "react-anchor-blank-noopener",
"startPosition": { "character": 23, "line": 4 }
},
{
"failure": "Anchor tags with target=\"_blank\" should also include rel=\"noopener noreferrer\"",
"name": "file.tsx",
"ruleName": "react-anchor-blank-noopener",
"startPosition": { "character": 23, "line": 5 }
}
]);
});

it('should fail on rel with only one term', () : void => {
const script : string = `
import React = require('react');
const a = <a target="_blank" rel="noreferrer" >link</a>;
const b = <a target={"_blank"} rel={"noopener"} >link</a>;
`;

TestHelper.assertViolations(ruleName, script, [
{
"failure": "Anchor tags with target=\"_blank\" should also include rel=\"noopener noreferrer\"",
"name": "file.tsx",
"ruleName": "react-anchor-blank-noopener",
"startPosition": { "character": 23, "line": 4 }
},
{
"failure": "Anchor tags with target=\"_blank\" should also include rel=\"noopener noreferrer\"",
"name": "file.tsx",
"ruleName": "react-anchor-blank-noopener",
"startPosition": { "character": 23, "line": 5 }
}
]);
});

it('should fail on rel with only one term but other terms as well', () : void => {
const script : string = `
import React = require('react');
const a = <a target="_blank" rel="noreferrer whatever" >link</a>;
const b = <a target={"_blank"} rel={"whatever noopener"} >link</a>;
`;

TestHelper.assertViolations(ruleName, script, [
{
"failure": "Anchor tags with target=\"_blank\" should also include rel=\"noopener noreferrer\"",
"name": "file.tsx",
"ruleName": "react-anchor-blank-noopener",
"startPosition": { "character": 23, "line": 4 }
},
{
"failure": "Anchor tags with target=\"_blank\" should also include rel=\"noopener noreferrer\"",
"name": "file.tsx",
"ruleName": "react-anchor-blank-noopener",
"startPosition": { "character": 23, "line": 5 }
}
]);
});

it('should fail on rel with only one term but other terms as well - self closing', () : void => {
const script : string = `
import React = require('react');
const a = <a target="_blank" rel="noreferrer whatever" />;
const b = <a target={"_blank"} rel={"whatever noopener"} />;
`;

TestHelper.assertViolations(ruleName, script, [
{
"failure": "Anchor tags with target=\"_blank\" should also include rel=\"noopener noreferrer\"",
"name": "file.tsx",
"ruleName": "react-anchor-blank-noopener",
"startPosition": { "character": 23, "line": 4 }
},
{
"failure": "Anchor tags with target=\"_blank\" should also include rel=\"noopener noreferrer\"",
"name": "file.tsx",
"ruleName": "react-anchor-blank-noopener",
"startPosition": { "character": 23, "line": 5 }
}
]);
});
});
3 changes: 2 additions & 1 deletion tslint.json
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@
],
"react-a11y-lang": true,
"react-a11y-meta": true,
"react-a11y-anchors": true
"react-a11y-anchors": true,
"react-anchor-blank-noopener": true
}
}

0 comments on commit b15bd1c

Please sign in to comment.