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

"should X be blocked" is a bad pattern #1714

Open
annevk opened this issue Sep 29, 2023 · 5 comments
Open

"should X be blocked" is a bad pattern #1714

annevk opened this issue Sep 29, 2023 · 5 comments
Labels
clarification Standard could be clearer

Comments

@annevk
Copy link
Member

annevk commented Sep 29, 2023

What is the issue with the Fetch Standard?

  1. These are not should requirements. They are musts.
  2. They return non-Infra values.
  3. The usage of headings and question marks seems to lead to some bugs, such as not being able to navigate to the caller of https://fetch.spec.whatwg.org/#should-response-to-request-be-blocked-due-to-mime-type?

Ideally we'd change Fetch, CSP, and Mixed Content around the same time so we can have this continue to be somewhat consistent.

Maybe something like:

To determine if a response response to a request request is blocked due to its MIME type:

  1. ...
  2. Return true.

@antosart @carlosjoan91 thoughts?

@annevk annevk added the clarification Standard could be clearer label Sep 29, 2023
@antosart
Copy link
Member

antosart commented Oct 2, 2023

Makes all sense to me. For CSP:

For 3. I already have a draft PR to fix the headings problems in w3c/webappsec-csp#621.
For 1. I think it would make sense for me to update that draft removing all the "should"s.

For 2., I was already struggling before. The wording 'if the result of determining whether response should be blocked by Content Security Policy given response is "Allowed"' sounds overly elaborate. If we change the result types into true or false from infra, could we just call 'if response is blocked by Content Security Policy'? And define something like

To determine whether a [=response=] <var>response</var> is
<dfn for=response export>blocked by Content Security Policy</dfn>
execute the following steps: ... 

@annevk
Copy link
Member Author

annevk commented Oct 2, 2023

Yeah, I think that's workable.

I'm not sure if for=response makes sense though. It doesn't belong to response after all, it just happens to take one. No for attribute seems fine.

Should we change CSP and Mixed Content first and then Fetch? I think that makes the most sense given the dependencies or am I missing something?

@antosart
Copy link
Member

antosart commented Oct 4, 2023

Sounds good. I'll try to move forward with the CSP change.

@antosart
Copy link
Member

antosart commented Oct 5, 2023

I made progress on CSP w3c/webappsec-csp#621.

One thing: I realized that the various "is request blocked" algorithms actually have side effects (reporting violations), hence, opposite to what I was proposing before, I have the impression that a more verbose formulation (calling "if determining whether response is blocked by Content Security Policy given response returns true") is more appropriate, since it emphasises that the algorithm does more than just returning a boolean.

@annevk
Copy link
Member Author

annevk commented Oct 5, 2023

Sounds reasonable, though if you pass response at the end the definition name should have "a response" in it I think.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clarification Standard could be clearer
Development

No branches or pull requests

2 participants