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

Do not overwrite original Host header if was provided #34

Closed
wants to merge 1 commit into from

Conversation

dex4er
Copy link

@dex4er dex4er commented Jan 24, 2014

It is unfortunate, that HTTP::Tiny overwrites Host header. Usually it is the same as host:port, but in some cases there is a need to set the Host header to something different.

Please allow to change this header.

@dagolden
Copy link

Thank you for the patch. However, as I understand the HTTP 1.1 spec, section 14.23, the current behavior is correct:

The Host field value MUST represent the naming authority of the origin server or gateway given by the original URL.

Can you explain your case that needs to be different and whether you think that would still be compliant with the specification?

@dex4er
Copy link
Author

dex4er commented Jan 24, 2014

https://github.com/leedo/Plack-App-Proxy/blob/master/t/basic.t overrides original Host header. I've tried to replace LWP::UserAgent with HTTP::Tiny and I've noticed that Host header is always overwritten with host:port taken from URL.

I think that non-standard Host header can be useful ie. for reverse proxying and other unusual techniques.
I would like to use non-standard HTTP client as an internal backend of Plack::App::Proxy.

Perhaps HTTP::Tiny->new constructor could allow to set another option like preserve_host_header and set the standard Host header unless this option is used explicitly?

@dex4er
Copy link
Author

dex4er commented Jan 24, 2014

My changes for Plack::App::Proxy https://github.com/dex4er/perl-Plack-App-Proxy/tree/feature/no_lwp_depends
The t/basic.t hangs up with unmodified HTTP::Tiny.

@dagolden
Copy link

Ah. If you're looking to implement a non-standard client, then I think the right thing to do is to subclass HTTP::Tiny. You just need to override the _prepare_headers_and_cb method to save your original header field, call the superclass method, and restore the host header afterwards.

I'm going to close the ticket, as strict "MUST" spec compliance is a design goal of HTTP::Tiny and what you want is counter to the spec (albeit for a good reason, just not one that HTTP::Tiny will implement).

@dagolden dagolden closed this Jan 24, 2014
@dex4er
Copy link
Author

dex4er commented Jan 24, 2014

Ok, it does work for t/basic.t from Plack::App::Proxy. I'm attaching code snippet because the solution is not so obvious:

package My::HTTP::Tiny;

use base 'HTTP::Tiny';

# Preserve Host header
sub _prepare_headers_and_cb {
    my ($self, $request, $args, $url, $auth) = @_;
    my $host;
    while (my ($k, $v) = each %{$args->{headers}}) {
        $host = $v if lc $k eq 'host';
    }
    $self->SUPER::_prepare_headers_and_cb($request, $args, $url, $auth);
    $request->{headers}{'host'} = $host if $host;
    return;
}

@dagolden
Copy link

Great! I'm glad it was simple (even if not obvious).

@Flimm
Copy link

Flimm commented Jul 16, 2014

I separately stumbled upon this problem, it didn't occur to search for duplicates until after I submitted my pull request. Any way, for the record, here's the URL of the duplicate:

#44

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.

3 participants