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

noopener alone not allowed #3044

Closed
mauro1855 opened this issue Aug 17, 2021 · 5 comments
Closed

noopener alone not allowed #3044

mauro1855 opened this issue Aug 17, 2021 · 5 comments

Comments

@mauro1855
Copy link

mauro1855 commented Aug 17, 2021

Hi,

I've been reading the discussion surrounding noopener / noreferrer, including what was done in #2043.
However, I still find the behaviour strange.

Here is my link:
<a href="https://some-external-link.com" target="_blank" rel="noopener">Link</a>

I'm getting a 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

The html spec link redirects me to noopener documentation, and yet it is still requiring me to add noreferrer. As far as I understand, the use or noreferrer or noopener removes the security risk, so the rule should not be triggered if one of them is present (no matter which). In this case, since I'm using noopener (thus removing the security risk), the warning should not be shown, imho.

Why don't I use noreferrer? I think it's important for websites to know where their traffic is coming from, so I don't intend on hiding the referrer, and I don't think you should either. The referrer information helps bloggers and independent content creators properly monitor the traffic arriving to their websites.

@ljharb
Copy link
Member

ljharb commented Aug 17, 2021

See #2022 (comment):

Due to the fact that noreferrer predates noopener, there are cases where noopener is ignored (because the behavior is not implemented).

That information also violates the privacy of the user, and I don't think your opinion is the common one here about what people should do - referrers aren't reliable and anyone relying on that info is getting a very incomplete picture, no matter what you choose to do. You are welcome to disable the rule, of course.

@mauro1855
Copy link
Author

mauro1855 commented Aug 17, 2021

Hi @ljharb,

I've read a lot of comments related to this here in the project, and it's not always obvious if the comments are referring to the use of noopener isolated, or in conjunction with noreferrer, like noreferrer noopener. That issue was initially opened because the linter was requiring both noreferrer noopener, which is not really needed since noreferrer implies noopener.

At the time, you even had an opinion which is compatible with mine: #2022 (comment):

Given that, it seems we should modify the rule - by default, even - to no longer require both (but, to continue to allow both).

I agree with this. Both noopener and noreferrer address the security constraint, so strictly speaking I don't think the rule should complain that the code is insecure if we are already using noopener. In other words, the rule should be satisfied with the use of noopener or noreferrer since, in modern browsers, any of these is enough to guarantee there is no security issue. Once again, I even highlight the fact that the rule mentions the use of noreferrer, but then the html.spec link is pointing to the noopener section of the page.

As for the privacy of the users, to be honest I think that is up for the developer and/or user to decide, together with the specific legal framework observed by the website owners. But even if that is up for discussion, I just don't think the rule is correct in saying there is a security risk even when noopener is being used.

@ljharb
Copy link
Member

ljharb commented Aug 17, 2021

The risk is as highlighted/referenced in my previous comment - that noopener isn't supported in as many browsers as noreferrer.

@Nemo64
Copy link

Nemo64 commented Sep 10, 2021

The thing is: this rule only applies to target="_blank" links.

If we really consider the Referer header to be dangerous (which it can be), then all external links should include noreferrer, regardless of target="_blank". If it's just the opener reference, then noopener on target="_blank" is enough.

However I understand that noopener is not supported on IE and if noreferrer has the same effect, then that is a good reason to still enforce noreferrer as a general precaution.

But I think the lint message should tell that noreferrer implies noopener and maybe even that it should be preferred because of wider support, as it currently only references noopener.

While looking though the source, I found that it's possible to set "allowReferrer": true which then will allow noopener, which might interest you @mauro1855.

{
  "extends": ["next", "next/core-web-vitals"],
  "rules": {
    "react/jsx-no-target-blank": ["error", {"allowReferrer": true}]
  }
}

While looking that up, I found that over the last 2 years, browsers started to imply noopener, which is nice.

There is also the much more granular Referrer-Policy header that can take care of it.
That header can even reduce the Referer to only include the Origin, which I really like as it can still give another server admin some idea where traffic is roughly coming from without potentially exposing url parameters.

@ljharb
Copy link
Member

ljharb commented Sep 16, 2021

@Nemo64 i'd be happy to review a PR that improves the error message, and points users to where they can read more about the nuance.

cutiful added a commit to cutiful/eslint-plugin-react that referenced this issue Sep 29, 2021
Show different error messages depending on whether referrer is allowed; clarify
about `noreferrer` only being necessary in older browsers.

Closes jsx-eslint#3044.
@ljharb ljharb closed this as completed in 7844d8e Sep 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants