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

Requests made without Accept-Encoding header #648

Comments

@JasperJuergensen
Copy link

Describe the bug

Requests made by the requests helper do not set the Accept-Encoding header (unless set by the one calling the requests helper, which is not the case for the Issuer at least). Not setting the Accept-Encoding header means, that the server can decide about content encoding, as stated in the RFCs for HTTP/1.1 (RFC7231, RFC9110):

If no Accept-Encoding header field is in the request, any content coding is considered acceptable by the user agent.

and

A request without an Accept-Encoding header field implies that the
user agent has no preferences regarding content-codings. Although
this allows the server to use any content-coding in a response, it
does not imply that the user agent will be able to correctly process
all encodings.

The requests helper does not do any decoding of compressed content in the response (and also no checks for encoded content), so that JSON decoding of encoded/compressed responses fails:

SyntaxError: Unexpected token 'd�jӚ�
                                    "... is not valid JSON
    at JSON.parse (<anonymous>)
    at IncomingMessage.get (./node-openid-client/lib/helpers/request.js:155:30)
    at processResponse (./node-openid-client/lib/helpers/process_response.js:55:25)
    at Issuer.discover (./node-openid-client/lib/issuer.js:153:18)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)

Even though most servers will not compress when the Accept-Encoding header is missing, there are cases where the server still does. An example would be the traefik server version v3.0.0-beta5 with a compression middleware enabled, as stated in their documentation about compression middlewares:

If the Accept-Encoding request header is absent, it is meant as br compression is requested.

To Reproduce
This is not really about the client/server configuration, so reproducing is easy on the library side and more complex on the server setup side.

const { Issuer } = require('openid-client');
Issuer.discover('http://openid-server.example.com');

Traefik configuration:

entryPoints:
  http:
    address: ':80'
http:
  middlewares:
    default-compress:
      compress: true
  services:
    openid-server:
      loadBalancer:
        servers:
          - url: 'http://localhost:8080'
  routers:
    openid-server:
      rule: Host(`openid-server.example.com`)
      entryPoints:
        - http
      service: openid-server
      middlewares:
        - default-compress

Steps to reproduce the behaviour:

  1. Start an openid-server behind traefik v3.0.0-beta5 with compression middleware enabled (see example traefik configuration above)
  2. Run Issuer.discover() with an URL pointing to the openid-server behind traefik

Expected behaviour
The expected behaviour would be, that the openid-client library (in particular the requests helper) is aware of content encoding / compression.
An easy fix would be to set the Accept-Encoding header to identity, which should disable encoding/compression on standard conform servers (see MDM web docs: Accept-Encoding header). Doing this fixes the problem in the scenario with traefik v.3.0.0-beta5 with the compression middleware enabled.
A more complex solution would be to add support for compression (and then set the Accept-Encoding header to compression methods supported).

Environment:

  • openid-client version: 5.6.1
  • node version: v20.10.0

Additional context
Add any other context about the problem here.

  • the bug is happening on latest openid-client too.
  • i have searched the issues tracker on github for similar issues and couldn't find anything related.
@panva
Copy link
Owner

panva commented Dec 21, 2023

@JasperJuergensen thank you

An easy fix would be to set the Accept-Encoding header to identity

That's likely the quickest route to take here.

@panva panva closed this as completed in abcb564 Dec 22, 2023
@panva panva removed the triage label Dec 22, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Mar 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.