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

http-log plugin: Host header not including port #13067

Closed
1 task done
dingjiayi opened this issue May 23, 2024 · 3 comments · Fixed by #13116
Closed
1 task done

http-log plugin: Host header not including port #13067

dingjiayi opened this issue May 23, 2024 · 3 comments · Fixed by #13116
Labels

Comments

@dingjiayi
Copy link
Contributor

Is there an existing issue for this?

  • I have searched the existing issues

Kong version ($ kong version)

Kong 3.6

Current Behavior

  • use http-log plugin with config http_endpoint="http://host_name:8080/log"

  • endpoint receive request with host head: Host: host_name

Expected Behavior

endpoint receive request with host head: Host: host_name:8080

The "Host" header field in a request provides the host and port information from the target URI ...

From the RFC , we can see the Host header should include the port.
If no port is included, the default port for the service requested is implied (e.g., 443 for an HTTPS URL, and 80 for an HTTP URL).

Steps To Reproduce

No response

Anything else?

No response

@dingjiayi dingjiayi changed the title http-log plugin: Host header including port http-log plugin: Host header not including port May 23, 2024
@ADD-SP
Copy link
Contributor

ADD-SP commented May 27, 2024

@dingjiayi Thanks for your report. I'd like to clarify one thing.

endpoint receive request with host head: Host: host_name:8080

Do you mean the Host header as part of the Kong request log sent by the http-log plugin? Or the Host header of the request that was used to send the Kong request log?

My understanding is that you configured the http-log plugins using http_endpoint="http://host_name:8080/log", and the logging server received a logging request from this plugin, and the Host header of this request is host_name instead of host_name:8080. Am I right?

@ADD-SP ADD-SP added the pending author feedback Waiting for the issue author to get back to a maintainer with findings, more details, etc... label May 27, 2024
@dingjiayi
Copy link
Contributor Author

@ADD-SP

My understanding is that you configured the http-log plugins using http_endpoint="http://host_name:8080/log", and the logging server received a logging request from this plugin, and the Host header of this request is host_name instead of host_name:8080. Am I right?

Right. Sorry for my unclear description.

This is caused by line 117 ["Host"] = host, as blow

local headers = {
["Host"] = host,
["Content-Type"] = content_type,
["Content-Length"] = content_length,
["Authorization"] = userinfo and "Basic " .. encode_base64(userinfo) or nil
}

If it's confirmed to be a bug, I would be willing to submit a PR to fix it.

I think it could be resolved by directly removing the code on line 117. Just like many other HTTP client libraries, lua-resty-http automatically sets the Host header in the underlying layer when the upper-layer request doesn't have a Host header set.

@ADD-SP ADD-SP added bug and removed pending author feedback Waiting for the issue author to get back to a maintainer with findings, more details, etc... labels May 27, 2024
@ADD-SP
Copy link
Contributor

ADD-SP commented May 27, 2024

@ADD-SP

My understanding is that you configured the http-log plugins using http_endpoint="http://host_name:8080/log", and the logging server received a logging request from this plugin, and the Host header of this request is host_name instead of host_name:8080. Am I right?

Right. Sorry for my unclear description.

This is caused by line 117 ["Host"] = host, as blow

local headers = {
["Host"] = host,
["Content-Type"] = content_type,
["Content-Length"] = content_length,
["Authorization"] = userinfo and "Basic " .. encode_base64(userinfo) or nil
}

If it's confirmed to be a bug, I would be willing to submit a PR to fix it.

I think it could be resolved by directly removing the code on line 117. Just like many other HTTP client libraries, lua-resty-http automatically sets the Host header in the underlying layer when the upper-layer request doesn't have a Host header set.

I think this is a bug, PR is welcome. @dingjiayi

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants