Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proxy: fix parser in non-authorization-case #206

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

yamahubuki
Copy link

@yamahubuki yamahubuki commented Oct 17, 2021

I fixed the bug in parsing proxyInfo from environment.

When describing a proxy address, it is common not to write a scheme like 'https: //'.

However, urllib.parse.urlparse is not handling the description without the scheme correctly.

>>> import sys
>>> sys.version
'3.8.5 (tags/v3.8.5:580fbb0, Jul 20 2020, 15:43:08) [MSC v.1926 32 bit (Intel)]'

>>> import urllib
>>> import urllib.parse
>>> urllib.parse.urlparse("http://id:pw@example.com:8080")
ParseResult(scheme='http', netloc='id:pw@example.com:8080', path='', params='', query='', fragment='')
>>> urllib.parse.urlparse("http://example.com:8080")
ParseResult(scheme='http', netloc='example.com:8080', path='', params='', query='', fragment='')

I was having trouble connecting to the proxy, so I rewrote it to code that doesn't use urllib.

@temoto
Copy link
Member

temoto commented Oct 17, 2021

@yamahubuki do you want to write a test?

@yamahubuki
Copy link
Author

@temoto
I added the testcase and fix code for pass existing cases.

@temoto
Copy link
Member

temoto commented Nov 18, 2021

@yamahubuki please rebase on master, it has CI fixed, it should show test failure in py2.

@yamahubuki
Copy link
Author

@temoto
I merged latest master.
But I cannot view the test result from workflow.

The displayed message is as follows.

3 workflows awaiting approval
First-time contributors need a maintainer to approve running workflows

Would you allow me the workflow to run?

@temoto
Copy link
Member

temoto commented Nov 19, 2021

IMHO, best option here is detect have:colon@without.double// then add default scheme in front and make urlparse great again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants