Expanded proxy support, made it easier to set proxy settings #2007
Conversation
merceyz
commented
Apr 5, 2017
merceyz
commented
- Added a ProxyOptions class
- Added a CefSettings.Proxy property
- Made ClientAdapter::GetAuthCredentials take care of the authentication for the proxy
|
Will look at this sometime next week 👍 |
CefSharp/ProxyOptions.cs
Outdated
| /// <param name="username">The username required for authentication</param> | ||
| /// <param name="password">The password required for authentication</param> | ||
| /// <param name="bypassList">The list of domains that shouldn't be affected by the proxy, Format: example.com;example2.com</param> | ||
| public ProxyOptions(string ip, string port, string username, string password, string bypassList) |
There was a problem hiding this comment.
Can we just use default values and get rid of the overloads?
| @@ -732,6 +733,16 @@ namespace CefSharp | |||
| bool ClientAdapter::GetAuthCredentials(CefRefPtr<CefBrowser> browser, CefRefPtr<CefFrame> frame, bool isProxy, | |||
| const CefString& host, int port, const CefString& realm, const CefString& scheme, CefRefPtr<CefAuthCallback> callback) | |||
| { | |||
| if (isProxy && Cef::proxyOptions != nullptr && Cef::proxyOptions->IP == StringUtils::ToClr(host) && !String::IsNullOrEmpty(Cef::proxyOptions->Username) | |||
There was a problem hiding this comment.
What if you wanted to provide a different username and password after you've initialized CEF?
I'd rather not having additional code here, specially that adds a new behavior.
It probably makes more sense to provide a default implementation of IRequestHandler that contains this logic. (If we do go down that path, probably should add default implementations for all the handlers.)
There was a problem hiding this comment.
What if you wanted to provide a different username and password after you've initialized CEF?
That's now possible by doing CefSharpSettings.Proxy.Username = "x" and CefSharpSettings.Proxy.Password = "y"
I don't quite agree with you, I'm hoping this can stay as is so that anyone using the IRequestHandler doesn't have to implement this logic themself
CefSharp.Core/CefSettings.h
Outdated
| @@ -368,6 +369,15 @@ namespace CefSharp | |||
| } | |||
|
|
|||
| /// <summary> | |||
| /// The proxy options that will be used for all connections | |||
| /// </summary> | |||
| property ProxyOptions^ Proxy | |||
There was a problem hiding this comment.
I'd rather see this as part of CefSharpSettings.
| @@ -41,5 +41,10 @@ static CefSharpSettings() | |||
| /// the event handlers are hooked in the static constructor for the ChromiumWebBrowser class | |||
| /// </summary> | |||
| public static bool ShutdownOnExit { get; set; } | |||
|
|
|||
| /// <summary> | |||
| /// The proxy options that will be used for all connections | |||
There was a problem hiding this comment.
A think we should expand on this, explain what's going on. Command line arguments are set, if you set username/password then GetAuthCredentials will be called automatically. Also note that if you use this option it's impossible to change the proxy at dynamically at runtime.
There was a problem hiding this comment.
Also say that GetAuthCredentials won't be called, for a proxy server that matches the IP
| if (isProxy && CefSharpSettings::Proxy != nullptr && CefSharpSettings::Proxy->IP == StringUtils::ToClr(host) && !String::IsNullOrEmpty(CefSharpSettings::Proxy->Username) | ||
| && !String::IsNullOrEmpty(CefSharpSettings::Proxy->Password)) | ||
| { | ||
| auto frameWrapper = gcnew CefFrameWrapper(frame); |
There was a problem hiding this comment.
Seems unnecessary to create these wrappers, can just execute the callback directly.
| @@ -732,6 +732,16 @@ namespace CefSharp | |||
| bool ClientAdapter::GetAuthCredentials(CefRefPtr<CefBrowser> browser, CefRefPtr<CefFrame> frame, bool isProxy, | |||
| const CefString& host, int port, const CefString& realm, const CefString& scheme, CefRefPtr<CefAuthCallback> callback) | |||
| { | |||
| if (isProxy && CefSharpSettings::Proxy != nullptr && CefSharpSettings::Proxy->IP == StringUtils::ToClr(host) && !String::IsNullOrEmpty(CefSharpSettings::Proxy->Username) | |||
There was a problem hiding this comment.
What about creating a HasUsernameAndPassword() method (or property) to ProxySettings, make this a little cleaner, little less C++
|
@merceyz I've granted you access to all of the Once your happy this is ready to release into the wild then feel free to merge yourself 👍 |
Thank you, though I haven't gotten an invitation yet I appreciate it. |
Check https://github.com/cefsharp Tried sending you another one, would only let me edit the existing invite, so it's def been sent. |
|
There it was, cheers
Didn't get an E-mail nor a notification, so I suppose one would have to know the invite was sent and where to find it |
It said an |
Probably Would you like this to be a part of |
I'm reluctant to merge anything other than bug fixes this late in the release process. Plan is to generate the |
