Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Refactor client-hints, add Permissions-Policy #12

Merged
merged 9 commits into from
Feb 24, 2021
Merged

Conversation

kamermans
Copy link
Member

This PR reworks the Client-Hints / CORS-support headers so all code uses the same set of client-hints.

It also implements Permissions-Policy, the replacement for Feature-Policy as of Chrome 88
For more info see:

@kamermans kamermans added bug Something isn't working enhancement New feature or request labels Feb 22, 2021
@jonarnes
Copy link
Collaborator

Permission policy does not seem to work... It doesn't delegate the hints to imageengine. When I removed the permission haeader, and left the feature policy in, it worked. PP seems to be bleeding edge. Good catch though! Was not aware of its inclusion in v88

@jonarnes
Copy link
Collaborator

Ah. " missing.
$permissions[] = strtolower( "ch-${hint}=(\"${protocol}://${host}\")" );

@kamermans
Copy link
Member Author

Nice, thanks for testing that, I have a small update to push that cleans up some code and then I'll test it as well and we can create a new version.

@kamermans kamermans marked this pull request as ready for review February 23, 2021 18:38
@kamermans kamermans requested a review from jonarnes February 23, 2021 22:02
Copy link
Collaborator

@jonarnes jonarnes left a comment

Choose a reason for hiding this comment

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

lgtm

@jonarnes jonarnes merged commit f412356 into master Feb 24, 2021
@jonarnes jonarnes deleted the cors-rework branch February 24, 2021 08:07
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants