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

Allow OPTIONS * requests to pass cookie path check for default setting #977

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jonchurch
Copy link
Member

Based on the discussion in #976

tldr; we add sessions to OPTIONS already, as long as the path matches the cookie settings, except for the rare but valid OPTIONS * request

#976 (comment)

Actually this isn't a spec issue. The spec informs that this is a valid request, and Node lets it through because of that. But whether the request matches the path we've configured is up for debate. We're comparing the path option for cookies to the request's path.

The semantics of cookie path matching and wildcard requests don't map well to each other. Given a specific path like: cookieOptions.path = "/api/v1/" we are checking that we allow everything under it's subdirs such as /api/v1/foo || /api/v1/foo/bar/baz, while blocking /api/v2/foo. So I'd consider cookieOptions.path = /api/v1/ to not match * because our cookie is set granularly.

But the default is cookieOptions.path = "/" which covers all subdirs on the domain. I'd consider that equivalent to * and would expect the options wildcard request to pass under the default.

I don't think anyone else handles this, fastify session would also not match the path for example.

So if a change was going to be taken, it should be to treat a wildcard as semantically the same as when the cookie path is set to all subdirs. Because apparently HTTP has more than one way to express "all subdirs" by way of the options wildcard request-target. TIL

(note: should anyone bother with sessions on OPTIONS? also up for debate! but the reality is rn we would dress all options requests in sessions with the default settings, except these wildcard ones)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant