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

Handle pac files that return a list of proxies by taking the first one #48

Merged
merged 4 commits into from
May 18, 2018

Conversation

JohnAdders
Copy link
Contributor

It's valid for pac files to return a list to allow for failures, this patch simply always takes the first one

@genotrance
Copy link
Owner

Instead of just picking the first, it will be better to change the ; separator to ,. That way, it will be parsed correctly in caller get_destination() which calls parse_proxy() later and Px will attempt to failover to the next one in the list if the first proxy doesn't work.

I think you could also have DIRECT in the list so if PAC doesn't send the exact string DIRECT (which is handled in get_destination()), it will be incorrectly treated as a proxy hostname. It might be helpful to remove DIRECT from a list response but accept it only if it is an exact string. Thoughts?

@JohnAdders
Copy link
Contributor Author

Update to take into account your comments, thinking about it I'm not sure it handles the case where DIRECT is the first element of a list but I'm not sure that makes sense, do you want me to try and handle that case?

@genotrance
Copy link
Owner

If DIRECT is the first in list, we could probably just treat it as a simple DIRECT case and give up otherwise. I don't think Px can handle the DIRECT or proxy fallback case right now as it is. And it makes little sense to have DIRECT,proxy anyway so I wouldn't want to invest too much time on that unless we get more info.

I have no way to test any of these changes, is this something you can test?

@JohnAdders
Copy link
Contributor Author

Have added removal of first item being DIRECT, I'm testing it with a PAC that either returns a list of 2 proxies or DIRECT

@JohnAdders
Copy link
Contributor Author

Code seems to work as expected with winhttp_find_proxy_for_url returning
DIRECT;fake_proxy;real_proxy
fake_proxy;DIRECT;real_proxy
fake_proxy;real_proxy;DIRECT
fake_proxy;real_proxy
real_proxy
DIRECT

@genotrance genotrance merged commit 6a95d1b into genotrance:master May 18, 2018
@genotrance
Copy link
Owner

Thank you for verifying. 👍

genotrance added a commit that referenced this pull request May 18, 2018
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.

2 participants