Parse Proxy Auto-Configuration strings#9127
Merged
outofambit merged 17 commits intodevelopmentfrom Feb 19, 2020
Merged
Conversation
\t is technically valid in PAC strings
Chromium ensures we get compliant strings
Okay I guess we probably should support case insensitive protocols, it's part of the spec after all. This reverts commit 7a8304b.
outofambit
approved these changes
Feb 18, 2020
Contributor
outofambit
left a comment
There was a problem hiding this comment.
thanks for adding tests, that really helped with understanding this!
This was referenced Feb 19, 2020
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I'm extracting the PAC parser from my ongoing work to better support proxies in order to lessen the review burden of the final work. Note that this PR will add an unused function and that's okay.
In order to know what to tell Git (or cURL rather) what (if any) proxy it should be using we'll first need to figure out what the system is set up to use. For that we'll leverage the, poorly documented, resolveProxy method in Electron. The method returns a PAC string which we'll need to parse in order to convert it to something that's usable for cURL which is the entire purpose of this method.
The code is documented (perhaps overly so) with several references that's useful if you've never heard of PAC files or PAC strings. The best primer IMO is MDN's article on PAC files.
We don't have to support all the weird variants or the raw output of an actual
FindProxyForURLif a user's organization has one set up since Chromium will do an initial parsing round and reformat before handing it off to us.TL;DR Chops and mashes some strings with more documentation than logic