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

Spaces in PAC proxy list cause blank proxies to be added #184

Closed
chunderbolt opened this issue Jun 28, 2023 · 2 comments
Closed

Spaces in PAC proxy list cause blank proxies to be added #184

chunderbolt opened this issue Jun 28, 2023 · 2 comments

Comments

@chunderbolt
Copy link

If you've got multiple proxies in your PAC file (see the Examples section), they're formatted with a semicolon separator. The Mozilla docs give this example:

PROXY w3proxy.netscape.com:8080; PROXY mozilla.netscape.com:8081

Wikipedia has a fairly arcane looking definition for the returnValue format, but from what I can tell it says they're semicolon separated with no space? I don't think people generally stick to that definition.

I see something like this in my Px logs fairly often:

Process-3: Thread_7: 1687968010: /do_CONNECT/do_curl/get_destination: 0ee0e0f633ce30675d71bc5356a0500ca8cb3724: Proxy = [('one.proxy.mycompany.com', 8080), ('', 80), ('two.proxy.mycompany.com', 8080)]

Now, fortunately my PAC file just concatenates the two proxies with a "; ", so the blank proxy is never first in the list to be chosen. If you had a space at the start of one of them, though, the blank proxy will appear first in the list and be used, causing a timeout.

@chunderbolt chunderbolt changed the title Strange PAC file parsing Spaces in PAC proxy list cause blank proxies to be added Jun 28, 2023
@genotrance
Copy link
Owner

Was just looking at the code for this:

px/px/pac.py

Line 84 in 0064b74

return proxies.replace(" ", ",").replace(";", ",")

Not sure why “ “ is replaced with “,”. It means every blank space is treated as another entry. I need to look at the PAC spec again to see what spaces really mean.

@genotrance genotrance added the bug label Jul 12, 2023
@genotrance
Copy link
Owner

This is fixed in v0.9.0 still in development - see branch.

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

No branches or pull requests

2 participants