-
Notifications
You must be signed in to change notification settings - Fork 1
Commit
This changeset introduces support for SPDY server push streams. It is heavily based off work done by Layer, but modified to expose a richer interface to the application. It also undos the SPDYSessionManager changes. Many changes went in SPDYStream in order to support both the NSURLConnection delegate model, and the new CocoaSPDY push stream delegate model. The app can register for SPDYExtendedDelegate callbacks on each NSURLRequest. That, in turn, allows the app to be notified of new push streams, and to subsequently register for notifications of those via a SPDYPushResponseDataDelegate or simpler completion block. Fire-and-forget caching is also an option. Additional usage notes are in SPDYProtocol.h. Stream closing was substantially changed internally. Better support for HEADERS frames is included here. Unit tests for pushed streams are included here along with all the necessary mocks. It's a bit complicated, but has been simplified as much as possible with helpers. A number of small changes and fixes were made to the code as unit tests were developed.
- Loading branch information
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
// | ||
// SPDYCacheStoragePolicy.h | ||
// SPDY | ||
// | ||
// Copyright (c) 2014 Twitter, Inc. All rights reserved. | ||
// Licensed under the Apache License v2.0 | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Derived from code in Apple, Inc.'s CustomHTTPProtocol sample | ||
// project, found as of this notice at | ||
// https://developer.apple.com/LIBRARY/IOS/samplecode/CustomHTTPProtocol | ||
// | ||
|
||
#include <Foundation/Foundation.h> | ||
|
||
/*! Determines the cache storage policy for a response. | ||
* \details When we provide a response up to the client we need to tell the client whether | ||
* the response is cacheable or not. The default HTTP/HTTPS protocol has a reasonable | ||
* complex chunk of code to determine this, but we can't get at it. Thus, we have to | ||
* reimplement it ourselves. This is split off into a separate file to emphasise that | ||
* this is standard boilerplate that you probably don't need to look at. | ||
* \param request The request that generated the response; must not be nil. | ||
* \param response The response itself; must not be nil. | ||
* \returns A cache storage policy to use. | ||
*/ | ||
extern NSURLCacheStoragePolicy SPDYCacheStoragePolicy(NSURLRequest *request, NSHTTPURLResponse *response); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,79 @@ | ||
// | ||
// SPDYCacheStoragePolicy.m | ||
// SPDY | ||
// | ||
// Copyright (c) 2014 Twitter, Inc. All rights reserved. | ||
// Licensed under the Apache License v2.0 | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Derived from code in Apple, Inc.'s CustomHTTPProtocol sample | ||
// project, found as of this notice at | ||
// https://developer.apple.com/LIBRARY/IOS/samplecode/CustomHTTPProtocol | ||
// | ||
|
||
#import "SPDYCacheStoragePolicy.h" | ||
|
||
extern NSURLCacheStoragePolicy SPDYCacheStoragePolicy(NSURLRequest *request, NSHTTPURLResponse *response) | ||
{ | ||
bool cacheable; | ||
NSURLCacheStoragePolicy result; | ||
|
||
// First determine if the request is cacheable based on its status code. | ||
|
||
switch (response.statusCode) { | ||
case 200: | ||
case 203: | ||
This comment has been minimized.
Sorry, something went wrong.
This comment has been minimized.
Sorry, something went wrong.
kgoodier
Author
Owner
|
||
case 206: | ||
case 301: | ||
case 304: | ||
case 404: | ||
case 410: | ||
cacheable = YES; | ||
break; | ||
default: | ||
cacheable = NO; | ||
break; | ||
} | ||
|
||
// If the response might be cacheable, look at the "Cache-Control" header in | ||
// the response. | ||
|
||
// IMPORTANT: We can't rely on -rangeOfString: returning valid results if the target | ||
// string is nil, so we have to explicitly test for nil in the following two cases. | ||
|
||
if (cacheable) { | ||
NSString *responseHeader; | ||
|
||
responseHeader = [response.allHeaderFields[@"cache-control"] lowercaseString]; | ||
if (responseHeader != nil && [responseHeader rangeOfString:@"no-store"].location != NSNotFound) { | ||
cacheable = NO; | ||
} | ||
} | ||
|
||
// If we still think it might be cacheable, look at the "Cache-Control" header in | ||
// the request. | ||
|
||
if (cacheable) { | ||
NSString *requestHeader; | ||
|
||
requestHeader = [request.allHTTPHeaderFields[@"cache-control"] lowercaseString]; | ||
if (requestHeader != nil && | ||
[requestHeader rangeOfString:@"no-store"].location != NSNotFound && | ||
[requestHeader rangeOfString:@"no-cache"].location != NSNotFound) { | ||
cacheable = NO; | ||
} | ||
} | ||
|
||
// Use the cacheable flag to determine the result. | ||
|
||
if (cacheable) { | ||
// Modern versions of iOS use file protection to protect the cache, and thus are | ||
// happy to cache HTTPS on disk. Previous code here returned | ||
// NSURLCacheStorageAllowedInMemoryOnly for https. | ||
result = NSURLCacheStorageAllowed; | ||
} else { | ||
result = NSURLCacheStorageNotAllowed; | ||
} | ||
|
||
return result; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
// | ||
// SPDYCanonicalRequest.h | ||
// SPDY | ||
// | ||
// Copyright (c) 2014 Twitter, Inc. All rights reserved. | ||
// Licensed under the Apache License v2.0 | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Derived from code in Apple, Inc.'s CustomHTTPProtocol sample | ||
// project, found as of this notice at | ||
// https://developer.apple.com/LIBRARY/IOS/samplecode/CustomHTTPProtocol | ||
// | ||
|
||
#import <Foundation/Foundation.h> | ||
|
||
/* | ||
The Foundation URL loading system needs to be able to canonicalize URL requests | ||
for various reasons (for example, to look for cache hits). The default HTTP/HTTPS | ||
protocol has a complex chunk of code to perform this function. Unfortunately | ||
there's no way for third party code to access this. Instead, we have to reimplement | ||
it all ourselves. This is split off into a separate file to emphasise that this | ||
is standard boilerplate that you probably don't need to look at. | ||
IMPORTANT: While you can take most of this code as read, you might want to tweak | ||
the handling of the "Accept-Language" in the CanonicaliseHeaders routine. | ||
*/ | ||
|
||
extern NSMutableURLRequest *SPDYCanonicalRequestForRequest(NSURLRequest *request); |
5 comments
on commit 46e8cbe
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is epic. I finally got SPDY push working with my server. Thanks for this code.
Is this as far as you got with server push? I noticed it doesn't look like its in your fork or twitter's repo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be great to get this into the official repo. Couple of things:
- This commit is pretty old. Does this branch have push enabled? Or is this the only commit that does this? It looks like this specific commit isn't in the current branch?
- I actually had some problems setting the push delegate on a mutable request (had to set it manually with the debugger). I'm confused how this works in the current project which seems to be doing similar things, but it looks like my mutable request is being changed:
// My code
NSMutableURLRequest *req = [NSMutableURLRequest requestWithURL:url];
[req setExtendedDelegate:self queue:[NSOperationQueue mainQueue]];
id task = [session dataTaskWithRequest:req completionHandler:^(NSData *data, NSURLResponse *response, NSError *error) { /* ... */ }
I set a break point here and in + (BOOL)canInitWithRequest:(NSURLRequest *)request (I think this is the next time our code gets hit?
// My debugger
(lldb) po req
<NSMutableURLRequest: 0x7fe31a5c7110> { URL: http://localhost:8080 }
(lldb) po request
<NSURLRequest: 0x7fe31a633b80> { URL: http://localhost:8080 }
It looks like Apple is making a copy of the request we pass in? Or maybe only if its a mutable request to ensure immutability?
I'm running on iOS 9 and XCode 7 and maybe this is a new bug? Just wanted to let you know for when you test this.
Finally, let me know if you need any help with this. I'm really busy over the next month, but should have some time and much more soon.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh dear. I didn't check the commit you referenced since I was on a phone and just assumed it was the right one. It wasn't -- you found an abandoned approach. Apologies for misleading you. The extended delegate just didn't work right due to numerous issues, including the one you just mentioned. It also presented an unnecessarily complicated interface to the app.
I thought you were referencing this: #7. It's what I intend to merge into the main repo, after a rebase. It does not use any new delegates. Notifications back to the app are done via a global NSNotfication, and it is up to the app to filter the URL for ones of interest. CocoaSPDY only caches the pushed request for the lifetime of the parent request; the app must make a new NSURLRequest and "hook up" to the pushed request using normal Apple NSURL interfaces before the stream goes away.
Since you are both motivated and interested in this work, I'll get the rebase done and a pull request to the official repo posted soon, and you can try it out. Thanks for your help!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are 201 (Created) and 204 (No Content) not cacheable? 201 seems like it has the exact semantics as 200 and 204 may have value in the headers even though there’s no body.