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

[Header] Making the port optional in the header "Host" #548

Closed
fullmoonissue opened this issue Feb 23, 2017 · 8 comments
Closed

[Header] Making the port optional in the header "Host" #548

fullmoonissue opened this issue Feb 23, 2017 · 8 comments

Comments

@fullmoonissue
Copy link

Hi everyone,

In my current use case, I have a nginx above the access to elasticsearch.

The host header created during the process of the request contains the port ("Host: 'website.com:80'").

This actually causes 400 Bad Request or 404 Not Found messages from nginx.

Without any port on the host header, there's no more problem.

Could we add the port into the host header in an configurable way ?

In addition, the RFC (Section 14.23) show that the port is optional.

Here is the line which causes my issue.

What do you think of it ?

Regards,

@polyfractal
Copy link
Contributor

Hm, why is your proxy returning 400/404 because of the port? Is nginx configured to accept connections on port 80, or port 9200?

The RFC says an omitted port implies the "default port for the service requested (e.g., "80" for an HTTP URL)". In this case, the service's default port is 9200, which is at-odds with the convention of port 80 being the default. It isn't a strictly HTTP/web server... it's a search engine that exposes it's api over HTTP. It makes the situation somewhat confusing :)

I'd prefer we see if the proxy configuration can be fixed. The port may be optional, but the presence of the port should always be handled correctly regardless. It would be preferable if the client remains simple in this regard, rather than doing somewhat unexpected things like dropping ports.

@fullmoonissue
Copy link
Author

Hi @polyfractal,

To complete the picture about how the request is handled, here are the steps :
=> HaProxy receive the request on the :80 port
=> These request is forwarded to Varnish on the :8080 port
=> Then, forward to Nginx on the :82 port
=> Nginx' upstream call ES on the :9200 port.

On the nginx' configuration, it listen on the *:82 port and the server_name does not contain any wilcard after the hostname.

On the one hand, the default port in this case would be correct (port 80), on the other hand the port 9200 would be called but thanks to the different "forwards".

A WIP is still in research to not being affected by the port in the header host (like a wildcard on the server_name or a direct communication between haproxy and ES).

I hope it gives you more details about this particular need and the will of configurable drop of port in the host header.

Regards,

@fullmoonissue
Copy link
Author

Hi @polyfractal,

Is there a possibility that this old PR could be merged ?
It answers perfectly on the current situation.

Your comment seem not to be interested about the request of the drop of port in the host's header but the PR is not closed yet so I wonder...

In the end, we put a patch.
It's a direct communication between HaProxy and ES (we have to bypass an infrastructure behaviour) but we can keep it in that way only temporarily.

I really need to know if a debate is able to continue or if a fork would be the ending solution.

Nice day.

Regards,

@killerwolf
Copy link

+1

@afrozenpeach
Copy link
Contributor

My company ran into a similar issue to this. We use a HAProxy instance in front of the elasticsearch server. HAProxy is configured to look at the host name in the header to know where to route requests. What I found out is that elasticsearch doesn't use CURLOPT_PORT to specify the port it's connecting to, but uses the hostname instead. While this works in most instances, it's not the most proper way of handling things.

PR #782 that I submitted last night changes the port to be in CURLOPT_PORT in $connectionParams, and stores it as a separate member variable. The current version of the PR is working, and all tests are passing.

@nicomro
Copy link

nicomro commented Sep 18, 2018

+1

1 similar comment
@Adriks976
Copy link

+1

@ezimuel
Copy link
Contributor

ezimuel commented Jul 11, 2019

Fixed with #782

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

No branches or pull requests

7 participants