[rust-server] add support for '|' in path segments #667
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@bjgill thanks for the PR. Please resolve the merge conflicts and I'll merge it into master. |
|
Thanks - now ready for merge once CI is green |
A-Joshi
pushed a commit
to ihsmarkitoss/openapi-generator
that referenced
this pull request
Feb 27, 2019
* Added encoding for vertical bar character in object IDs for client requests * Updated sample
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR checklist
./bin/to update Petstore sample so that CIs can verify the change. (For instance, only need to run./bin/{LANG}-petstore.shand./bin/security/{LANG}-petstore.shif updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in.\bin\windows\.master,4.0.x. Default:master.Description of the PR
This merge request changes the characters that are encoded for object IDs to also include the vertical bar character (|). This is causing an issue with some of our microservices that are sending IDs containing the vertical bar character as part of the URL path to other services, which are responding with a
400 Bad request. Testing has shown that it is the presence of the unencoded vertical bar character that is causing this error. According to RFC 3986 this is neither unreserved nor sub-delims and so should be encoded.Tested by building the microservice that was having the issue and capturing the network traffic. The vertical bar characters are now being correctly encoded as
%7C.There is another encode set called
USERINFO_ENCODE_SET(https://docs.diesel.rs/percent_encoding/struct.USERINFO_ENCODE_SET.html) but this includes a lot of additional characters. It's possible that this set would be suitable in this situation but we were reluctant to change that much in a tool that is widely used in case it adversely impacts other users.Many thanks to @rwincewicz, the original author of this contribution on whose behalf I am submitting this upstream.