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

zuul proxy changes order of query params #989

Closed
wants to merge 1 commit into from
Closed

zuul proxy changes order of query params #989

wants to merge 1 commit into from

Conversation

nbyl
Copy link
Contributor

@nbyl nbyl commented Apr 26, 2016

When forwarding requests using the SimpleHostRoutingFilter query parameters are first parsed to a map and afterwards reconstructed when forwarding. This leads sometimes to re-ordering and the query ?edit&p1=v1 may accidentally be changed to ?p1=v1&edit.

This should usually not be a problem, but some legacy applications expect the parameters in the query string to have a certain order. The patches re-uses the original query string when forwarding requests to ensure compatibility with these applications.

@dsyer dsyer closed this in 8952cff Apr 26, 2016
@dsyer dsyer added this to the 1.1.0 milestone Apr 26, 2016
@dsyer dsyer reopened this Apr 28, 2016
@dsyer
Copy link
Contributor

dsyer commented Apr 28, 2016

I had to revert this change because it caused some test failures. Can you look into it please?

@nbyl
Copy link
Contributor Author

nbyl commented May 13, 2016

sorry for the long reply time. I have looked into the issue and there is a conflict between the "classic" behaviour (parse and filter the query parameters) and the behaviour required in this use case (pass the query string through as it is).

I'm currently working on a proposal, which should be done next week.

@dsyer dsyer removed this from the 1.1.0 milestone Jun 9, 2016
@spencergibb
Copy link
Member

ping @nbyl

@nbyl
Copy link
Contributor Author

nbyl commented Jul 29, 2016

@spencergibb Sorry for the delay. I have checked the code but the problem is currently not solvable. The parameters are mixed together inside the netflix stack. They are passed back as a map containing no information about the original order. So there is currently no way to solve this in SimpleHostRoutingFilter as I have tried here.

We have solved this using custom zuul filter that bypasses all filtering done insinde the current code and forwards the stuff directly to the target. Are you interested to have this feature in the project?

@jebeaudet
Copy link
Contributor

jebeaudet commented Aug 2, 2016

Related to my last comment here #971, I believe this should be merged (UTs failing are most likely related to query string encoding of #682, but I haven't verified so I might be wrong).

@spencergibb
Copy link
Member

Closing

there is currently no way to solve this in SimpleHostRoutingFilter as I have tried here

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

4 participants