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

Resolve Service Worker redirects based on the response #27444

Merged
merged 1 commit into from
Feb 5, 2021

Commits on Feb 5, 2021

  1. Resolve Service Worker redirects based on the response

    We currently resolve Service-Worker-forwarded location headers using the
    request. While this matches Firefox, this does not match the spec or
    Safari's behavior. Instead, the spec says to resolve the location header
    based on the response's URL.
    
    This comes up if the FetchEvent was for /, but the Service Worker
    responded with ev.respondWith(fetch("/foo/", {redirect: "manual"})). In
    that case, a Location: bar.html header would result in /bar.html by our
    version and /foo/bar.html by the spec's version.
    
    Align with the spec. This makes the redirect go where it would have gone
    under {redirect: "follow"}. This has two platform-visible behavior
    changes:
    
    - First, cases like the above will result in a different URL.
    
    - Second, script-constructed Response objects do not have a URL list. If
      the URLs are absolute, this works fine. If they are relative, those
      fetches will now result in a network error. Note Response.redirect()
      internally constructs absolute URLs, so those continue to work. This
      only affects ev.respondWith(new Response(... location: "bar.html"}})).
    
    Both of these changes match Safari.
    
    Note that, as of writing, the Fetch spec describes this behavior in
    terms of a location URL property on the response object. This would
    require computing the location URL earlier and preserving it across many
    layers, including persisting in CacheStorage. See
    https://chromium-review.googlesource.com/c/chromium/src/+/2648648.
    
    Instead, this CL uses the equivalent formulation in
    whatwg/fetch#1149. See also discussion in
    whatwg/fetch#1146.
    
    Bug: 1170379
    Change-Id: Ibb6b12566244fd259029e67787dd7f08edeece9d
    Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2665871
    Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
    Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
    Reviewed-by: Ben Kelly <wanderview@chromium.org>
    Commit-Queue: David Benjamin <davidben@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#850874}
    davidben authored and chromium-wpt-export-bot committed Feb 5, 2021
    Configuration menu
    Copy the full SHA
    8da6e39 View commit details
    Browse the repository at this point in the history