fix: url-encode the fragment of the redirect URL of the authorize response #649
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.
This patch reverts the encoding logic for the fragment of the redirect URL returned as part of the authorize response to what was the one before version
0.36.0
. In that version, the code was refactored and the keys and values of the fragment ceased to be url-encoded. This in turn reflected on all Ory Hydra versions starting from1.9.0
and provoked a breaking change that made the parsing of the fragment impossible if any of the params contain a character like&
or=
because they get treated as separators instead of as textRelated Issue or Design Document
Fixes #648
Checklist
and signed the CLA.
introduces a new feature.
vulnerability. If this pull request addresses a security vulnerability, I
confirm that I got green light (please contact
security@ory.sh) from the maintainers to push
the changes.
works.
appropriate).
Further comments
These changes reflect on Ory Hydra, partially restoring the behaviour related to the fragment encoding to the one that was before version
1.9
. The OAuth specs mandate the fragment to be encoded using theapplication/x-www-form-urlencoded
format, which is slightly different from the usual query string format: in fact, the spaces are not encoded as%20
but rather as a+
. For this reason, decoding the value in Javascript must first replace all+
signs with spaces and only then calldecodeURIComponent()
. Since before the spaces were encoded wrongly, this patch strictly speaking is still breaking the frontends. Let me know if we want to completely restore the logic or if the current implementation is fine as-is: in that case, a note should probably be added to the CHANGELOG