Bug 136107 - Add USE(APPLE_INTERNAL_SDK)-guards around SPI in ResourceHandle code
Summary: Add USE(APPLE_INTERNAL_SDK)-guards around SPI in ResourceHandle code
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Daniel Bates
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-08-20 10:06 PDT by Daniel Bates
Modified: 2017-11-15 13:10 PST (History)
8 users (show)

See Also:


Attachments
Patch (8.07 KB, patch)
2014-08-20 10:12 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff
Patch (1.47 KB, patch)
2017-11-02 03:59 PDT, Frédéric Wang (:fredw)
no flags Details | Formatted Diff | Diff
Patch (7.38 KB, patch)
2017-11-02 04:18 PDT, Frédéric Wang (:fredw)
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Bates 2014-08-20 10:06:45 PDT
Towards getting iOS WebKit to build using the public SDK we should guard SPI headers with USE(APPLE_INTERNAL_SDK) and add appropriate forward declarations in the following files:

platform/network/ResourceHandleClient.h
platform/network/ResourceHandleInternal.h
platform/network/ios/ResourceHandleIOS.mm
platform/network/mac/ResourceErrorMac.mm
Comment 1 Daniel Bates 2014-08-20 10:12:20 PDT
Created attachment 236875 [details]
Patch
Comment 2 Daniel Bates 2014-08-20 13:03:56 PDT
Comment on attachment 236875 [details]
Patch

Will update patch to fix Windows build issues.
Comment 3 David Kilzer (:ddkilzer) 2014-12-17 14:05:28 PST
Is this still needed after the fix for this bug?

Bug 136487: [iOS] Make WebCore build and link with public SDK
Comment 4 BJ Burg 2016-02-02 11:47:25 PST
There is still this:

#ifndef ResourceHandleClient_h
#define ResourceHandleClient_h

...

#if USE(CFNETWORK)
#include <CFNetwork/CFURLCachePriv.h>
#include <CFNetwork/CFURLResponsePriv.h>
#endif


The private includes could probably just be replaced with #include <CFNetworkSPI.h>.
Comment 5 Frédéric Wang (:fredw) 2017-11-02 03:46:43 PDT
(In reply to Brian Burg from comment #4)
> There is still this:
> 
> #ifndef ResourceHandleClient_h
> #define ResourceHandleClient_h
> 
> ...
> 
> #if USE(CFNETWORK)
> #include <CFNetwork/CFURLCachePriv.h>
> #include <CFNetwork/CFURLResponsePriv.h>
> #endif
> 
> 
> The private includes could probably just be replaced with #include
> <CFNetworkSPI.h>.

It looks like USE_CFNETWORK has been removed in r207151. Do we still want that change?
Comment 6 Frédéric Wang (:fredw) 2017-11-02 03:59:35 PDT
Created attachment 325704 [details]
Patch
Comment 7 Frédéric Wang (:fredw) 2017-11-02 04:00:05 PDT
(In reply to Frédéric Wang (:fredw) from comment #6)
> Created attachment 325704 [details]
> Patch

Untested patch, based on previous comments.
Comment 8 Frédéric Wang (:fredw) 2017-11-02 04:18:04 PDT
Created attachment 325705 [details]
Patch
Comment 9 Frédéric Wang (:fredw) 2017-11-02 04:19:02 PDT
(In reply to Brian Burg from comment #4)
> There is still this:

Actually, it seems none of the changes were actually taken. So I just uploaded an untested rebase of Daniel's patch.
Comment 10 Alex Christensen 2017-11-02 22:49:46 PDT
Comment on attachment 325705 [details]
Patch

Why are we trying to make iOS build using CFURLConnection?  Right now we're using NSURLConnection on iOS WebKitLegacy and NSURLSession on WebKit, and we're moving towards using NSURLSession everywhere.  We don't intend to maintain an iOS build that uses CFURLConnection.
Comment 11 Frédéric Wang (:fredw) 2017-11-03 00:09:51 PDT
(In reply to Alex Christensen from comment #10)
> Comment on attachment 325705 [details]
> Patch
> 
> Why are we trying to make iOS build using CFURLConnection?  Right now we're
> using NSURLConnection on iOS WebKitLegacy and NSURLSession on WebKit, and
> we're moving towards using NSURLSession everywhere.  We don't intend to
> maintain an iOS build that uses CFURLConnection.

Then maybe this old bug should be WONTFIX :-)
Comment 12 Darin Adler 2017-11-03 07:56:11 PDT
Comment on attachment 325705 [details]
Patch

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

> Source/WebCore/platform/network/ios/ResourceHandleIOS.mm:42
> +#if USE(APPLE_INTERNAL_SDK)
>  #import <CFNetwork/CFSocketStreamPriv.h>
>  #import <Foundation/NSURLRequestPrivate.h>
> +#else
> +#import <Foundation/NSURLRequest.h>
> +@interface NSURLRequest (Details)
> ++ (BOOL)allowsAnyHTTPSCertificateForHost:(NSString *)host;
> ++ (NSArray*)allowsSpecificHTTPSCertificateForHost:(NSString *)host;
> +@end
> +#endif
>  
> -using namespace WebCore;
> +extern "C" const CFStringRef _kCFStreamSSLTrustedLeafCertificates;

In theory this belongs in an SPI.h header; someone should move it there eventually.

> Source/WebCore/platform/network/mac/ResourceErrorMac.mm:43
> -#if PLATFORM(IOS) && USE(CFURLCONNECTION)
> +#if PLATFORM(IOS) && USE(CFURLCONNECTION) && USE(APPLE_INTERNAL_SDK)
>  #import <CFNetwork/CFSocketStreamPriv.h>
>  #endif
>  
> +#if USE(CFURLCONNECTION)
> +extern "C" {
> +const CFStringRef _kCFStreamPropertySSLClientCertificates;
> +const CFStringRef _kCFStreamPropertySSLClientCertificateState;
> +}
> +#endif

Ditto.
Comment 13 Frédéric Wang (:fredw) 2017-11-03 08:01:51 PDT
Committed r224395: <https://trac.webkit.org/changeset/224395>
Comment 14 Frédéric Wang (:fredw) 2017-11-03 08:09:43 PDT
Comment on attachment 325705 [details]
Patch

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

>> Source/WebCore/platform/network/ios/ResourceHandleIOS.mm:42
>> +extern "C" const CFStringRef _kCFStreamSSLTrustedLeafCertificates;
> 
> In theory this belongs in an SPI.h header; someone should move it there eventually.

Right. I see other places where it's done e.g. Source/WebCore/platform/cocoa/TelephoneNumberDetectorCocoa.cpp ; maybe this can be addressed in bug 164684.
Comment 15 Radar WebKit Bug Importer 2017-11-15 13:10:57 PST
<rdar://problem/35568927>