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

Put a fallback URL at the beginning of signed exchanges #288

Merged
merged 13 commits into from
Aug 20, 2018

Conversation

jyasskin
Copy link
Member

@jyasskin jyasskin commented Aug 14, 2018

This lets the browser redirect there if it doesn't recognize the version of the signed exchange.

This sits on top of #287 and #281, so please only review 99fa76f. It fixes #242.

One question here is whether to include the method with the fallback URL. We can't do anything with a non-GET method right now, and after many years of HTTP, the only acceptable methods are GET and HEAD anyway, so I think we should consider dropping the method entirely, although not in this PR.

signed-responses: Preview, Diff

Loading: Preview

This still needs a definition of request matching in a subsequent PR,
and it probably has several mistakes, which I'm hoping my reviewers will
catch.
As previously agreed with Chrome's loading team.
This makes it clearer that application/signed-exchange shouldn't parse
and reserialize the headers. It suggests that plain responses and Pushed
exchanges should serialize and reparse the headers, but they had to do
the serialization step anyway, and reparsing ensures that untrusted
headers aren't used.
The CBOR structure I was using before was an attempt to be
forward-compatible, but now that the context string changes on each
draft version, there's no need to include the item names in the message.

Fixes WICG#276.
This lets the browser redirect there if it doesn't recognize the version
of the signed exchange.
@jyasskin jyasskin requested a review from nyaxt August 14, 2018 23:54
Copy link
Collaborator

@nyaxt nyaxt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

99fa76f lgtm


1. The map mapping:
* The byte string ':method' to the byte string containing `exchange`'s
request's method.
* The byte string ':url' to the byte string containing `exchange`'s request's
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering if we'd need to fix the blow example, that includes ':url' in cbor representation.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. I'll file a separate PR for this

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Filed #290

@nyaxt nyaxt merged commit 99fa76f into WICG:master Aug 20, 2018
@jyasskin jyasskin deleted the fallback-url branch August 20, 2018 21:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fallback URLs for forward compatibility
3 participants