You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by GitBox <gi...@apache.org> on 2022/04/13 12:47:21 UTC

[GitHub] [thrift] pspeter opened a new pull request, #2565: Fix bug in THttpClient proxy handling

pspeter opened a new pull request, #2565:
URL: https://github.com/apache/thrift/pull/2565

   Previously, creating an instance of this class would throw whenever the http(s)_proxy environment variables were set, since it tried concatenating a string with a bytes array.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@thrift.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [thrift] fishy commented on a diff in pull request #2565: Fix bug in Python's THttpClient proxy handling

Posted by GitBox <gi...@apache.org>.
fishy commented on code in PR #2565:
URL: https://github.com/apache/thrift/pull/2565#discussion_r853294124


##########
lib/py/src/transport/THttpClient.py:
##########
@@ -100,7 +100,7 @@ def basic_proxy_auth_header(proxy):
         ap = "%s:%s" % (urllib.parse.unquote(proxy.username),
                         urllib.parse.unquote(proxy.password))
         cr = base64.b64encode(ap.encode()).strip()
-        return "Basic " + cr
+        return "Basic " + cr.decode()

Review Comment:
   this is a python 2 vs. 3 issue. the old code works in python 2 and the new code works in python 3. currently we still officially support both so you probably need to find a way to make it work in both 2 and 3 (maybe `six` can help in this case?)
   
   we do have a plan to drop python 2 support, so that won't be needed after we do that. but we just don't have the time to actually work on that yet (you are welcomed to take that on :) https://issues.apache.org/jira/browse/THRIFT-5537)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [thrift] pspeter commented on a diff in pull request #2565: Fix bug in Python's THttpClient proxy handling

Posted by GitBox <gi...@apache.org>.
pspeter commented on code in PR #2565:
URL: https://github.com/apache/thrift/pull/2565#discussion_r853824427


##########
lib/py/src/transport/THttpClient.py:
##########
@@ -100,7 +100,7 @@ def basic_proxy_auth_header(proxy):
         ap = "%s:%s" % (urllib.parse.unquote(proxy.username),
                         urllib.parse.unquote(proxy.password))
         cr = base64.b64encode(ap.encode()).strip()
-        return "Basic " + cr
+        return "Basic " + cr.decode()

Review Comment:
   Ah, that makes sense. I'll see if I can make this work using `six`.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [thrift] pspeter commented on pull request #2565: Fix bug in Python's THttpClient proxy handling

Posted by GitBox <gi...@apache.org>.
pspeter commented on PR #2565:
URL: https://github.com/apache/thrift/pull/2565#issuecomment-1103631303

   @fishy the return type of the function should now always be `str` in Python 2 and 3, so it is consistent to the current behavior in Python2 while also fixing the function in Python 3.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [thrift] fishy commented on pull request #2565: Fix bug in Python's THttpClient proxy handling

Posted by GitBox <gi...@apache.org>.
fishy commented on PR #2565:
URL: https://github.com/apache/thrift/pull/2565#issuecomment-1104066332

   This is the correct _type_, yes, but it's not the correct string:
   
   ```
   $ python3
   Python 3.10.4 (main, Mar 24 2022, 13:07:27) [GCC 11.2.0] on linux
   Type "help", "copyright", "credits" or "license" for more information.
   >>> import base64
   >>> str(base64.b64encode(b'foo'))
   "b'Zm9v'"
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [thrift] pspeter commented on pull request #2565: Fix bug in Python's THttpClient proxy handling

Posted by GitBox <gi...@apache.org>.
pspeter commented on PR #2565:
URL: https://github.com/apache/thrift/pull/2565#issuecomment-1104856580

   You're right, `ensure_str` it is.
   
   Python 3:
   ```
   >>> cr = b"abc"
   >>> "Basic " + six.ensure_str(cr)
   'Basic abc'
   ```
   
   
   Python 2:
   ```
   >>> cr = "abc"
   >>> "Basic " + six.ensure_str(cr)
   'Basic abc'
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org