WICG / private-network-access

https://wicg.github.io/private-network-access/
Other
52 stars 21 forks source link

Could this new step in CORS preflight not be carried out earlier? #86

Closed jub0bs closed 1 year ago

jub0bs commented 1 year ago

In section 3.1.2. (entitled CORS preflight), item 4.2 reads as follows (please ignore the difference in numbering format):

  1. Immediately after checking CORS-unsafe request-header names in response against request’s header list:
    1. If request’s target IP address space is not null, then:
      1. Let allow be the result of extracting header list values given "Access-Control-Allow-Private-Network" and response’s header list.
      2. If allow is not "true", return a network error.

(my emphasis)

Is there a reason this step comes "so" late? For performance reasons, could that step not be moved, for instance, to immediately after the CORS check that takes place at step 7 of CORS-preflight fetch?

letitz commented 1 year ago

Sure? Right now, the spec says it should be inserted as step 7.8, but it could be step 7.1. I've no strong opinion about that.

jub0bs commented 1 year ago

@letitz For context, my line of thought was that processing the ACAM and ACAH headers can potentially be costly (e.g. if the server responds with long header values). We could avoid all that work if the request's issuing origin happens to resolve to a less private network and the server hasn't allowed PNA.

letitz commented 1 year ago

Fair enough! Would you care to send a PR?

jub0bs commented 1 year ago

@letitz I can certainly do that :)

letitz commented 1 year ago

Great, thanks!

letitz commented 1 year ago

This should have been closed when #90 was merged.

jub0bs commented 1 year ago

@letitz Weird, I thought I had left a comment on this... Anyway, have you created a crbug to track this change in Chromium? One reason I'm asking is that maintainers of CORS middleware may accordingly want to adjust the order in which they check things.

letitz commented 1 year ago

I was also briefly confused, having déjà vu... It's because you left the comment and I replied on the PR (#90)!

jub0bs commented 1 year ago

@letitz Sorry! All good.