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

Fix upstream default port when HTTP_PROXY #1440

Merged
merged 6 commits into from
Feb 5, 2024
Merged

Conversation

eguzki
Copy link
Member

@eguzki eguzki commented Jan 31, 2024

What

When the gateway is configured to use http_proxy (i.e proxy between APIcast and some plain HTTP/1.1 upstream), and the port is not specified in api_backend service attribute, the request going from APIcast has a wrong absolute URI path

GET http://www.example.org:nil/pub/WWW/TheProject.html HTTP/1.1

Note that the port in the request line is nil.

The issue is not happening for https_proxy based connections. It turns out that the camel use case is affected for https_proxy use case.

Verification Steps

  • Build docker image from this git branch
make runtime-image IMAGE_NAME=apicast-test
  • Run http_proxy dev environment
cd dev-environments/http-proxy-plain-http-upstream/
make gateway IMAGE_NAME=apicast-test
  • Send request to APIcast
curl --resolve get.example.com:8080:127.0.0.1 -v "http://get.example.com:8080/?user_key=123"

Request should return 200

* Added get.example.com:8080:127.0.0.1 to DNS cache
* Hostname get.example.com was found in DNS cache
*   Trying 127.0.0.1:8080...
* Connected to get.example.com (127.0.0.1) port 8080 (#0)
> GET /?user_key=123 HTTP/1.1
> Host: get.example.com:8080
> User-Agent: curl/7.81.0
> Accept: */*
> 
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
< Server: openresty
< Date: Wed, 31 Jan 2024 22:44:40 GMT
< Content-Type: application/json
< Content-Length: 251
< Connection: keep-alive
< Via: 1.1 tinyproxy (tinyproxy/1.11.1)
< Access-Control-Allow-Origin: *
< Access-Control-Allow-Credentials: true
< 
{
  "args": {
    "user_key": "123"
  }, 
  "headers": {
    "Accept": "*/*", 
    "Connection": "close", 
    "Host": "example.com", 
    "User-Agent": "curl/7.81.0"
  }, 
  "origin": "192.168.64.3", 
  "url": "http://example.com/get?user_key=123"
}
* Connection #0 to host get.example.com left intact
  • Inspect the traffic between APIcast and the proxy
docker compose -p http-proxy-plain-http-upstream logs -f proxy

The request path should not contain nil port

proxy  | GET http://example.com/get?user_key=123 HTTP/1.1\r

@eguzki eguzki changed the title Fix upstrem default port when HTTP_PROXY Fix upstream default port when HTTP_PROXY Jan 31, 2024
@eguzki eguzki marked this pull request as ready for review January 31, 2024 23:02
@eguzki eguzki requested a review from a team as a code owner January 31, 2024 23:02
@eguzki eguzki requested a review from tkan145 January 31, 2024 23:02
@eguzki
Copy link
Member Author

eguzki commented Feb 2, 2024

It turns out that the camel use case is affected. Fix added.

@eguzki eguzki requested a review from tkan145 February 2, 2024 14:07
@tkan145
Copy link
Contributor

tkan145 commented Feb 5, 2024

LGTM.

Please also update the CHANGELOG file

@eguzki
Copy link
Member Author

eguzki commented Feb 5, 2024

Added CHANGELOG entry. Since there are new commits in the PR, I need new approval to be fair.

@tkan145
Copy link
Contributor

tkan145 commented Feb 5, 2024

+1 feel free to merge when you are ready

@eguzki eguzki merged commit 3e561f0 into master Feb 5, 2024
11 checks passed
@eguzki eguzki deleted the fix-upstrem-default-port branch February 5, 2024 12:26
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.

None yet

2 participants