HTTP::Request automatically sets http header 'Host' from uri host

[skaji]

HTTP::Request automatically sets http header 'Host' from uri host in https://github.com/sergot/http-useragent/blob/master/lib/HTTP/Request.pm6#L33

But there may be a case where uri host is not the same as http header 'Host', eg, reverse proxy http requests.

In fact, we want to use HTTP::Request in Crust https://github.com/tokuhirom/p6-Crust/pull/22 where we want to set http header 'Host' different from uri host.

If HTTP::Request is intended to be used for general purpose, then I want HTTP::Request not to set http header 'Host' from uri host.

I'd like to get your feedback, thanks!

[jonathanstowe]

Hi, In principle I agree. At minimum if the host header is set in the constructor for example then that shouldn't be over-written later (because clearly the users preference should be respected.)

I think I moved setting the Host there in the first place as there was a problem with relative redirects otherwise, but there are other ways round that I am sure.

[skaji]

@jonathanstowe Thank you for your reply!

if the host header is set in the constructor for example then that shouldn't be over-written later

Then do you accept the following change?

diff --git lib/HTTP/Request.pm6 lib/HTTP/Request.pm6
index 14dfa85..4dff3a6 100644
--- lib/HTTP/Request.pm6
+++ lib/HTTP/Request.pm6
@@ -30,7 +30,7 @@ multi method new(*%args) {
     }

     my $header = HTTP::Header.new(|%fields);
-    $header.field(Host => get-host-value($uri)) if $uri;
+    $header.field(Host => get-host-value($uri)) if $uri && !$header.field('Host');
     self.bless(:$method, :$url, :$header, :$file, :$uri);
 }

Actually I'm not sure how to handle uri() and host() methods. https://github.com/sergot/http-useragent/blob/master/lib/HTTP/Request.pm6#L54 https://github.com/sergot/http-useragent/blob/master/lib/HTTP/Request.pm6#L62-L67

[jonathanstowe]

Yeah using the Host header to pass around the host does somewhat complicate it and is rather ugly and inflexible because really we are interested in three different hosts potentially - the Host header host, the connect host and the URI host, these may all differ in different combinations under different circumstances.

But yes the change you have is good. I'll have to look more carefully at how those other methods are used to work out how best to proceed with them :)

[ugexe]

It seems a lot of this middleware type behavior could be simplified in Crust by using roles... my $request = HTTP::Request.new but role :: { multi method uri { <do something else when HTTP::Request.uri is called, and possibly continue the dispatch to the original method> }

[jonathanstowe]

I think that the stuff around the usage of the host header does need to be cleaned up somewhat irrespective of any particular use-case :)