Merge pull request #1216 from gfx/no-target-blank/only-for-external-l…

…inks Don't issue errors for `<a/>` with target=_blank and non-external URLs
jsx-eslint · May 24, 2017 · 5aa4470 · 5aa4470
2 parents 089beec + 89c33b6
commit 5aa4470
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 28 deletions.
diff --git a/docs/rules/jsx-no-target-blank.md b/docs/rules/jsx-no-target-blank.md
@@ -1,6 +1,6 @@
 # Prevent usage of unsafe `target='_blank'` (react/jsx-no-target-blank)
 
-When creating a JSX element that has an a tag, it is often desired to have
+When creating a JSX element that has an `a` tag, it is often desired to have
 the link open in a new tab using the `target='_blank'` attribute. Using this
 attribute unaccompanied by `rel='noreferrer noopener'`, however, is a severe
 security vulnerability ([see here for more details](https://mathiasbynens.github.io/rel-noopener))
@@ -11,14 +11,16 @@ This rules requires that you accompany all `target='_blank'` attributes with `re
 The following patterns are considered errors:
 
 ```jsx
-var Hello = <a target='_blank'></a>
+var Hello = <a target='_blank' href="http://example.com/"></a>
 ```
 
 The following patterns are not considered errors:
 
 ```jsx
 var Hello = <p target='_blank'></p>
-var Hello = <a target='_blank' rel='noopener noreferrer'></a>
+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>
 ```
 

diff --git a/lib/rules/jsx-no-target-blank.js b/lib/rules/jsx-no-target-blank.js
@@ -8,6 +8,31 @@
 // Rule Definition
 // ------------------------------------------------------------------------------
 
+function isTargetBlank(attr) {
+  return attr.name.name === 'target' &&
+    attr.value.type === 'Literal' &&
+    attr.value.value.toLowerCase() === '_blank';
+}
+
+function hasExternalLink(element) {
+  return element.attributes.find(function (attr) {
+    return attr.name &&
+      attr.name.name === 'href' &&
+      attr.value.type === 'Literal' &&
+      /^(?:\w+:|\/\/)/.test(attr.value.value);
+  });
+}
+
+function hasSecureRel(element) {
+  return element.attributes.find(function (attr) {
+    if (attr.name.name === 'rel') {
+      var tags = attr.value.type === 'Literal' && attr.value.value.toLowerCase().split(' ');
+      return !tags || (tags.indexOf('noopener') >= 0 && tags.indexOf('noreferrer') >= 0);
+    }
+    return false;
+  });
+}
+
 module.exports = {
   meta: {
     docs: {
@@ -26,25 +51,12 @@ module.exports = {
         }
 
         if (
-          node.name.name === 'target' &&
-          node.value.type === 'Literal' &&
-          node.value.value.toLowerCase() === '_blank'
+          isTargetBlank(node) &&
+          hasExternalLink(node.parent) &&
+          !hasSecureRel(node.parent)
         ) {
-          var relFound = false;
-          var attrs = node.parent.attributes;
-          for (var idx in attrs) {
-            if (attrs[idx].name && attrs[idx].name.name === 'rel') {
-              var tags = attrs[idx].value.type === 'Literal' && attrs[idx].value.value.toLowerCase().split(' ');
-              if (!tags || (tags.indexOf('noopener') >= 0 && tags.indexOf('noreferrer') >= 0)) {
-                relFound = true;
-                break;
-              }
-            }
-          }
-          if (!relFound) {
-            context.report(node, 'Using target="_blank" without rel="noopener noreferrer" ' +
-            'is a security risk: see https://mathiasbynens.github.io/rel-noopener');
-          }
+          context.report(node, 'Using target="_blank" without rel="noopener noreferrer" ' +
+          'is a security risk: see https://mathiasbynens.github.io/rel-noopener');
         }
       }
     };

diff --git a/tests/lib/rules/jsx-no-target-blank.js b/tests/lib/rules/jsx-no-target-blank.js
@@ -35,31 +35,39 @@ ruleTester.run('jsx-no-target-blank', rule, {
     {code: '<p target="_blank"></p>'},
     {code: '<a href="foobar" target="_BLANK" rel="NOOPENER noreferrer"></a>'},
     {code: '<a target="_blank" rel={relValue}></a>'},
-    {code: '<a target={targetValue} rel="noopener noreferrer"></a>'}
+    {code: '<a target={targetValue} rel="noopener noreferrer"></a>'},
+    {code: '<a target={targetValue} href="relative/path"></a>'},
+    {code: '<a target={targetValue} href="/absolute/path"></a>'}
   ],
   invalid: [{
-    code: '<a target="_blank"></a>',
+    code: '<a target="_blank" href="http://example.com"></a>',
     errors: [{
       message: 'Using target="_blank" without rel="noopener noreferrer" is a security risk:' +
       ' see https://mathiasbynens.github.io/rel-noopener'
     }]
   }, {
-    code: '<a target="_blank" rel=""></a>',
+    code: '<a target="_blank" rel="" href="http://example.com"></a>',
     errors: [{
       message: 'Using target="_blank" without rel="noopener noreferrer" is a security risk:' +
       ' see https://mathiasbynens.github.io/rel-noopener'
     }]
   }, {
-    code: '<a target="_blank" rel="noopenernoreferrer"></a>',
+    code: '<a target="_blank" rel="noopenernoreferrer" href="http://example.com"></a>',
     errors: [{
       message: 'Using target="_blank" without rel="noopener noreferrer" is a security risk:' +
       ' see https://mathiasbynens.github.io/rel-noopener'
     }]
   }, {
-    code: '<a target="_BLANK"></a>',
+    code: '<a target="_BLANK" href="http://example.com"></a>',
     errors: [{
       message: 'Using target="_blank" without rel="noopener noreferrer" is a security risk:' +
       ' see https://mathiasbynens.github.io/rel-noopener'
-    }]}
-  ]
+    }]
+  }, {
+    code: '<a target="_blank" href="//example.com"></a>',
+    errors: [{
+      message: 'Using target="_blank" without rel="noopener noreferrer" is a security risk:' +
+      ' see https://mathiasbynens.github.io/rel-noopener'
+    }]
+  }]
 });