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

[New] Add `jsx-no-script-url` to prevent usage of `javascript:` URLs #2419

Open
wants to merge 2 commits into
base: master
from

Conversation

@sergei-startsev
Copy link
Contributor

commented Sep 25, 2019

In React 16.9 any URLs starting with javascript: scheme log a warning. React considers the pattern as a dangerous attack surface, see details and in a future major release it will throw an error if it encounters a javascript: URL.

The rule prevents usage of javascript: URLs in JSX.

@sergei-startsev sergei-startsev force-pushed the sergei-startsev:jsx-no-script-url branch from 9ec97c4 to 92f3cef Sep 25, 2019
@ljharb ljharb added the new rule label Oct 1, 2019
@ljharb ljharb requested a review from gaearon Oct 1, 2019
The following patterns are **not** considered warnings:

```jsx
<Foo href="javascript:"></Foo>

This comment has been minimized.

Copy link
@ljharb

ljharb Oct 1, 2019

Collaborator

what about a Link component? we should provide a way to indicate that a specific list of custom components is like an "a", and also to override the name of the "href" prop on those elements.

This comment has been minimized.

Copy link
@sergei-startsev

sergei-startsev Oct 1, 2019

Author Contributor

@ljharb Exports can be renamed during import, so Link component can be imported as Foo:

import { Link as Foo } from "react-router-dom";

How are you going to handle it?
Moreover if you pass javascript:void(0) as to property to Link component from react-router-dom, it doesn't set the same value as href attribute:

<Link to="javascript:void(0)">Link</Link>

// it adds '/', JS isn't run when you click
<a href="/javascript:void(0)">Link</a>

This comment has been minimized.

Copy link
@ljharb

ljharb Oct 1, 2019

Collaborator

It'd be by name; if you rename it to Link2 it wouldn't error, to Link it would.

That's certainly one example of a Link component, but not one I've ever seen since blindly prepending / would break some kinds of URLs.

This comment has been minimized.

Copy link
@sergei-startsev

sergei-startsev Oct 1, 2019

Author Contributor

@ljharb The PR has been updated, take a look when you get a chance.

This comment has been minimized.

Copy link
@sergei-startsev

sergei-startsev Oct 6, 2019

Author Contributor

@ljharb are there any updates here?

This comment has been minimized.

Copy link
@ljharb

ljharb Oct 8, 2019

Collaborator

i'll rereview, but i'm waiting to hear from @gaearon before merging this.

This comment has been minimized.

Copy link
@sergei-startsev

sergei-startsev Oct 16, 2019

Author Contributor

@ljharb We have over two weeks since the last commit. Is there any chance to be merged?

@sergei-startsev sergei-startsev force-pushed the sergei-startsev:jsx-no-script-url branch 4 times, most recently from 3285ca7 to 01d9fec Oct 1, 2019
@sergei-startsev sergei-startsev requested a review from ljharb Oct 6, 2019
@sergei-startsev sergei-startsev force-pushed the sergei-startsev:jsx-no-script-url branch from 01d9fec to 2f267ca Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.