Skip to content

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

Open
yamahubuki wants to merge 3 commits intohttplib2:masterfrom
yamahubuki:master
Open

proxy: fix parser in non-authorization-case#206
yamahubuki wants to merge 3 commits intohttplib2:masterfrom
yamahubuki:master

Conversation

@yamahubuki
Copy link
Copy Markdown

@yamahubuki yamahubuki commented Oct 17, 2021 •
edited
Loading

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
Copy Markdown
Member

temoto commented Oct 17, 2021

@yamahubuki do you want to write a test?

@yamahubuki
Copy link
Copy Markdown
Author

yamahubuki commented Oct 18, 2021

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

@temoto
Copy link
Copy Markdown
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
Copy Markdown
Author

yamahubuki commented Nov 19, 2021

@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
Copy Markdown
Member

temoto commented Nov 19, 2021 •
edited
Loading

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

@temoto temoto mentioned this pull request Feb 1, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@yamahubuki @temoto