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

bug: The HTTP routing host field cannot match the port #10531

Closed
DokiDoki1103 opened this issue Nov 22, 2023 · 22 comments
Closed

bug: The HTTP routing host field cannot match the port #10531

DokiDoki1103 opened this issue Nov 22, 2023 · 22 comments
Assignees

Comments

@DokiDoki1103
Copy link

DokiDoki1103 commented Nov 22, 2023

Current Behavior

req

{
  "id": "getting-started-ip",
  "uri": "/*",
  "host":"8.130.141.31:7070",
  "upstream": {
    "type": "roundrobin",
    "nodes": {
      "10.43.242.12:7070": 1
    }
  }
}

resp

{
    "value": {
        "priority": 0,
        "uri": "/*",
        "create_time": 1700623575,
        "update_time": 1700623788,
        "upstream": {
            "pass_host": "pass",
            "nodes": {
                "10.43.242.12:7070": 1
            },
            "type": "roundrobin",
            "scheme": "http",
            "hash_on": "vars"
        },
        "id": "getting-started-ip",
        "status": 1,
        "host": "8.130.141.31:7070"
    },
    "key": "/apisix/routes/getting-started-ip"
}
图片
@shreemaan-abhishek
Copy link
Contributor

could you describe your problem in the issue description?

@shreemaan-abhishek shreemaan-abhishek added the bug Something isn't working label Nov 22, 2023
@DokiDoki1103
Copy link
Author

The HTTP routing host field does not recognize the port

@shreemaan-abhishek
Copy link
Contributor

I just tested it and I realise that the radixtree library doesn't support ip:port in host header for host based matching

@shreemaan-abhishek shreemaan-abhishek added feature-request and removed bug Something isn't working labels Nov 22, 2023
@DokiDoki1103
Copy link
Author

I think you can optimize it, thank you

@moonming
Copy link
Member

I think you can optimize it, thank you

@DokiDoki1103 Welcome to contribute PR to fix this

@theweakgod
Copy link
Contributor

I think you can optimize it, thank you

@DokiDoki1103 Welcome to contribute PR to fix this

Would it be possible for me to try it out?

@shreemaan-abhishek
Copy link
Contributor

If you want to take this up comment here: api7/lua-resty-radixtree#138 I will assign it to you.

@haoyang1994
Copy link

@DokiDoki1103 Did you try to configure host=8.130.141.31 instead of host=8.130.141.31:7070? I don't think it's a good practice to add port matching in host because I feel it can lead to confusion and it's not necessary for most cases. If what you need is server port filter, a possible workaround way is like this:

"uri": "/*",
"host": "8.130.141.31",
"vars": [
[
"server_port",
"==",
"7070"
]
]

@theweakgod
Copy link
Contributor

If you want to take this up comment here: api7/lua-resty-radixtree#138 I will assign it to you.

Ok, this does need to be modified in radixtree

@DokiDoki1103
Copy link
Author

server_port

@DokiDoki1103 Did you try to configure host=8.130.141.31 instead of host=8.130.141.31:7070? I don't think it's a good practice to add port matching in host because I feel it can lead to confusion and it's not necessary for most cases. If what you need is server port filter, a possible workaround way is like this:

"uri": "/*", "host": "8.130.141.31", "vars": [ [ "server_port", "==", "7070" ] ]

How should I configure it here when I need to use the ingress controller?

@DokiDoki1103
Copy link
Author

like this?

    - name: rbd-app-ui
      match:
        exprs:
          - subject:
              scope: ScopeHeader
              name: server_port
            op: Equal
            value: 7070
        paths:
          - /*
      backends:
        - serviceName: rbd-app-ui
          servicePort: 7070

@theweakgod
Copy link
Contributor

@DokiDoki1103 Did you try to configure host=8.130.141.31 instead of host=8.130.141.31:7070? I don't think it's a good practice to add port matching in host because I feel it can lead to confusion and it's not necessary for most cases. If what you need is server port filter, a possible workaround way is like this:

"uri": "/*", "host": "8.130.141.31", "vars": [ [ "server_port", "==", "7070" ] ]

If the host contains port, then it will match the host, otherwise it will not match. I think this design will be more suitable for you.
you can check this pr #139

@haoyang1994
Copy link

Sorry, I may not have expressed myself clearly. The port appearing in http_host and vars.server_port are sometimes not equivalent. Take the following curl command as an example. http_host is 10.70.2.23:9000, but server_port will be 8000.
curl "http://10.70.2.23:8000/api/test/1" -H 'host: 10.70.2.23:9000'

This situation may occur especially when there is a load balancer before apisix. Intuitively, one may feel that the port in the routing configuration needs to match the host in the http request header.
That's why I thought the port on the host in the routing configuration may be confusing. But anyway, it doesn't appear to be harmful, and whoever configures the port on the host should know what to expect.

@DokiDoki1103
Copy link
Author

@DokiDoki1103 Did you try to configure host=8.130.141.31 instead of host=8.130.141.31:7070? I don't think it's a good practice to add port matching in host because I feel it can lead to confusion and it's not necessary for most cases. If what you need is server port filter, a possible workaround way is like this:
"uri": "/*", "host": "8.130.141.31", "vars": [ [ "server_port", "==", "7070" ] ]

If the host contains port, then it will match the host, otherwise it will not match. I think this design will be more suitable for you. you can check this pr #139

Thank you for your PR. Indeed, this will solve my current problem

@DokiDoki1103
Copy link
Author

Sorry, I may not have expressed myself clearly. The port appearing in http_host and vars.server_port are sometimes not equivalent. Take the following curl command as an example. http_host is 10.70.2.23:9000, but server_port will be 8000. curl "http://10.70.2.23:8000/api/test/1" -H 'host: 10.70.2.23:9000'

This situation may occur especially when there is a load balancer before apisix. Intuitively, one may feel that the port in the routing configuration needs to match the host in the http request header. That's why I thought the port on the host in the routing configuration may be confusing. But anyway, it doesn't appear to be harmful, and whoever configures the port on the host should know what to expect.

If I want to access the HTTP service on a certain port, then your method is more effective, but if I want to match the port through Host, your method is not supported

@theweakgod
Copy link
Contributor

Sorry, I may not have expressed myself clearly. The port appearing in http_host and vars.server_port are sometimes not equivalent. Take the following curl command as an example. http_host is 10.70.2.23:9000, but server_port will be 8000. curl "http://10.70.2.23:8000/api/test/1" -H 'host: 10.70.2.23:9000'

This situation may occur especially when there is a load balancer before apisix. Intuitively, one may feel that the port in the routing configuration needs to match the host in the http request header. That's why I thought the port on the host in the routing configuration may be confusing. But anyway, it doesn't appear to be harmful, and whoever configures the port on the host should know what to expect.

Yes, thank you for your advice.I will take it

@theweakgod
Copy link
Contributor

Sorry, I may not have expressed myself clearly. The port appearing in http_host and vars.server_port are sometimes not equivalent. Take the following curl command as an example. http_host is 10.70.2.23:9000, but server_port will be 8000. curl "http://10.70.2.23:8000/api/test/1" -H 'host: 10.70.2.23:9000'

This situation may occur especially when there is a load balancer before apisix. Intuitively, one may feel that the port in the routing configuration needs to match the host in the http request header. That's why I thought the port on the host in the routing configuration may be confusing. But anyway, it doesn't appear to be harmful, and whoever configures the port on the host should know what to expect.

Hey, I've finished changing it. Thanks again for your advice

@haoyang1994
Copy link

I think we should think more about what effect we want to achieve:

If http_host is example.com, server_port is 8000, and the host configured in route is example.com:8000, is it be expected to match?
A more confusing example, an http request, if http_host is example.com, server_port is 8000, and the host configured in route is example.com:80, is the route be expected to match?

@haoyang1994
Copy link

Defining the host port in the route as server_port matching is actually a simpler method.

@theweakgod
Copy link
Contributor

theweakgod commented Nov 30, 2023

I think we should think more about what effect we want to achieve:

If http_host is example.com, server_port is 8000, and the host configured in route is example.com:8000, is it be expected to match? A more confusing example, an http request, if http_host is example.com, server_port is 8000, and the host configured in route is example.com:80, is the route be expected to match?

No, I don't think so.

  1. When load balancing proxy requests, you can set whether to keep the original host. If the server_port is 8000 and the route is set to "www.foo.com", you can set the Host to www.foo.com through proxy_set_header. I don't think apisix needs to handle the situation that front load balancing can handle. I don't think it is necessary to consider both the original Host and the Host after being proxied. In the http route of apisix, you only need to consider the Host in the request header when making a request, not the ip and port of the tcp connection.

  2. If you really need to match www.foo.com:8000, then you can customize a request header in the load balancing to record the original host, and set Host $proxy_header through proxy_set_header(upstream set www.foo.com:8000). Set "www.foo.com:8000" in route to match www.foo.com:8000.

  3. I think it is unnecessary to deal with the special case of www.foo.com:80, because the port of www.foo.com is 80 by default, and there is no need to handle it specially. Route set "www.foo.com" can fix the problem.

@theweakgod
Copy link
Contributor

I think we should think more about what effect we want to achieve:
If http_host is example.com, server_port is 8000, and the host configured in route is example.com:8000, is it be expected to match? A more confusing example, an http request, if http_host is example.com, server_port is 8000, and the host configured in route is example.com:80, is the route be expected to match?

No, I don't think so.

  1. When load balancing proxy requests, you can set whether to keep the original host. If the server_port is 8000 and the route is set to "www.foo.com", you can set the Host to www.foo.com through proxy_set_header. I don't think apisix needs to handle the situation that front load balancing can handle. I don't think it is necessary to consider both the original Host and the Host after being proxied. In the http route of apisix, you only need to consider the Host in the request header when making a request, not the ip and port of the tcp connection.
  2. If you really need to match www.foo.com:8000, then you can customize a request header in the load balancing to record the original host, and do not set Host $host through proxy_set_header. Set "www.foo.com:8000" in route to match www.foo.com:8000.
  3. I think it is unnecessary to deal with the special case of www.foo.com:80, because the port of www.foo.com is 80 by default, and there is no need to handle it specially.

@shreemaan-abhishek Do you think the host on port 80 needs special treatment?

@shreemaan-abhishek
Copy link
Contributor

@theweakgod I agree with you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

No branches or pull requests

5 participants