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.

Sign up for GitHub

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

Jump to bottom

Keep headers order while sending them #1569

Open
AlttiRi opened this issue May 31, 2022 · 3 comments · May be fixed by #1662
Open

Keep headers order while sending them #1569

AlttiRi opened this issue May 31, 2022 · 3 comments · May be fixed by #1662
Labels
bug

Comments

@AlttiRi
Copy link

AlttiRi commented May 31, 2022 •
edited

Bug

It sorts headers (except default "Host" one).

Expected behavior

It should not sort the headers while sending them (while performing a network request).

Example

import nodeFetch from "node-fetch";

const url = "http://127.0.0.1:3000/";
const headers = {
    "xxx-3": "3",
    "xxx-2": "2",
    "xxx-1": "1",
}
console.log(Object.keys(headers));
console.log(await (await nodeFetch(url, {headers})).text());

It sends (You can check it with a HTTP server that logs the input request headers)

{
  accept: '*/*',
  'accept-encoding': 'gzip,deflate,br',
  connection: 'close',
  'user-agent': 'node-fetch',
  'xxx-1': '1',
  'xxx-2': '2',
  'xxx-3': '3',
  host: '127.0.0.1:3000'
}

Headers were sorted.
"host" header were not sorted. (It keeps the append order.)

Additional comment

If you want to simulate Headers object behavior you should sort the headers only when a user iterates through them, but not when you send the request.

nodejs/undici#1309 (comment)

More over, not all headers were sorted ("host" that appends before sending, if you did not set it manually).
It just makes a useless work (sorting).

Info

  • node-fetch v3.2.3
@AlttiRi AlttiRi added the bug label May 31, 2022
@AlttiRi
Copy link
Author

AlttiRi commented May 31, 2022 •
edited

Also it think it should send (by default) "accept-encoding": "gzip, deflate, br" instead of "accept-encoding": "gzip,deflate,br".

Like it browsers do.
They separate encoding with ", ", not just with ",".

Also it would more correct to prepend "host" header, instead appending.
It's usually the first header.

This was referenced May 31, 2022
Open
Closed
@jimmywarting
Copy link
Collaborator

I have been planing to make a switch to a more standard/spec'ed implementation of the Header class and use fetch-headers instead... it will sort the headers when iteration over them. it also runs the test against WPT own test cases of making sure that it is up to spec.

@AlttiRi
Copy link
Author

AlttiRi commented May 31, 2022

Yes, when a user code iterates a Header object the iterator returns the headers in sorted order.
But the inner state keeps the order in which the headers were added. In this order the headers are send in TCP socket.

Keeping the headers order is required when a user want to make exactly the same requests as it do other libs, browsers.

@JVBorges JVBorges linked a pull request Oct 17, 2022 that will close this issue
Open
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Remove header sorting when iterates over it JVBorges/node-fetch
2 participants
@jimmywarting @AlttiRi