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

Add Cross-Origin-Opener/Embedder-Policy response headers #2008

Merged
merged 1 commit into from
Jan 18, 2021

Conversation

pgjones
Copy link
Member

@pgjones pgjones commented Jan 18, 2021

These header can only have a few distinct values, hence the use of an
enum. They are more recent headers, and make it easier to control
these policies (e.g. not having to write noopener everywhere). This
code makes it a little easier to use the headers correctly - as the
Enum fixes the values (preventing typos, mistakes etc).

https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Cross-Origin-Embedder-Policy
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Cross-Origin-Opener-Policy

  • Add tests that demonstrate the correct behavior of the change. Tests should fail without the change.
  • Add or update relevant docs, in the docs folder and in code.
  • Add an entry in CHANGES.rst summarizing the change and linking to the issue.
  • Add .. versionchanged:: entries in any relevant code docs.
  • Run pre-commit hooks and fix any issues.
  • Run pytest and tox, no tests failed.

Please merge before #2005 if this is good to merge.

@davidism
Copy link
Member

Looks good overall. We're not using enums anywhere else, and there's no reference to them in the property docstrings, is it worth adding them right now?

@pgjones
Copy link
Member Author

pgjones commented Jan 18, 2021

We're not using enums anywhere else

I think these are the only headers that are so constrained, can't think of when this would otherwise come up. I've updated the docstrings.

I think these are helpful (I intend to make use of them).

src/werkzeug/wrappers/response.py Outdated Show resolved Hide resolved
src/werkzeug/wrappers/response.py Outdated Show resolved Hide resolved
CHANGES.rst Outdated Show resolved Hide resolved
@davidism
Copy link
Member

I was thinking about the cookie samesite property, I'm sure there's others too. If we do plan to add more, we might want to move them to an werkzeug.http.enums module. Anyway, don't need to add or move them now, just thinking forward. Renames/moves will be way easier in the future thanks to module-level __getattr__ in Python 3.7, so I'm not too worried.

These header can only have a few distinct values, hence the use of an
enum. They are more recent headers, and make it easier to control
these policies (e.g. not having to write noopener everywhere). This
code makes it a little easier to use the headers correctly - as the
Enum fixes the values (preventing typos, mistakes etc).

https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Cross-Origin-Embedder-Policy
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Cross-Origin-Opener-Policy
@pgjones
Copy link
Member Author

pgjones commented Jan 18, 2021

I see, that makes sense. I've update this - thanks for the suggestions.

@davidism davidism merged commit cf50110 into pallets:master Jan 18, 2021
@pgjones pgjones deleted the cooep branch January 18, 2021 18:05
@pgjones
Copy link
Member Author

pgjones commented Jan 18, 2021

Thanks!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants