Properly encode location header #23133
Merged
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
jasontedor
added
:Core/Infra/REST API
jasontedor
force-pushed
the
location-encoding
branch
2 times, most recently
from
70653a5
to
4acd74f
Compare
Today when trying to encode the location header to ASCII, we rely on the Java URI API. This API requires a proper URI which blows up whenever the URI contains, for example, a space (which can happen if the type, ID, or routing contain a space). This commit addresses this issue by properly encoding the URI. Additionally, we remove the need to create a URI simplifying the code flow.
jasontedor
force-pushed
the
location-encoding
branch
from
4acd74f
to
4e62731
Compare
rjernst
approved these changes
Feb 12, 2017
| @@ -201,31 +204,43 @@ public RestStatus status() { | |||
| } | |||
|
|
|||
| /** | |||
| * Gets the location of the written document as a string suitable for a {@code Location} header. | |||
| * @param routing any routing used in the request. If null the location doesn't include routing information. | |||
| * Return the relative URI for the location of the document suitable for use in the {@code Location} header. The use of relative URIs is | |||
There was a problem hiding this comment.
nit: can you cap this at 72 columns?
There was a problem hiding this comment.
I think it's odd that we would have a 140 character limit on Java source code but only 72 characters on Javadocs (and comments?). If you feel strongly about this, I think that we should discuss it more broadly.
jasontedor
added a commit
that referenced
this pull request
Feb 13, 2017
Today when trying to encode the location header to ASCII, we rely on the Java URI API. This API requires a proper URI which blows up whenever the URI contains, for example, a space (which can happen if the type, ID, or routing contain a space). This commit addresses this issue by properly encoding the URI. Additionally, we remove the need to create a URI simplifying the code flow. Relates #23133
jasontedor
added a commit
that referenced
this pull request
Feb 13, 2017
Today when trying to encode the location header to ASCII, we rely on the Java URI API. This API requires a proper URI which blows up whenever the URI contains, for example, a space (which can happen if the type, ID, or routing contain a space). This commit addresses this issue by properly encoding the URI. Additionally, we remove the need to create a URI simplifying the code flow. Relates #23133
|
Thanks for reviewing @rjernst. |
jasontedor
added a commit
that referenced
this pull request
Feb 17, 2017
Today when trying to encode the location header to ASCII, we rely on the Java URI API. This API requires a proper URI which blows up whenever the URI contains, for example, a space (which can happen if the type, ID, or routing contain a space). This commit addresses this issue by properly encoding the URI. Additionally, we remove the need to create a URI simplifying the code flow. Relates #23133
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.
Today when trying to encode the location header to ASCII, we rely on the Java URI API. This API requires a proper URI which blows up whenever the URI contains, for example, a space (which can happen if the type, ID, or routing contain a space). This commit addresses this issue by properly encoding the URI. Additionally, we remove the need to create a URI simplifying the code flow.
Closes #23115, relates #21057