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

Rely on variants #360

Merged
merged 3 commits into from
Jan 7, 2019
Merged

Rely on variants #360

merged 3 commits into from
Jan 7, 2019

Conversation

jyasskin
Copy link
Member

@jyasskin jyasskin commented Jan 3, 2019

This depends on httpwg/http-extensions#744. @mnot should probably take a look.


Preview | Diff

@jyasskin jyasskin requested a review from nyaxt January 3, 2019 01:03
identifies one of those headers that enforces the integrity of the exchange's
payload.
refresh that signature. Each signature directly signs the exchange's URL and
headers and identifies one of those headers that enforces the integrity of the
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we clarify that they are response headers to align with your other edits?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. I'll do that in the morning before I merge.

@nyaxt nyaxt mentioned this pull request Jan 7, 2019
10 tasks
this returned "invalid" or didn't return a certificate chain, return
"invalid".
1. Let `exchange` be the exchange metadata and headers parsed out of `headers`.
1. If `exchange`'s request method is not safe (Section 4.2.1 of {{!RFC7231}}),
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we ban POST method by this "not safe" check. (POST is actually cachable per https://tools.ietf.org/html/rfc7231#section-4.2.3 ). Should we keep this check?

Copy link
Member Author

Choose a reason for hiding this comment

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

This line did ban POST, but since the headers can't encode a method anymore, I don't think we need to keep it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah I missed that. Thanks for the explanation :)

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.

2 participants