-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Don't warn react/jsx-no-target-blank if "opener" or "referrer" are in the rel attribute #2054
Comments
I'm confused; the first one already doesn't warn: https://github.com/yannickcr/eslint-plugin-react/blob/master/tests/lib/rules/jsx-no-target-blank.js#L37 As for the others - see https://mathiasbynens.github.io/rel-noopener/. "opener" is never recommended, so I would expect those to warn as well. The same logic seems to support not allowing "referrer" either. |
The first one is currently the only case that doesn't warn, I'm just suggesting that explicitly saying you want For newer browsers, I indeed haven't found any case where I'd want to use |
If something is only safe on newer browsers, then it’s not safe, and shouldn’t be permitted. What cases do you have where you want the referrer header sent? |
We're using a module to generate URLs instead of hardcoding paths (a manually maintained reverse router, if you will), and that trips the plugin even for ultimately relative URLs with
|
hmm. so it's not even about external URLs, it's about certain special cases of dynamic URLs where you'd prefer to not even have to specify opener/referrer? Considering that force-opening links in a new tab is inherently a hostile UX interaction, and even more so for links on the same site, I'm not inclined to let the default behavior allow either Is there a rule option you could think of that would be simpler than eslint override comments or removing the blank target, that would support your use case? |
I have encountered this issue for a particular link to an external site where it is desirable to keep the Referer heading working. I agree that Referrer is a bit different though; unless there's a particularly sensitive piece of information in the URL, it is not a security concern. It is also sent as standard practice for any link which isn't I think the proposed solution of allowing developers to explicitly specify that referrer is OK for a particular link is a good one. If I could mark the link as "noopener referrer" and have the lint warning disappear I'd be happy with that. I don't agree with allowing "opener" though; if somebody wants to do that they should have to explicitly disable the rule, because that indicates a genuine design flaw. Right now my only recourse is to surround the block with Rule options are not useful to me, because I want to be able to apply this to a specific link, not a bunch of links with something in common. I want the rule to apply as normal everywhere else. |
Allowing an explicit |
I totally agree with that. The referer bit can leak privacy but being able to set |
Just to add the Lighthouse audit for this issue recommends that one of https://developers.google.com/web/tools/lighthouse/audits/noopener |
This creates noises. Most of my links have I'd like an option to avoid warnings when one of them is used (instead of both). I also believe it should become the default behaviour. |
> warning Using target="_blank" without rel="noreferrer" is a security risk: see https://html.spec.whatwg.org/multipage/links.html#link-type-noopener react/jsx-no-target-blank This seems to be valid. See jsx-eslint/eslint-plugin-react#2054
> warning Using target="_blank" without rel="noreferrer" is a security risk: see https://html.spec.whatwg.org/multipage/links.html#link-type-noopener react/jsx-no-target-blank This seems to be valid. See jsx-eslint/eslint-plugin-react#2054
just ran into an issue where i had
finally figured out i needed to add if it's possible for the warning to be more clear about what needs to be changed I think that would be a great improvement. |
@aaronmgdr Merged PR #2925 already fixes this particular issue (#2924); once there's a release, there won't be an error anymore with |
Details in this issue from eslint-plugin-react: jsx-eslint/eslint-plugin-react#2054
In other words, these should all not warn:
However, if either
(no)opener
or(no)referer
are missing, the rule should still warn.The text was updated successfully, but these errors were encountered: