chore: validate client-certificates on context creation #32168
Conversation
mxschmitt
force-pushed
the
validate-certs-on-context-creation
branch
from
807fcfa
to
71d147a
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
mxschmitt
force-pushed
the
validate-certs-on-context-creation
branch
from
71d147a
to
af899a6
Compare
This comment has been minimized.
This comment has been minimized.
| @@ -735,6 +736,25 @@ export function verifyClientCertificates(clientCertificates?: channels.BrowserNe | |||
| throw new Error('key is specified without cert'); | |||
| if (cert.pfx && (cert.cert || cert.key)) | |||
| throw new Error('pfx is specified together with cert, key or passphrase'); | |||
| try { | |||
There was a problem hiding this comment.
If we move this into ClientCertificatesProxy constructor (and a similar place in fetch), perhaps we can save the resulting secure context and reuse it? Perhaps that gives us a bit more clarity (created a context ones and the using it) and a small performance win?
There was a problem hiding this comment.
I tried to apply this feedback to the SocksCertificateProxy, there it seems to work out well. For the APIRequestContext, https.request unfortunately doesn't accept a tls.SecureContext - so we keep things there as it is.
This seems to be aligned with the goal of the PR: clear and fast user-feedback which seems to be the case now for both, browser and APIRequestAPI (like before) usage.
mxschmitt
force-pushed
the
validate-certs-on-context-creation
branch
from
af899a6
to
2c8790e
Compare
Test results for "tests 1"2 flaky30034 passed, 860 skipped Merge workflow run. |
No description provided.