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

Router QueryParameterMatcher fails to respect present_match #37951

Open
balogio opened this issue Jan 9, 2025 · 0 comments
Open

Router QueryParameterMatcher fails to respect present_match #37951

balogio opened this issue Jan 9, 2025 · 0 comments
Labels
bug triage Issue requires triage

Comments

@balogio
Copy link

balogio commented Jan 9, 2025

Title: Router QueryParameterMatcher fails to respect present_match

Description:
When configuring a route matcher to match based on the non-existence of a query_parameter, the route is incorrectly matched when present_match is set to false. This causes traffic to be routed to the incorrect upstream.

Expected Functionality: Based on the documentation, I'd expect that setting present_match: true would return true if the param key exists, and false if the param key does not exist. present_match: false should then return false if the key exists, and true if the param key does not exist.

Repro steps:
All steps are using the example config in the below config section. Overall, this is very easy to reproduce.
envoy -c Path/To/example.yaml --service-cluster test_cluster --service-node test_node --log-level debug
The config should return the following responses:
/?foo -> foo
/?bar -> bar
/?foo=1&bar=1 -> Catchall Response!
/?anyOtherKey -> Catchall Response!

But running curl against http://localhost:10000 shows a very different story:
curl 'http://localhost:10000?foo' -> Catchall Response! -- should return foo
curl 'http://localhost:10000?foo=1' -> Catchall Response! -- should return foo
curl 'http://localhost:10000?foo=1&bar=1' -> foo -- should return Catchall Response!
curl 'http://localhost:10000?bar' -> Catchall Response! -- should return bar
curl 'http://localhost:10000?bar=1' -> Catchall Response! -- should return bar
curl 'http://localhost:10000?bar=1&foo=1' -> foo -- should return Catchall Response!
curl 'http://localhost:10000?anyOtherKey' -> correctly returns Catchall Response!

To really drive this home, comment out lines 19-40 in the example config, so that you are only left with the following 2 matchers:

             - match:
                  prefix: /
                  query_parameters:
                    - name: foo
                      present_match: false
                    - name: bar
                      present_match: false
                direct_response:
                  status: 200
                  body:
                    inline_string: "foo bar, you should never see this"
              - match:
                  prefix: "/"
                direct_response:
                  status: 200
                  body:
                    inline_string: "Catchall Response!"

We should get Catchall Response! for everything even if we pass in foo and bar in our request, but that is not the case:

curl 'http://localhost:10000?foo' -> Catchall - correct
curl 'http://localhost:10000?bar' -> Catchall - correct
curl 'http://localhost:10000?foo&bar' -> foo bar, you should never see this
curl 'http://localhost:10000?bar=1&foo=1' -> foo bar, you should never see this

Config:

static_resources:
  listeners:
  - name: listener_0
    address:
      socket_address: { address: 0.0.0.0, port_value: 10000 }
    filter_chains:
    - filters:
      - name: envoy.filters.network.http_connection_manager
        typed_config:
          "@type": type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager
          codec_type: AUTO
          stat_prefix: ingress_http
          route_config:
            name: local_route
            virtual_hosts:
            - name: local_service
              domains: ["*"]
              routes:
              - match:
                  prefix: /
                  query_parameters:
                    - name: "foo"
                      present_match: true
                    - name: "bar"
                      present_match: false
                direct_response:
                  status: 200
                  body:
                    inline_string: "foo" 
              - match:
                  prefix: /
                  query_parameters:
                    - name: foo
                      present_match: false
                    - name: bar
                      present_match: true
                direct_response:
                  status: 200
                  body:
                    inline_string: "bar"
              - match:
                  prefix: /
                  query_parameters:
                    - name: foo
                      present_match: false
                    - name: bar
                      present_match: false
                direct_response:
                  status: 200
                  body:
                    inline_string: "foo bar, you should never see this"
              - match:
                  prefix: "/"
                direct_response:
                  status: 200
                  body:
                    inline_string: "Catchall Response!"
          http_filters:
          - name: envoy.filters.http.router
            typed_config:
              "@type": type.googleapis.com/envoy.extensions.filters.http.router.v3.Router

Envoy Info:

Related Issues:
This looks to be the same as #14656, which was present_match: false not being respected for HeaderMatch: #14656

@balogio balogio added bug triage Issue requires triage labels Jan 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug triage Issue requires triage
Projects
None yet
Development

No branches or pull requests

1 participant