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: retain order of headers in requests as specified #1518

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Fusl
Copy link

@Fusl Fusl commented Mar 14, 2023

As a side effect of how fasthttp handles custom headers, retaining the order of entries is already possible for most headers specified, however - and this is problematic for very old web servers that expect headers to be sent in a specific order - certain headers, such as Host and User-Agent can't be reordered due to how those are handled in a special way. This proposed change makes sure that if such headers are specified using r.Header.Set("Host", "foo") and r.Header.Set("User-Agent", "bar"), the correct order of headers is retained.

This change implements a behavior that was requested multiple times by different people in the standard net/http package:

Since the Golang maintainers have no interest in implementing this behavior or even accept an already fully functional proposed change, I've started using fasthttp and implementing this behavior into the library for myself and others to use.

Example code:

r.Header.Set("X-Foo", "bar")
r.Header.Set("User-Agent", "xyz")
r.Header.Set("X-Bar", "foo")
r.Header.Set("Host", "example.com")

Header sent with the current behavior:

Host: example.com
User-Agent: xyz
X-Foo: bar
X-Bar: foo

Header sent with the new behavior:

X-Foo: bar
User-Agent: xyz
X-Bar: foo
Host: example.com

@erikdubbelboer
Copy link
Collaborator

There seems to be a linting issue, can you fix that.

You said problematic for very old web servers, I'm curious which web servers? I understand why the Go team doesn't want to implement this extra complexity, because nobody gave a good argument for it. If you have an actual case of a web server that can't handle it that would be good to know.

@Fusl
Copy link
Author

Fusl commented Mar 15, 2023

If you have an actual case of a web server that can't handle it that would be good to know.

I can't share much information on this but I'm currently porting a very fragile 120k+ LOC application that's written in expect and talks with a very old closed source application exposing some of its API functions as HTTP endpoints. Thankfully the server speaks some kind of HTTP/1.1 but is expecting the User-Agent header to include some authentication information and the headers be ordered in a very specific order based on the type of request made.

Another system I'd like to include this library in the future in is a reverse proxy I maintain for an API where the proxy is supposed to act as transparently as possible towards the server to maintain compatibility. Modifying or reordering some of the request headers would break this convention and make the proxy no longer be transparent.

In addition, someone I worked with on this patch also mentioned that this would allow developers to avoid fingerprinting of HTTP requests and now that I think of it, it would, in my opinion, be an important feature because fingerprinting can become very evasive.

@erikdubbelboer
Copy link
Collaborator

There is a bunch of tests failing, can you have a look at that? You can run the tests locally with go test ./....

@stokito
Copy link
Contributor

stokito commented Jun 22, 2023

This sounds like a very strange and fragile feature. Introducing the contract for the fasthttp can make it harder to support a fastest speed. For example this checks, additional fields slows a little bit but also make it more difficult to understand and debug code.

Why my customer should loose speed and money for your whimsical server? In my case I have s low latency proxy with a heavy load and each millisecond is counting.

Headers fingerprinting is something weird too. TLS hides everything, isn't?

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.

None yet

3 participants