Skip to content

cache-control: no-cache: use quoted-string form #4177

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

alxndrsn
Copy link
Contributor

@alxndrsn alxndrsn commented Apr 23, 2025

Update comment example of qualified no-cache directives (no-cache=...) to use the quoted-string form instead of the token form.

Per https://datatracker.ietf.org/doc/html/rfc9111#name-no-cache-2:

This directive uses the quoted-string form of the argument syntax. A sender SHOULD NOT generate the token form (even if quoting appears not to be needed for single-entry lists).

This relates to...

cache-control response header's qualified no-cache directive.

Rationale

Seems more correct to follow spec recommendations in documentation.

Support for token form seems to be deliberate:

test('no-cache with headers', () => {
let directives = parseCacheControlHeader('max-age=10, no-cache=some-header, only-if-cached')
deepStrictEqual(directives, {
'max-age': 10,
'no-cache': [
'some-header'
],
'only-if-cached': true
})
directives = parseCacheControlHeader('max-age=10, no-cache="some-header", only-if-cached')
deepStrictEqual(directives, {
'max-age': 10,
'no-cache': [
'some-header'
],
'only-if-cached': true
})
directives = parseCacheControlHeader('max-age=10, no-cache="some-header, another-one", only-if-cached')
deepStrictEqual(directives, {
'max-age': 10,
'no-cache': [
'some-header',
'another-one'
],
'only-if-cached': true
})
})

Changes

Update comment and test examples of qualified no-cache directives (no-cache=...) to use the quoted-string form instead of the token form.

Features

N/A

Bug Fixes

N/A

Breaking Changes and Deprecations

Should not break anything, as no code changed.

Status

alxndrsn added 3 commits April 23, 2025 05:13
Update comment and test examples of qualified no-cache directives (no-cache=...) to use the quoted-string form instead of the token form.

Per https://httpwg.org/specs/rfc9111.html#cache-response-directive.no-cache:

> This directive uses the quoted-string form of the argument syntax. A sender SHOULD NOT generate the token form (even if quoting appears not to be needed for single-entry lists).
@alxndrsn
Copy link
Contributor Author

The no-cache=... parsing code seems to go out of its way to support various different token or mixed token/quoted-string formats:

test('no-cache with headers', () => {
let directives = parseCacheControlHeader('max-age=10, no-cache=some-header, only-if-cached')
deepStrictEqual(directives, {
'max-age': 10,
'no-cache': [
'some-header'
],
'only-if-cached': true
})
directives = parseCacheControlHeader('max-age=10, no-cache="some-header", only-if-cached')
deepStrictEqual(directives, {
'max-age': 10,
'no-cache': [
'some-header'
],
'only-if-cached': true
})
directives = parseCacheControlHeader('max-age=10, no-cache="some-header, another-one", only-if-cached')
deepStrictEqual(directives, {
'max-age': 10,
'no-cache': [
'some-header',
'another-one'
],
'only-if-cached': true
})
})

Would it be better to only allow quote-string format?

@alxndrsn alxndrsn marked this pull request as ready for review April 23, 2025 05:45
@mcollina
Copy link
Member

Would it be better to only allow quote-string format?

@flakey5 was this due to the compatibility tests?

@flakey5
Copy link
Member

flakey5 commented Apr 23, 2025

@flakey5 was this due to the compatibility tests?

Yes. If this isn't wanted behavior, the test cases for it in the caching suite will need to be added to the ignoredTests list in test/cache-interceptor/cache-tests.mjs

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

Successfully merging this pull request may close these issues.

3 participants