http
http copied to clipboard
Added easy-to-use callback to handle custom invalid certificate acceptance
Fixes #458
This pull basically adds the following interface:
// set callback for specific client
ioClient.setBadCertificateCallback((cert, host, port) => true, false);
// or set it globally (which will even work for [http.get()] etc.
http.Client().setBadCertificateCallback((cr, host, port) => true);
where
(cert, host, port) => ...
is a callback which gets invoked if a https-request is made but the certificate appears to be invalid for any rason. If the function returns true the request is processed anyways.
The callback can be set for the current client only or also for all consecutive clients (second parameter; default: set for both the current and all consecutive clients)
This does not work for BrowserClient. I don't know yet if this can be implemented analogous. Trying to invoke this for any client other then IOClient will raise a UnimplementedError
What is done:
- a new test case is added for all relevant new calls
- an additional https server is added to the test suite. It behaves completely analogus to the existing (http) test server but having a security context set being initialized with an included self-signed dummy-certificate / private key.
- the https mock server is under tests
- no existing nor new tests break
that said, I didn't make any pull request yet. Please keep in touch for any problems. I will gladly try to fix. Opening this request marked as draft. If it is in any acceptable stage, I will mark it as mergeable. Just ping me for that also.
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).
:memo: Please visit https://cla.developers.google.com/ to sign.
Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.
What to do if you already signed the CLA
Individual signers
- It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.
Corporate signers
- Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
- The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
- The email used to register you as an authorized contributor must also be attached to your GitHub account.
ℹ️ Googlers: Go here for more info.
Any feedback welcome...
Anyone wants to check this / give feedback / merge this?
@kevmoo @natebosch @srawlins ?
I don't like the idea of the forInstanceOnly argument and the global variable.
My bigger concern is having an IO specific API that leaks to http.Client() instead of only IOClient.
The supported approach here is to use http.IOClient(HttpClient()..badCertificatCallback = callback). You can then hold that client and use DI (or a global variable if you prefer) to reuse the same client.
If we specifically need calls like http.get to be configurable in code that you don't own we'd probably want to look at some kind of zone override API like
/// Runs [callback] in a Zone where top level http methods [get] etc use [client] instead of a default constructed client.
void runWithClient(void Function() callback, Client client);
cc @lrhn
Thanks for looking into it @natebosch
Honestly, the sole idea was to provide an easy way of handling this common use case. After all, this will be an issue for any development-environment dealing with https connections. The ability to intercept with the certificate handling is kind of opaque (as of yet) and needs detailed knowledge about the internals of the package (i.e., that a thing like IOClient exists and how to configure it properly; or that there is a thing like HttpOverrides which intercepts with client creation).
Exposing the method to Client had multiple considerations: Other clients than IOClientmight be able to support this behaviour; also it provides auto-complete support and removes the necessity for explicit conversion (http.Client() returns type Client after all, which would hide it if only exposed to IOClient).
I mean the global variable (if you refer to the static one by that) could be just removed. It's just there to enable http.get()-calls also respect the setting.
Other clients than
IOClientmight be able to support this behaviour
BrowserClient can't support this because the browser doesn't support this. Keeping Client as only the cross-platform subset of the API is a critical part of the design for this package. We want to avoid the throw UnimplementedError anywhere in this package.
Ok, I see.
So moving it up a level to IOClient, removing the static and documenting that feature with a concrete example (like var client = (http.Client() as IOClient)..setBadCertificateCallback(...);) as hint when the HandshakeException is thrown would be ok or you have reservations against that?
This is really required. Need it so badly
We do now have a runWithClient function to impact the behavior of the top level methods.
https://pub.dev/documentation/http/latest/http/runWithClient.html
The supported way to handle this is to construct an IoClient with the bad certificate callback in place, and run the app in a zone with that client set.
Other clients which support the configuration can also handle it at construction time - I don't think we need the setter in the public API.