Bug 230137 – Add infrastructure to allow TLS during PCM tests

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Alex Christensen]

Add infrastructure to allow TLS during PCM tests

[Alex Christensen]

Created attachment 437809 [details]
Patch

[EWS Watchlist]

Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See https://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

[Alex Christensen]

Created attachment 437810 [details]
Patch

[Alex Christensen]

Created attachment 437829 [details]
Patch

[Alex Christensen]

Created attachment 437832 [details]
Patch

[Alex Christensen]

Created attachment 437836 [details]
Patch

[Alex Christensen]

Created attachment 437841 [details]
Patch

[Alex Christensen]

Created attachment 437844 [details]
Patch

[EWS]

Committed r282269 (241547@main): <https://commits.webkit.org/241547@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 437844 [details].

[Radar WebKit Bug Importer]

<rdar://problem/82975108>

[John Wilander]

Comment on attachment 437844 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=437844&action=review

> Source/WebKit/NetworkProcess/PrivateClickMeasurement/PrivateClickMeasurementDecoder.cpp:52
> +const uint8_t* Decoder::decodeFixedLengthReference(size_t size, size_t)

Is this just testing infrastructure? If so, should we name it as such?

> Source/WebKit/NetworkProcess/PrivateClickMeasurement/cocoa/PrivateClickMeasurementNetworkLoaderCocoa.mm:43
> +    if (![challenge.protectionSpace.host isEqualToString:@"127.0.0.1"]

I know we've talked several times about having these known local test server hosts statically available somewhere so we don't have to spread these stings out.

> Source/WebKit/NetworkProcess/PrivateClickMeasurement/cocoa/PrivateClickMeasurementNetworkLoaderCocoa.mm:113
> +    if (allowedLocalTestServerTrust() && url.host() != "127.0.0.1")

Ditto on the host.

> Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm:-1646
> -

Accidental?

[Alex Christensen]

(In reply to John Wilander from comment #11)
> Comment on attachment 437844 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=437844&action=review
> 
> > Source/WebKit/NetworkProcess/PrivateClickMeasurement/PrivateClickMeasurementDecoder.cpp:52
> > +const uint8_t* Decoder::decodeFixedLengthReference(size_t size, size_t)
This is used by many types to decode a DataReference.  Right now it's only used for CertificateInfo decoding in testing stuff, but its signature needs to look just like this.  It will likely be used for other things in the future.

> > Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm:-1646
> > -
> 
> Accidental?
Kind of.

[John Wilander]

(In reply to Alex Christensen from comment #12)
> (In reply to John Wilander from comment #11)
> > Comment on attachment 437844 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=437844&action=review
> > 
> > > Source/WebKit/NetworkProcess/PrivateClickMeasurement/PrivateClickMeasurementDecoder.cpp:52
> > > +const uint8_t* Decoder::decodeFixedLengthReference(size_t size, size_t)
> This is used by many types to decode a DataReference.  Right now it's only
> used for CertificateInfo decoding in testing stuff, but its signature needs
> to look just like this.  It will likely be used for other things in the
> future.

Yeah, that code made more sense not labeled as testing when I got to the rest of the patch. In the beginning it looked like complex, risky code added just for testing and I was worried others wouldn't know it was for testing. But then I realized it's just general encode/decode.

> > > Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm:-1646
> > > -
> > 
> > Accidental?
> Kind of.