Reapply Fix escaping in the URL conversion #36949
Closed
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
facebook-github-bot
added
CLA Signed
|
This pull request was exported from Phabricator. Differential Revision: D45078923 |
Base commit: 0e5d54a |
|
This pull request was exported from Phabricator. Differential Revision: D45078923 |
cipolleschi
force-pushed
the
export-D45078923
branch
from
00b9bdb
to
5010e6b
Compare
cipolleschi
added a commit
to cipolleschi/react-native
that referenced
this pull request
Apr 23, 2023
Summary: Pull Request resolved: facebook#36949 This change is a second attempt at fixing URL encoding and escaping that was already tried [here](facebook@2b4e1f5). We had to roll it back due to some internal tests failing as it looks like Jest is manipulating the URL somehow. We manage to replicate the issue, which occur when we pre-decode a url even if it is not partially encoded (we were too aggrsssive). This fix ensure that we pre-decode the urls only if they present some `%` characters. This change should also fix facebook#28508 for good. ## Changelog: [iOS][Fixed] - Properly escape URLs Reviewed By: mdvacca Differential Revision: D45078923 fbshipit-source-id: b7154302260304291b09be5d5e7a0d43e0514ed3
|
This pull request was exported from Phabricator. Differential Revision: D45078923 |
cipolleschi
added a commit
to cipolleschi/react-native
that referenced
this pull request
Apr 23, 2023
Summary: Pull Request resolved: facebook#36949 This change is a second attempt at fixing URL encoding and escaping that was already tried [here](facebook@2b4e1f5). We had to roll it back due to some internal tests failing as it looks like Jest is manipulating the URL somehow. We manage to replicate the issue, which occur when we pre-decode a url even if it is not partially encoded (we were too aggrsssive). This fix ensure that we pre-decode the urls only if they present some `%` characters. The problem here was that the e2e tests sends some urls with some `%` symbol which does not belongs to an escape sequence. For example: `anna://launch?height=25%`. The previous code (v1) was trying to unescape this case. V2 fixes this. This change should also fix facebook#28508 for good. ## Changelog: [iOS][Fixed] - Properly escape URLs Reviewed By: mdvacca Differential Revision: D45078923 fbshipit-source-id: 0f29e94523c35e97c86daf7d710c599363d56352
cipolleschi
force-pushed
the
export-D45078923
branch
from
5010e6b
to
5a8deb5
Compare
|
This pull request was exported from Phabricator. Differential Revision: D45078923 |
cipolleschi
added a commit
to cipolleschi/react-native
that referenced
this pull request
Apr 24, 2023
Summary: Pull Request resolved: facebook#36949 This change is a second attempt at fixing URL encoding and escaping that was already tried [here](facebook@2b4e1f5). We had to roll it back due to some internal tests failing as it looks like Jest is manipulating the URL somehow. We manage to replicate the issue, which occur when we pre-decode a url even if it is not partially encoded (we were too aggrsssive). This fix ensure that we pre-decode the urls only if they present some `%` characters. The problem here was that the e2e tests sends some urls with some `%` symbol which does not belongs to an escape sequence. For example: `anna://launch?height=25%`. The previous code (v1) was trying to unescape this case. V2 fixes this. This change should also fix facebook#28508 for good. ## Changelog: [iOS][Fixed] - Properly escape URLs Reviewed By: mdvacca Differential Revision: D45078923 fbshipit-source-id: 94e882e797db879c4f1be24d163817aa60ef3776
cipolleschi
force-pushed
the
export-D45078923
branch
from
5a8deb5
to
43f6a83
Compare
|
This pull request was exported from Phabricator. Differential Revision: D45078923 |
cipolleschi
added a commit
to cipolleschi/react-native
that referenced
this pull request
Apr 24, 2023
Summary: Pull Request resolved: facebook#36949 This change is a second attempt at fixing URL encoding and escaping that was already tried [here](facebook@2b4e1f5). We had to roll it back due to some internal tests failing as it looks like Jest is manipulating the URL somehow. We manage to replicate the issue, which occur when we pre-decode a url even if it is not partially encoded (we were too aggrsssive). This fix ensure that we pre-decode the urls only if they present some `%` characters. The problem here was that the e2e tests sends some urls with some `%` symbol which does not belongs to an escape sequence. For example: `anna://launch?height=25%`. The previous code (v1) was trying to unescape this case. V2 fixes this. This change should also fix facebook#28508 for good. ## Changelog: [iOS][Fixed] - Properly escape URLs Reviewed By: mdvacca Differential Revision: D45078923 fbshipit-source-id: a09e434ac1365949f1a738c9c17111a92d55677b
cipolleschi
force-pushed
the
export-D45078923
branch
from
43f6a83
to
f677a49
Compare
|
This pull request was exported from Phabricator. Differential Revision: D45078923 |
cipolleschi
added a commit
to cipolleschi/react-native
that referenced
this pull request
Apr 25, 2023
Summary: Pull Request resolved: facebook#36949 This change is a second attempt at fixing URL encoding and escaping that was already tried [here](facebook@2b4e1f5). We had to roll it back due to some internal tests failing as it looks like Jest is manipulating the URL somehow. We manage to replicate the issue, which occur when we pre-decode a url even if it is not partially encoded (we were too aggrsssive). This fix ensure that we pre-decode the urls only if they present some `%` characters. The problem here was that the e2e tests sends some urls with some `%` symbol which does not belongs to an escape sequence. For example: `anna://launch?height=25%`. The previous code (v1) was trying to unescape this case. V2 fixes this. This change should also fix facebook#28508 for good. ## Changelog: [iOS][Fixed] - Properly escape URLs Reviewed By: mdvacca Differential Revision: D45078923 fbshipit-source-id: 6daac460dd4424131341d18c400a39ce2b605436
cipolleschi
force-pushed
the
export-D45078923
branch
from
f677a49
to
0505565
Compare
cipolleschi
added a commit
to cipolleschi/react-native
that referenced
this pull request
Apr 25, 2023
Summary: Pull Request resolved: facebook#36949 This change is a second attempt at fixing URL encoding and escaping that was already tried [here](facebook@2b4e1f5). We had to roll it back due to some internal tests failing as it looks like Jest is manipulating the URL somehow. We manage to replicate the issue, which occur when we pre-decode a url even if it is not partially encoded (we were too aggrsssive). This fix ensure that we pre-decode the urls only if they present some `%` characters. The problem here was that the e2e tests sends some urls with some `%` symbol which does not belongs to an escape sequence. For example: `anna://launch?height=25%`. The previous code (v1) was trying to unescape this case. V2 fixes this. This change should also fix facebook#28508 for good. ## Changelog: [iOS][Fixed] - Properly escape URLs Reviewed By: mdvacca Differential Revision: D45078923 fbshipit-source-id: 3e6cbd0c0532e85c31fb0b7aa3ab3cef1c5c6f6c
cipolleschi
force-pushed
the
export-D45078923
branch
from
0505565
to
1a394ba
Compare
|
This pull request was exported from Phabricator. Differential Revision: D45078923 |
Summary: Pull Request resolved: facebook#36949 This change is a second attempt at fixing URL encoding and escaping that was already tried [here](facebook@2b4e1f5). We had to roll it back due to some internal tests failing as it looks like Jest is manipulating the URL somehow. We manage to replicate the issue, which occur when we pre-decode a url even if it is not partially encoded (we were too aggrsssive). This fix ensure that we pre-decode the urls only if they present some `%` characters. The problem here was that the e2e tests sends some urls with some `%` symbol which does not belongs to an escape sequence. For example: `anna://launch?height=25%`. The previous code (v1) was trying to unescape this case. V2 fixes this. This change should also fix facebook#28508 for good. ## Changelog: [iOS][Fixed] - Properly escape URLs Reviewed By: mdvacca Differential Revision: D45078923 fbshipit-source-id: 2796d175ac21befc49afd1b940d5c2a0c93ed00b
|
This pull request was exported from Phabricator. Differential Revision: D45078923 |
cipolleschi
force-pushed
the
export-D45078923
branch
from
1a394ba
to
b307f96
Compare
|
This pull request has been merged in 5e983d5. |
jeongshin
pushed a commit
to jeongshin/react-native
that referenced
this pull request
May 7, 2023
Summary: Pull Request resolved: facebook#36949 This change is a second attempt at fixing URL encoding and escaping that was already tried [here](facebook@2b4e1f5). We had to roll it back due to some internal tests failing as it looks like Jest is manipulating the URL somehow. We manage to replicate the issue, which occur when we pre-decode a url even if it is not partially encoded (we were too aggrsssive). This fix ensure that we pre-decode the urls only if they present some `%` characters. The problem here was that the e2e tests sends some urls with some `%` symbol which does not belongs to an escape sequence. For example: `anna://launch?height=25%`. The previous code (v1) was trying to unescape this case. V2 fixes this. This change should also fix facebook#28508 for good. ## Changelog: [iOS][Fixed] - Properly escape URLs Reviewed By: mdvacca Differential Revision: D45078923 fbshipit-source-id: 010a5c173784f8341a1a08bcbd06a6ad14299c75
OlimpiaZurek
pushed a commit
to OlimpiaZurek/react-native
that referenced
this pull request
May 22, 2023
Summary: Pull Request resolved: facebook#36949 This change is a second attempt at fixing URL encoding and escaping that was already tried [here](facebook@2b4e1f5). We had to roll it back due to some internal tests failing as it looks like Jest is manipulating the URL somehow. We manage to replicate the issue, which occur when we pre-decode a url even if it is not partially encoded (we were too aggrsssive). This fix ensure that we pre-decode the urls only if they present some `%` characters. The problem here was that the e2e tests sends some urls with some `%` symbol which does not belongs to an escape sequence. For example: `anna://launch?height=25%`. The previous code (v1) was trying to unescape this case. V2 fixes this. This change should also fix facebook#28508 for good. ## Changelog: [iOS][Fixed] - Properly escape URLs Reviewed By: mdvacca Differential Revision: D45078923 fbshipit-source-id: 010a5c173784f8341a1a08bcbd06a6ad14299c75
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.
Summary:
This change is a second attempt at fixing URL encoding and escaping that was already tried here.
We had to roll it back due to some internal tests failing as it looks like Jest is manipulating the URL somehow.
We manage to replicate the issue, which occur when we pre-decode a url even if it is not partially encoded (we were too aggrsssive).
This fix ensure that we pre-decode the urls only if they present some
%characters.This change should also fix #28508 for good.
Changelog:
[iOS][Fixed] - Properly escape URLs
Differential Revision: D45078923