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

Client request pre-pends accept header when provided in options #153

Closed
brianpos opened this issue Jul 21, 2022 · 6 comments
Closed

Client request pre-pends accept header when provided in options #153

brianpos opened this issue Jul 21, 2022 · 6 comments

Comments

@brianpos
Copy link

If I try to provide the accept headers in the request options to a client request the client injects the application/json to the front like this:
application/json, application/fhir+json; fhirVersion=4.0, application/fhir+json

This is my test code snippit (was trying to test using the signal to abort long running requests)

        let ctrl = new AbortController();
        const client = FHIR.client(`${this.fhirServerUrl}`);
        const reqOptions : fhirclient.RequestOptions = {
           url: url,
           signal: ctrl.signal,
           headers: { "Accept": "application/fhir+json; fhirVersion=4.0, application/fhir+json" }
           };
        const r2 = await client.request<fhir4.CapabilityStatement>(reqOptions);
        console.log(r2);

The behaviour should be to either:

  • if there is an accept header in the options already, do nothing
  • if there is an accept header in the options already, append the application/json as a last option

The interpretation of the header by the server is to scan through and stop when it finds one that it supports (not try and work out what is the most specific)

@brianpos
Copy link
Author

brianpos commented Jul 21, 2022

This is the code that does it...

client-js/src/lib.ts

Lines 101 to 105 in 6daa7a5

...options,
headers: {
accept: "application/json",
...options.headers
}

@brianpos
Copy link
Author

Solved my own problem changing my code to use the same casing, then it overwrites the default provided value rather than merging (not sure which later does that)
image

@brianpos
Copy link
Author

@jmandel found this and confirmed that this is indeed the behavior of fetch, it scans case insensitive in order, hence finding the base provided one first, then the ones provided in the options.
https://fetch.spec.whatwg.org/#terminology-headers
My workaround just clobbers it, and I'm kinda ok with that, but could do the other way around so it was then processed to the end rather than start.

@brianpos
Copy link
Author

Maybe a test like this before setting the accept header.
!Object.keys(options).map(k=>k.toLowerCase()).includes("accept")

@vlad-ignatov
Copy link
Collaborator

Yes, that will need fixing. Please use lowercased header names until a fix is released.

Fetch just does the best it can by "merging" these into single value, which is slightly better than ignoring one of them. There is no reliable way to solve this though, so this library should make sure it does not happen and merge headers itself before fetch is called.

vlad-ignatov added a commit that referenced this issue Jul 21, 2022
@vlad-ignatov
Copy link
Collaborator

Should be fixed and released in v2.5.0

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

No branches or pull requests

2 participants