facebook / react

The library for web and native user interfaces.
https://react.dev
MIT License
225.92k stars 46.09k forks source link

Bug: React Dev Tools Firefox extension fails to detect React #17997

Closed Kein closed 2 weeks ago

Kein commented 4 years ago

React extension version: 4.4.0

Steps To Reproduce

  1. Install Firefox 72.0.2x64
  2. Go to Discordapp.com

The current behavior

Extension reports: "This page does not appears to be using React"

The expected behavior

React is detected (just like it is currently in Chrome/Chromium)

hristo-kanchev commented 4 years ago

It's happening because the page is explicitly blocking our script that gets injected in the page. We are using that to detect if React is there or not.

Content Security Policy: The page’s settings blocked the loading of a resource at inline (“script-src”). in injectGlobalHook.js

Kein commented 4 years ago

What are my options here?

ruslan-shuster commented 4 years ago

Can I take this issue?

threepointone commented 4 years ago

@ruslan-shuster sure! assigning to you.

sannimdev commented 4 years ago

Chrome Too... I let my chrome browser can access URL and I turned on this addon.

ruslan-shuster commented 4 years ago

@ineggapps may I ask what is the url you were trying to access?

Kein commented 4 years ago

For me it works just fine in Chromium[ungoogled] 79.0.3945.88. In Vivladi 2.10/2.11 it is broken (chromium 80.x base)

ruslan-shuster commented 4 years ago

@Kein the problem which you described is now more or less understood 1) Your backend returns content-security-policy header which forbids injecting inline scripts to the page 2) react-devtools does inject some <script> tag to register window hooks, because Chrome content scripts cannot directly modify the window object, and react-devtools needs to install some hooks to the window.

So, what I'm going to do (if react core team agrees with this solution), is to implement some dedicated solution for firefox, which allows to install window hooks without injecting <script> tags.

You can see more discussion of the problem & solutions here.

ruslan-shuster commented 4 years ago

@threepointone I had some time investigating this issue, and in order to solve it some decisions should be taken which I, as the first-time contributor, don't feel empowered to take. I'd like to hear your opinion, and also opinion of @bvaughn, if possible. So, here is what happens:

1) There is a content-security-policy script-src directive, which specifies from which sources the scripts for the web page may come. On the issue reported, this directive was used to allow only specified origins and scripts with particular nonce.

2) Global hook(__REACT_DEVTOOLS_GLOBAL_HOOK__) is being injected using inline <script> tag, as well as react_devtools_backend, which are crucial for the react-devtools execution, and, as a consequence, for recognising React on the page.

The reason for such injection is that both Chrome and Firefox make some restriction for communication with the window. As far as I know, this is the only way you can run the code communicating with the window in Chrome.

If Firefox, you can use sharing objects with page scripts, but you need either to use cloneInto() all the time to export objects to the page script context, or use constructors defined on the window pretty much all the time when you want to make some object from extension be accessible from the page script content. Not only __REACT_DEVTOOLS_GLOBAL_HOOK__ should be exported in this way, but also all of the internals of the event emitter, because the closures will still hold them in the captured scope.

The ideal solution for Firefox would be to re-implement all the mechanisms that need to run in the context of the user page to use mechanisms described here, but it will make most of the code Firefox-specific, so we'll end up having and supporting two separate devtools frontend implementations. Since this issue was marked as good first issue and the effort would be quite large, I assume this is not what you aimed for.

Another solution would be to use hashes to whitelist <script>s which we're going to install as described here, and inform users to include those scripts as exceptions to their content-security-policy headers. This is small effort, and does not split extension functionality between browsers, but any time we change the scripts we inject the user will need to update the content-security-policy header.

The third solution is to notify the user when we did not succeed to install hooks. Here is another problem: we cannot for sure identify that it happened because of the content-security-policy, though some hacks to do it described here. .

So, I'd like to know what you think about it and what direction the solution should take?

bvaughn commented 4 years ago

This is a very helpful writeup, @ruslan-shuster. Thank you!

I agree with you that the first solution is not ideal.

The second one sounds promising, although I wonder how often the hash would change. (I guess the hook source changes pretty infrequently.)

Maybe we could even combine solutions two and three, so if the insert fails- and we think it's likely CSP related- we log the exact CSC sha hash string that would need to be used to enable the current version of DevTools tow ork?

ruslan-shuster commented 4 years ago

@bvaughn sounds good, thanks for your help! Now I know which direction to take. Working on it...

nightpool commented 4 years ago

One thing to note—this is a Firefox-specific bug, because the Content Script Policy spec says that add-ons and other user customizations should not be subject to content security policy, and Chrome implements this correctly.

https://bugzilla.mozilla.org/show_bug.cgi?id=1267027 is the Firefox tracking bug to fix this issue.

bvaughn commented 4 years ago

Thanks for that link, @nightpool

stale[bot] commented 4 years ago

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

nightpool commented 4 years ago

bump

stale[bot] commented 3 years ago

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

MateusWagnitz commented 3 years ago

bump

MeanManMachineMan commented 3 years ago

The same issue exists while visiting Github(this page in fact) and DIscord(www.discord.com). I'm using Firefox 85.0.1x64 on Manjaro. Content Security Policy: The page’s settings blocked the loading of a resource at inline (“script-src”). and Error walking your react tree are the two main errors I see in my console upon visiting Discord. I'm currently on React Dev Tools 4.10.1.

Alef5750 commented 3 years ago

Hi - I'm new to open source. Is this issue already being dealt with or can I take a shot at it?

bvaughn commented 3 years ago

@Alef5750 You are welcome to look into this issue if you'd be interested 😄 It's still up for grabs.

Alef5750 commented 3 years ago

Will do! Thanks!

On Wed, May 26, 2021 at 5:09 PM Brian Vaughn @.***> wrote:

@Alef5750 https://github.com/Alef5750 You are welcome to look into this issue if you'd be interested 😄 It's still up for grabs.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/facebook/react/issues/17997#issuecomment-848801568, or unsubscribe https://github.com/notifications/unsubscribe-auth/AP3YZTE2MSIDWSSN7QGP6JDTPT6J5ANCNFSM4KRMSQQA .

shuhblam commented 2 years ago

@bvaughn thanks for adding https://github.com/facebook/react/blob/main/packages/react-devtools/CONTRIBUTING.md I am going to see if I can duplicate this issue in firefox. do you know if similar is expected to work for firefox?

yarn build:chrome && yarn test:chrome --url=<url-to-test>
bvaughn commented 2 years ago

@colegillespie Yes. You can replace "chrome" with either "firefox" or "edge" if you wanted to test other browsers 👍🏼

shuhblam commented 2 years ago

@bvaughn confirmed, was able to repro the error with the tests running

yarn test:firefox --url=https://discord.com/

image

will see if i can fix now

shuhblam commented 2 years ago

@bvaughn I was able to get this working and tests passing. Quite a drastic change in the way the global hook is inserted so please check the PR and make sure I am not doing anything too aggregious.

garykhong commented 2 years ago

Hello, Can I lend a hand with this issue, or is it being looked after? I don't mind testing the PR as well.

bvaughn commented 2 years ago

@garykhong Feel free to take a look!

garykhong commented 2 years ago

Thanks Brian. Something came up, and I will try to look at this on the weekend. Do you have a guide on how I can debug the react dev tools?

bvaughn commented 2 years ago

Just the contributing guide: https://github.com/facebook/react/blob/main/packages/react-devtools/CONTRIBUTING.md

garykhong commented 2 years ago

Hi Brian,

I was able to get discord.com in firefox to load the react developer tools by setting the correct nonce on the injectGlobalHook.js script variable on line 8.

Is the ideal solution still to hash this script variable and to 'Add the hash to DevTools setting UI'? Or are you happy with catering just for Content Security Policies with nonce being set?

MeanManMachineMan commented 2 years ago

Hey @bvaughn @garykhong , is anyone looking at this or can I take a crack at it as a first-time contributor? :D

garykhong commented 2 years ago

Hey @bvaughn @garykhong , is anyone looking at this or can I take a crack at it as a first-time contributor? :D

Go for it Vidur

hauntingEcho commented 1 year ago

I'm seeing this issue as well. It looks like uBlock Origin was able to work around this by just shoving the script into a Blob URL

github-actions[bot] commented 3 months ago

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

hauntingEcho commented 3 months ago

bump

github-actions[bot] commented 3 weeks ago

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

github-actions[bot] commented 2 weeks ago

Closing this issue after a prolonged period of inactivity. If this issue is still present in the latest release, please create a new issue with up-to-date information. Thank you!