Bug 159039 – Integrate WebKit's CFURLConnection with App Transport Security

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

[Oliver Hunt]

Integrate WebKit's CFURLConnection with App Transport Security

[Oliver Hunt]

Created attachment 281870 [details]
Patch

[Alex Christensen]

Comment on attachment 281870 [details]
Patch

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

> Source/WebCore/platform/network/mac/ResourceHandleMac.mm:134
> +    extern const CFStringRef _kCFURLConnectionPropertyATSContext;

This should probably go in CFNetworkSPI.h.

> Source/WebCore/platform/network/mac/ResourceHandleMac.mm:233
> +    [propertyDictionary setObject:@{@"NSAllowsArbitraryLoadsInWebContent": @""} forKey:(NSString *)_kCFURLConnectionPropertyATSContext];

What does @"" mean here?

[Alex Christensen]

Is there a radar associated with this?

[Oliver Hunt]

Created attachment 281874 [details]
Patch

[Oliver Hunt]

<rdar://problem/26953685>

[Oliver Hunt]

Created attachment 281876 [details]
Patch

[Alex Christensen]

Comment on attachment 281876 [details]
Patch

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

> Source/WebCore/platform/network/mac/ResourceHandleMac.mm:231
> +    [propertyDictionary setObject:@{@"NSAllowsArbitraryLoadsInWebContent": @""} forKey:(NSString *)_kCFURLConnectionPropertyATSContext];

This needs to be surrounded by an #if PLATFORM(IOS) || (PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101100)
Or something similar.
Is this even needed on Mac?

[Oliver Hunt]

Created attachment 281879 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 281879 [details]
Patch

Clearing flags on attachment: 281879

Committed r202356: <http://trac.webkit.org/changeset/202356>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[David Kilzer (:ddkilzer)]

Comment on attachment 281879 [details]
Patch

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

> Source/WebCore/platform/network/mac/ResourceHandleMac.mm:230
> +#if TARGET_OS_IPHONE || (PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101100)

Kind of weird to use TARGET_OS_IPHONE instead of PLATFORM(IOS) here.

Should we be checking for iOS 10 here as well?  (The property is probably ignored on older builds, so it's probably not necessary to add a version check.

[Oliver Hunt]

(In reply to comment #11)
> Comment on attachment 281879 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=281879&action=review
> 
> > Source/WebCore/platform/network/mac/ResourceHandleMac.mm:230
> > +#if TARGET_OS_IPHONE || (PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101100)
> 
> Kind of weird to use TARGET_OS_IPHONE instead of PLATFORM(IOS) here.
> 
> Should we be checking for iOS 10 here as well?  (The property is probably
> ignored on older builds, so it's probably not necessary to add a version
> check.

I used identical guards to the definition point to avoid any chance of things going wrong :)

And yeah, it's a no-op option elsewhere :)