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

host_redirect cleans up the request port when port_redirect is not specified #17318

Open
jparklab opened this issue Jul 13, 2021 · 9 comments
Open

Comments

@jparklab
Copy link
Contributor

jparklab commented Jul 13, 2021

Description:
When host_redirect is specified but port_redirect is not, port is not preserved in the response. Envoy cleans up the port in the response(https://github.com/envoyproxy/envoy/blob/main/source/common/router/config_impl.cc#L791).
Envoy should preserve the port in the request if port_redirect is not specified or it should be clarified in the doc(https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/route/v3/route_components.proto#envoy-v3-api-msg-config-route-v3-redirectaction) that the port will be cleaned up if port_redirect is not specified.

Repro steps:

  • Configure a route to do host_redirect in a listener that listens to a port other than 80 or 443
  • Send a request to the route
curl -v --resolve redirect-test:10080:127.0.0.1 http://redirect-test:10080
* Added redirect-test:10080:127.0.0.1 to DNS cache
* Rebuilt URL to: http://redirect-test:10080/
* Hostname redirect-test was found in DNS cache
*   Trying 127.0.0.1...
* TCP_NODELAY set
* Connected to redirect-test (127.0.0.1) port 10080 (#0)
> GET / HTTP/1.1
> Host: redirect-test:10080
> User-Agent: curl/7.52.1
> Accept: */*
> 
< HTTP/1.1 301 Moved Permanently
< location: http://redirect-test.mydomain.com/
< date: Tue, 13 Jul 2021 17:33:32 GMT
< server: envoy
< content-length: 0
< 
* Curl_http_done: called premature == 0
* Connection #0 to host redirect-test left intact

Config:

static_resources:
  listeners:
  - address:
      socket_address:
        address: 0.0.0.0
        port_value: 10080
    filter_chains:
    - filters:
      - name: envoy.http_connection_manager
        config:
          codec_type: auto
          stat_prefix: ingress_http
          route_config:
            name: local_route
            virtual_hosts:
            - name: backend
              domains:
              - "*"
              routes:
              - match:
                  prefix: "/"
                redirect:
                  host_redirect: "redirect-test.mydomain.com"
          http_filters:
          - name: envoy.router
            config: {}
@jparklab jparklab added bug triage Issue requires triage labels Jul 13, 2021
@chrisxrepo
Copy link
Contributor

it lost the port

@chrisxrepo
Copy link
Contributor

you should set redirect-test.mydomain.com: 10080, it will work well

@snowp snowp added question Questions that are neither investigations, bugs, nor enhancements and removed triage Issue requires triage labels Jul 20, 2021
@github-actions
Copy link

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label Aug 19, 2021
@github-actions
Copy link

This issue has been automatically closed because it has not had activity in the last 37 days. If this issue is still valid, please ping a maintainer and ask them to label it as "help wanted" or "no stalebot". Thank you for your contributions.

@jparklab
Copy link
Contributor Author

Can we re-open this ticket? There is a workaround as I mentioned in the description, but I still believe it is unexpected to clean up the port.

@mattklein123 mattklein123 reopened this Aug 30, 2021
@mattklein123 mattklein123 added area/docs area/http help wanted Needs help! and removed question Questions that are neither investigations, bugs, nor enhancements stale stalebot believes this issue/PR has not been touched recently labels Aug 30, 2021
@choldrim
Copy link

Can we re-open this ticket? There is a workaround as I mentioned in the description, but I still believe it is unexpected to clean up the port.

@jparklab Would you show your workaround?

I met this same problem, envoy modified the location header of a 302 response, the location header was modified from '/user/login' to 'http://$svc_name/user/login', and it lost port number in $svc_name but my default svc port is not 80, so it failed.

@jparklab
Copy link
Contributor Author

@choldrim You can work around by specifying the redirect address with port as @chrisxrepo mentioned. #17318 (comment)

sunjayBhatia added a commit to sunjayBhatia/contour that referenced this issue Feb 8, 2023
Skips redirect tests that check for original request port in returned
Location header, failing likely due to:
envoyproxy/envoy#17318

Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com>
skriss pushed a commit to projectcontour/contour that referenced this issue Feb 8, 2023
Skips redirect tests that check for original request port in returned
Location header, failing likely due to:
envoyproxy/envoy#17318

Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com>
@arkodg
Copy link
Contributor

arkodg commented Feb 9, 2023

reframing the ask - as a user I would only like to set the hostname in the redirect field in the Envoy config

                redirect:
                  host_redirect: "redirect.example.com"

and would like to leave the port_redirect field empty and would like the response Location Header URL value
to contain the port section from the request.
So if my request was http://example.com:10080, the Location should be http://redirect.example.com:10080 .

This can easily be solved by statically setting the port_redirect field, but a valid use case seems to be dynamically deriving it from the request URL.

We would like such a functionality in Envoy Gateway due to the way the redirect port has been defined in the Gateway API

Port is the port to be used in the value of the Location header in the response. When empty, port (if specified) of the request is used.


@arkodg
Copy link
Contributor

arkodg commented Feb 9, 2023

hey @cpakulski can you ptal ?

yangyy93 pushed a commit to projectsesame/contour that referenced this issue Feb 16, 2023
…tour#5063)

Skips redirect tests that check for original request port in returned
Location header, failing likely due to:
envoyproxy/envoy#17318

Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com>
Signed-off-by: yy <yang.yang@daocloud.io>
yangyy93 pushed a commit to projectsesame/contour that referenced this issue Feb 16, 2023
…tour#5063)

Skips redirect tests that check for original request port in returned
Location header, failing likely due to:
envoyproxy/envoy#17318

Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com>
Signed-off-by: yy <yang.yang@daocloud.io>
vmw-yingy pushed a commit to vmw-yingy/contour that referenced this issue Feb 28, 2023
…tour#5063)

Skips redirect tests that check for original request port in returned
Location header, failing likely due to:
envoyproxy/envoy#17318

Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants