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

Feature Request: Support adding/removing query params #2098

Open
sethpollack opened this issue Nov 22, 2017 · 4 comments
Open

Feature Request: Support adding/removing query params #2098

sethpollack opened this issue Nov 22, 2017 · 4 comments
Labels
area/http beginner Good starter issues! help wanted Needs help!

Comments

@sethpollack
Copy link

In Nginx you can modify query params with something like set $args $args&api_key=foo;

We can do something similar with query_params_to_add and query_params_to_remove

@mattklein123 mattklein123 added beginner Good starter issues! enhancement Feature requests. Not bugs or questions. help wanted Needs help! labels Nov 22, 2017
Shikugawa pushed a commit to Shikugawa/envoy that referenced this issue Mar 28, 2020
Signed-off-by: Lizan Zhou <lizan@tetrate.io>
@dalegaspi
Copy link

dalegaspi commented Mar 26, 2021

i just assumed this is supported out of the box. 😞

is there a plan to get this feature added?

edit: ok i was able to do this using a Lua filter and I suppose for now this will do.

http_filters:
  - name: envoy.filters.http.lua
    typed_config:
      '@type': type.googleapis.com/envoy.config.filter.http.lua.v2.Lua
      inline_code: |
        function envoy_on_request(request_handle)
          path = request_handle:headers():get(":path")
          request_handle:headers():replace(":path", (path .. "&my-param=yeet"))
        end
  - name: envoy.router

note that the above code is very fragile since it assumes it's a GET, and assumes there's existing params, et cetera, et cetera...

@mattklein123 mattklein123 added area/http and removed enhancement Feature requests. Not bugs or questions. labels Mar 26, 2021
jpsim added a commit that referenced this issue Nov 28, 2022
Description: [StringJoiner](https://developer.android.com/reference/java/util/StringJoiner) was added in Android API level 24 but we still support back to API level 21, so this results in `NoClassDefFoundError` errors on devices running older Android versions.

Risk Level: Medium, this code is simple and unit tested.
Testing: Existing unit tests
Docs Changes: None
Release Notes: Added

Signed-off-by: JP Simard <jp@jpsim.com>
jpsim added a commit that referenced this issue Nov 29, 2022
Description: [StringJoiner](https://developer.android.com/reference/java/util/StringJoiner) was added in Android API level 24 but we still support back to API level 21, so this results in `NoClassDefFoundError` errors on devices running older Android versions.

Risk Level: Medium, this code is simple and unit tested.
Testing: Existing unit tests
Docs Changes: None
Release Notes: Added

Signed-off-by: JP Simard <jp@jpsim.com>
@siddharthkhonde
Copy link

hi. is this still open? I can take a look into this as a beginner.

@derekargueta
Copy link
Member

@alyssawilk @mattklein123 started working on this issue implementing it within the router similar to request_headers_to_add/remove but had a thought - would it be better to implement this as an HTTP filter? This thought occurred to me since the router is already fairly complex and having it be an HTTP filter might be better encapsulated and lower overhead if not used.

@mattklein123
Copy link
Member

@alyssawilk @mattklein123 started working on this issue implementing it within the router similar to request_headers_to_add/remove but had a thought - would it be better to implement this as an HTTP filter? This thought occurred to me since the router is already fairly complex and having it be an HTTP filter might be better encapsulated and lower overhead if not used.

I don't have a strong opinion. I think it's fine to build it in as I think that is what most people would expect. There shouldn't be any overhead of not used I don't think. Thanks for implementing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/http beginner Good starter issues! help wanted Needs help!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants