Skip to content

Conversation

ninoseki
Copy link

I know it's an edge case but sometimes I need to set host header manually for a debugging purpose.

The current implementation forces to use the default host header (= authority). So it is not possible to set the host header manually.

This PR will make possible to configure host header manually if it is given via headers.

@ioquatix
Copy link
Member

Why can’t you just set the authority to whatever you want?

@ninoseki
Copy link
Author

I'm using protocol-http1 through async-http.
Then I cannot set the authority when using Async::HTTP::Internet.
I know that I can use Async::HTTP::Client instead of Async::HTTP::Internet but I think it's good to manipulate the host header in Async::HTTP::Internet.

@ioquatix
Copy link
Member

Can you explain your use case?

@ninoseki
Copy link
Author

ninoseki commented Jul 13, 2020

Sometimes I need to check an access control based on Virtual Host.
(e.g. prohibiting a direct IP access, etc.)
Then I have to manipulate the host header.

@ioquatix
Copy link
Member

It should already be possible to do this by providing the hostname to the endpoint.

	endpoint = Async::HTTP::Endpoint.parse("http://theserver.com")
	authority = "my-hostname" # The hostname to use.
	client = Async::HTTP::Client.new(endpoint, endpoint.protocol, endpoint.scheme, authority)

That being said, I think we should use kwargs for this... because it's kind of a mess specifying them all as positional arguments when you only want to override one.

@ioquatix
Copy link
Member

I know that I can use Async::HTTP::Client instead of Async::HTTP::Internet but I think it's good to manipulate the host header in Async::HTTP::Internet.

This is not a good use case. Async::HTTP::Internet is for most users and follows the principle of least surprise. If you want to specify a custom authority, it shouldn't be by the header. Authority is very special field, and it's treated differently in HTTP/1 and HTTP/2.

@ioquatix ioquatix closed this Nov 30, 2020
@ninoseki ninoseki deleted the make-host-header-configurable branch November 30, 2020 02:26
@ninoseki
Copy link
Author

Noted, thank you for reviewing my PR.

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.

2 participants