Fix Service Worker matching external top-level URLs

[millennialdeveloper]

Description

At the moment the Service Worker matches any external top-level path (e.g. https://any.external.domain.com/?query_params) as a home url, as isHomeRoute makes a mere string equality check against url.pathname. This makes it return the index template for any such request.

I stumbled upon these while trying to load the script for Zendesk Chat, whose URL has the form https://v2.zopim.com/?APP_KEY_HASH.

This commit introduces an additional check to make sure that the URL origin matches the service worker origin, too, just as it would happen when passing a RegExp (bc of additional assumptions of workbox on the use of RegExps that only make them match on local URLS)

Related Issue

• Closes [Issue] Fix Service Worker matching external top-level URLs #3193

Acceptance

Verification Stakeholders

Verification Steps

• Run fetch('https://v2.zopim.com/?foo',{mode: 'no-cors'}).then(data => console.log(data)) in the console
• Verify that the service work doesn't intercept the request

Specification

Resolved issues:

1. resolves [Issue] Fix Service Worker matching external top-level URLs #3193: Fix Service Worker matching external top-level URLs

[sirugh]

@magento create issue from PR

[PWAStudioBot]

	Messages
📖	DangerCI Failures related to missing labels/description/linked issues/etc will persist until the next push or next nightly build run (assuming they are fixed).
📖	Access a deployed version of this PR here. Make sure to wait for the "pwa-pull-request-deploy" job to complete.

Generated by 🚫 dangerJS against 7895d27

[revanth0212]

Is this needed? The SW only has access to routes under the scope, in this case, the domain that launched it.

[millennialdeveloper]

I don't know about the whole service worker implementation in pwa-studio, but from here it seems that workbox may intercept external resources.

And without this change the script was loading through the service worker. The network tab showing it with a gear icon, and the response being the index template.

[sirugh]

Is this needed? The SW only has access to routes under the scope, in this case, the domain that launched it.

@revanth0212 I think, based on the link provided, if you pass a regex it will only match against the sw domain, but in our case we are intercepting all urls in the callback function.

Additionally, based on the isHomeRoute function it seems like ANY url without sub-paths ie www.google.com or zopim.com would return true so the SW would return our index. And if there was a sub path that happened to match the store code like www.google.com/en_us it would also return true and therefor our index file.

So if an external asset (like zen desk's js) is fetched from root, or if a file is requested from a path that is the first level that matches our store code, it will hit this sw route.

    if (url.pathname === '/') {
        return true;
    }

    // If store code is in the url, the home route will be url.com/view_code.
    // A trailing / may or may not follow.
    if (process.env.USE_STORE_CODE_IN_URL === 'true') {
        return AVAILABLE_STORE_VIEWS.some(
            ({ code }) =>
                url.pathname === `/${code}/` || url.pathname === `/${code}`
        );
    }
};```

[sirugh]

This is a valid fix for an issue that we're lucky to have not encountered before. Thank you for your contribution!

I will approve it, but I do wonder if we need to apply a similar pattern to this route that also just uses a callback instead of regex.

Additionally, if you are able to test the new path that would be cool. If not, please let us know so we can write the test before this gets merged.

[millennialdeveloper]

@sirugh i have no experience writing tests and it will be too long before I have proper time to learn

[sirugh]

No worries! I like to give people the opportunity but if it's not something you have time to do, we can absolutely handle that. I see you already gave it a shot which looks great :)

Thanks again for your time - we will take it from here.

[tjwiebell]

QA Pass ✅

I am getting MFTF failures for CartPageEditProductAndCheckout, but it appears the PR instance is serving the stale commit. Passes on a local, so not going to hold this up.