Decode a URL encoded proxy username or password #665
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
This was referenced Jun 5, 2018
|
@ab Please review when you can. Thanks! |
|
It looks like it's failing on rubocop configuration changes but all of the tests pass. I can look at this in a followup PR. |
jrafanie
added a commit
to jrafanie/manageiq
that referenced
this pull request
Oct 17, 2018
https://bugzilla.redhat.com/show_bug.cgi?id=1566615 Monkey patch RestClient for patch releases of rest-client 2.0.0. The underlying method, net_http_object, has not been modified since 2015 so it should be relatively safe to monkey patch this until the upstream PR gets merged/released. Upstream PR: rest-client/rest-client#665
jrafanie
added a commit
to jrafanie/manageiq
that referenced
this pull request
Oct 19, 2018
https://bugzilla.redhat.com/show_bug.cgi?id=1566615 Monkey patch RestClient for patch releases of rest-client 2.0.0. The underlying method, net_http_object, has not been modified since 2015 so it should be relatively safe to monkey patch this until the upstream PR gets merged/released. Upstream PR: rest-client/rest-client#665 To verify this works, run bin/rails console: Before: ``` irb(main):004:0> r = RestClient::Request.new(method: :get, url: 'http://127.0.0.1/api', proxy: 'http://%24myuser:%24%3Fxxxx@127.0.0.1') => <RestClient::Request @method="get", @url="http://127.0.0.1/api">=> <RestClient::Request @method="get", @url="https://127.0.0.1/api"> irb(main):002:0> r.net_http_object('host', 80).proxy_user => "%24myuser" irb(main):003:0> r.net_http_object('host', 80).proxy_pass => "%24%3Fxxxx" ``` After: ``` irb(main):004:0> r = RestClient::Request.new(method: :get, url: 'http://127.0.0.1/api', proxy: 'http://%24myuser:%24%3Fxxxx@127.0.0.1') => <RestClient::Request @method="get", @url="http://127.0.0.1/api">=> <RestClient::Request @method="get", @url="https://127.0.0.1/api"> irb(main):002:0> r.net_http_object('host', 80).proxy_user => "$myuser" irb(main):003:0> r.net_http_object('host', 80).proxy_pass => "$?xxxx" ```
|
Any chance this could get moving again anytime soon? |
adamruzicka
added a commit
to adamruzicka/foreman
that referenced
this pull request
Mar 24, 2021
Restclient as of version 2.1.0 uses provided credentials stored in a proxy URI object verbatim. If the credentials are escaped as they should be, then this leads to errors when we try to use the credentials because restclient sends the escaped form. There is a PR[1] in upstream restclient, which addresses this issue, but last movement there was in October 2018. ManageIQ folks decided to monkey patch restclient when they ran into the same issue, this commit is a loose adaptation of the original[1] and ManageIQ patches[2]. [1] - rest-client/rest-client#665 [2] - ManageIQ/manageiq#18105
adamruzicka
added a commit
to adamruzicka/foreman
that referenced
this pull request
Mar 24, 2021
Restclient as of version 2.1.0 uses provided credentials stored in a proxy URI object verbatim. If the credentials are escaped as they should be, then this leads to errors when we try to use the credentials because restclient sends the escaped form. There is a PR[1] in upstream restclient, which addresses this issue, but last movement there was in October 2018. ManageIQ folks decided to monkey patch restclient when they ran into the same issue, this commit is a loose adaptation of the original[1] and ManageIQ patches[2]. [1] - rest-client/rest-client#665 [2] - ManageIQ/manageiq#18105
adamruzicka
added a commit
to adamruzicka/foreman
that referenced
this pull request
Mar 24, 2021
Restclient as of version 2.1.0 uses provided credentials stored in a proxy URI object verbatim. If the credentials are escaped as they should be, then this leads to errors when we try to use the credentials because restclient sends the escaped form. There is a PR[1] in upstream restclient, which addresses this issue, but last movement there was in October 2018. ManageIQ folks decided to monkey patch restclient when they ran into the same issue, this commit is a loose adaptation of the original[1] and ManageIQ patches[2]. [1] - rest-client/rest-client#665 [2] - ManageIQ/manageiq#18105
adamruzicka
added a commit
to adamruzicka/foreman
that referenced
this pull request
Mar 24, 2021
Restclient as of version 2.1.0 uses provided credentials stored in a proxy URI object verbatim. If the credentials are escaped as they should be, then this leads to errors when we try to use the credentials because restclient sends the escaped form. There is a PR[1] in upstream restclient, which addresses this issue, but last movement there was in October 2018. ManageIQ folks decided to monkey patch restclient when they ran into the same issue, this commit is a loose adaptation of the original[1] and ManageIQ patches[2]. [1] - rest-client/rest-client#665 [2] - ManageIQ/manageiq#18105
adamruzicka
added a commit
to adamruzicka/foreman
that referenced
this pull request
Mar 24, 2021
Restclient as of version 2.1.0 uses provided credentials stored in a proxy URI object verbatim. If the credentials are escaped as they should be, then this leads to errors when we try to use the credentials because restclient sends the escaped form. There is a PR[1] in upstream restclient, which addresses this issue, but last movement there was in October 2018. ManageIQ folks decided to monkey patch restclient when they ran into the same issue, this commit is a loose adaptation of the original[1] and ManageIQ patches[2]. [1] - rest-client/rest-client#665 [2] - ManageIQ/manageiq#18105
adamruzicka
added a commit
to adamruzicka/foreman
that referenced
this pull request
Mar 24, 2021
Restclient as of version 2.1.0 uses provided credentials stored in a proxy URI object verbatim. If the credentials are escaped as they should be, then this leads to errors when we try to use the credentials because restclient sends the escaped form. There is a PR[1] in upstream restclient, which addresses this issue, but last movement there was in October 2018. ManageIQ folks decided to monkey patch restclient when they ran into the same issue, this commit is a loose adaptation of the original[1] and ManageIQ patches[2]. [1] - rest-client/rest-client#665 [2] - ManageIQ/manageiq#18105
adamruzicka
added a commit
to adamruzicka/foreman
that referenced
this pull request
Jun 10, 2021
Restclient as of version 2.1.0 uses provided credentials stored in a proxy URI object verbatim. If the credentials are escaped as they should be, then this leads to errors when we try to use the credentials because restclient sends the escaped form. There is a PR[1] in upstream restclient, which addresses this issue, but last movement there was in October 2018. ManageIQ folks decided to monkey patch restclient when they ran into the same issue, this commit is a loose adaptation of the original[1] and ManageIQ patches[2]. [1] - rest-client/rest-client#665 [2] - ManageIQ/manageiq#18105
ezr-ondrej
pushed a commit
to theforeman/foreman
that referenced
this pull request
Jun 11, 2021
Restclient as of version 2.1.0 uses provided credentials stored in a proxy URI object verbatim. If the credentials are escaped as they should be, then this leads to errors when we try to use the credentials because restclient sends the escaped form. There is a PR[1] in upstream restclient, which addresses this issue, but last movement there was in October 2018. ManageIQ folks decided to monkey patch restclient when they ran into the same issue, this commit is a loose adaptation of the original[1] and ManageIQ patches[2]. [1] - rest-client/rest-client#665 [2] - ManageIQ/manageiq#18105
Fixes rest-client#661 We already decode a URL encoded username or password using CGI.unescape, this change does the same thing for a proxy's credentials provided in a URL.
jrafanie
force-pushed
the
decode_url_encoded_proxy_user_pass
branch
from
6080346
to
35d1640
Compare
|
@ab I have pushed a commit to fix a sporadic test failure on this pull request. Even though the PR is a few years old, it still is correct and we have similar code running in ManageIQ for a long time. If you combine this PR + #759 to correct renamed rubocops, the rubocop part will no longer exit 1 and will instead exit 0 with some cop warnings to fix later on so both specs and cops will exit 0. Please let me know what we can do to get this merged. Thank you! |
Changes to the global RestClient.log were rolled back in the "before" instead of the "after", which meant, if tests made changes to the RestClient.log, those changes remained after the test. This was causing breakages when the request_spec.rb examples ran before the resource-spec.rb.
jrafanie
force-pushed
the
decode_url_encoded_proxy_user_pass
branch
from
35d1640
to
45ddcbb
Compare
|
cc @L2G @mattmanning please also review if you have a chance... thanks! |
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.
Fixes #661
We already decode a URL encoded username or password using CGI.unescape,
this change does the same thing for a proxy's credentials provided in a
URL.