Skip to content

fix: accept the first encoding when no Accept-Encoding is present - #83

Open
krzysdz wants to merge 2 commits into
jshttp:masterfrom
krzysdz:default-accept-any-encoding
Open

krzysdz wants to merge 2 commits into
jshttp:masterfrom
krzysdz:default-accept-any-encoding

Conversation

@krzysdz

@krzysdz krzysdz commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #82

typeof this.headers['accept-encoding'] !== 'string' was chosen as condition, because:

  • !this.headers['accept-encoding'] is true also for an empty header (empty string), which means that only identity encoding is accepted (empty header != no header)
  • !Object.hasOwn(this.headers, 'accept-encoding') does not play well with the tests, because the property (in tests) always exists and is undefined when simulating a "nonexistent" header

Note

Another possibility would be returning 'identity' if it exists in encodings (highest priority) like suggested in RFC 2616, but probably there is a reason why RFC 7231 and RFC 9110 removed this wording.

The more I look into this, the more I think it may be better to change it on the negotiator side.

Comment thread index.js Outdated
@krzysdz
krzysdz marked this pull request as draft October 4, 2026 02:09
@krzysdz
krzysdz marked this pull request as ready for review October 5, 2026 01:56
krzysdz added a commit to krzysdz/express that referenced this pull request Oct 5, 2026
…not present

The new acceptsEncodings test will fail. I consider this to be a bug in jshttp/accepts.

jshttp/accepts#83 (comment)
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.

No Accept-Encoding defaults to only 'identity'

1 participant