johnmichel / Library-Detector-for-Chrome

🔍 Extension that detects which JavaScript libraries are running on a page
https://chrome.google.com/webstore/detail/cgaocdmhkmfnkdkbnckgmpopcbpaaejo
MIT License
662 stars 111 forks source link

Handle non-successful asset-manifest requests #170

Closed johnmichel closed 4 years ago

johnmichel commented 4 years ago

Closes #158, #164

housseindjirdeh commented 4 years ago

Thanks @johnmichel ❤️

housseindjirdeh commented 4 years ago

@johnmichel This would still show a 404 if I'm not mistaken? Did you happen to test if it fixes the issue of logging in to BitBucket?

johnmichel commented 4 years ago

@johnmichel This would still show a 404 if I'm not mistaken? Did you happen to test if it fixes the issue of logging in to BitBucket?

@housseindjirdeh Yea, it'll still show a 404 in developer tools. It unfortunately doesn't appear to fix the BitBucket auth flow either. I'll see if I can figure out why that is and if there is a workaround.

johnmichel commented 4 years ago

@housseindjirdeh I was able to access BitBucket after wrapping the create-react-app test in a setTimeout call like so:

test: async function (win) {
    const craTest = async () => {
        // CRA does not have any explicit markers
        // This requests for an asset-manifest.json file that exists for mostly all v2 and v3 CRA apps
        try {
            const response = await fetch('asset-manifest.json');

            if (!response.ok) {
                return false;
            }
            const manifest = await response.json();

            const hasFilesEntrypoints = manifest.files || manifest.entrypoints;
            const containsMainBundle = manifest['main.js'] || (manifest.files && manifest.files['main.js']);

            if (hasFilesEntrypoints || containsMainBundle) {
                return { version: UNKNOWN_VERSION };
            }

            return false;
        }
        catch (err) {
            return false;
        }
    }

    setTimeout(
        craTest
    , 10000);
}

It's not the most elegant solution, but given that we have minimal visibility as to what is breaking the login process (my gut guess is that the request to asset-manifest.json to one of the auth server hosts is invalidating some kind of request), it might still be the "best" fix since this will delay or even prevent that request from occurring. Lemme know what you think.

johnmichel commented 4 years ago

After some more testing, that will delay the create-react-app test to the point where detectLibraries will always fail when trying to read the result for create-react-app 😞

Not really sure what the solution should be, but I'm open to ideas.

housseindjirdeh commented 4 years ago

@johnmichel Thanks a million for digging in.

I think the safest option would be to remove it entirely until we figure out a way to make it work. I used the method of requesting for asset-manifest.json since there's no specific signatures that can be used to detect usage of CRA. If I had known it would affect authentication on some sites, I wouldn't have merged it in 😅

Thanks again, I'll think of better ways to detect it, but I'll put up a PR soon to remove it for now.