Skip to content

Commit

Permalink
Merge pull request #175 from udacity/master
Browse files Browse the repository at this point in the history 
Fix for issue #174 - NSURLSessionTask completion block not called when not following redirects
  • Loading branch information
@AliSoftware
AliSoftware committed May 14, 2016
2 parents ec65530 + b8bc44a commit 46850d0
Show file tree
Hide file tree
Showing 4 changed files with 207 additions and 40 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

## Master

* Bugfix: task completion block never called when not following redirects.
[@adurdin](https://github.com/adurdin), [#175](https://github.com/AliSoftware/OHHTTPStubs/pull/175)

* Declare in the project settings that the library contains swift code.
[@rodericj](https://github.com/rodericj), [#173](https://github.com/AliSoftware/OHHTTPStubs/pull/173)

Expand Down
61 changes: 30 additions & 31 deletions OHHTTPStubs/Sources/OHHTTPStubs.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -418,6 +418,7 @@ - (void)startLoading
{
redirectLocationURL = nil;
}
// Notify if a redirection occurred
if (((responseStub.statusCode > 300) && (responseStub.statusCode < 400)) && redirectLocationURL)
{
NSURLRequest* redirectRequest = [NSURLRequest requestWithURL:redirectLocationURL];
Expand All @@ -432,39 +433,37 @@ - (void)startLoading
}
}];
}
else
{
[self executeOnClientRunLoopAfterDelay:responseStub.requestTime block:^{
if (!self.stopped)
// Send the response (even for redirections)
[self executeOnClientRunLoopAfterDelay:responseStub.requestTime block:^{
if (!self.stopped)
{
[client URLProtocol:self didReceiveResponse:urlResponse cacheStoragePolicy:NSURLCacheStorageNotAllowed];
if(responseStub.inputStream.streamStatus == NSStreamStatusNotOpen)
{
[client URLProtocol:self didReceiveResponse:urlResponse cacheStoragePolicy:NSURLCacheStorageNotAllowed];
if(responseStub.inputStream.streamStatus == NSStreamStatusNotOpen)
{
[responseStub.inputStream open];
}
[self streamDataForClient:client
withStubResponse:responseStub
completion:^(NSError * error)
{
[responseStub.inputStream close];
NSError *blockError = nil;
if (error==nil)
{
[client URLProtocolDidFinishLoading:self];
}
else
{
[client URLProtocol:self didFailWithError:responseStub.error];
blockError = responseStub.error;
}
if (OHHTTPStubs.sharedInstance.afterStubFinishBlock)
{
OHHTTPStubs.sharedInstance.afterStubFinishBlock(request, self.stub, responseStub, blockError);
}
}];
[responseStub.inputStream open];
}
}];
}
[self streamDataForClient:client
withStubResponse:responseStub
completion:^(NSError * error)
{
[responseStub.inputStream close];
NSError *blockError = nil;
if (error==nil)
{
[client URLProtocolDidFinishLoading:self];
}
else
{
[client URLProtocol:self didFailWithError:responseStub.error];
blockError = responseStub.error;
}
if (OHHTTPStubs.sharedInstance.afterStubFinishBlock)
{
OHHTTPStubs.sharedInstance.afterStubFinishBlock(request, self.stub, responseStub, blockError);
}
}];
}
}];
} else {
// Send the canned error
[self executeOnClientRunLoopAfterDelay:responseStub.responseTime block:^{
Expand Down
65 changes: 57 additions & 8 deletions OHHTTPStubs/UnitTests/Test Suites/AFNetworkingTests.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,32 +125,35 @@ -(void)test_AFHTTPRequestOperation_redirect
static const NSTimeInterval kRequestTime = 0.05;
static const NSTimeInterval kResponseTime = 0.1;

NSURL* redirectURL = [NSURL URLWithString:@"http://www.iana.org/domains/another/example"];
NSURL* redirectURL = [NSURL URLWithString:@"https://httpbin.org/get"];
[OHHTTPStubs stubRequestsPassingTest:^BOOL(NSURLRequest *request) {
return YES;
} withStubResponse:^OHHTTPStubsResponse *(NSURLRequest *request) {
return [[OHHTTPStubsResponse responseWithData:[NSData data] statusCode:311 headers:@{@"Location":redirectURL.absoluteString}]
return [[OHHTTPStubsResponse responseWithData:[NSData data] statusCode:302 headers:@{@"Location":redirectURL.absoluteString}]
requestTime:kRequestTime responseTime:kResponseTime];
}];

XCTestExpectation* redirectExpectation = [self expectationWithDescription:@"AFHTTPRequestOperation request was redirected"];
XCTestExpectation* expectation = [self expectationWithDescription:@"AFHTTPRequestOperation request finished"];
NSURLRequest* req = [NSURLRequest requestWithURL:[NSURL URLWithString:@"http://www.iana.org/domains/example/"]];

NSURLRequest* req = [NSURLRequest requestWithURL:[NSURL URLWithString:@"https://httpbin.org/redirect/1"]];
AFHTTPSessionManager *manager = [AFHTTPSessionManager manager];
[manager setResponseSerializer:[AFHTTPResponseSerializer serializer]];

AFHTTPResponseSerializer *serializer = [AFHTTPResponseSerializer serializer];
serializer.acceptableStatusCodes = [NSIndexSet indexSetWithIndex:302];
[manager setResponseSerializer:serializer];

__block __strong NSURL* url = nil;
[manager setTaskWillPerformHTTPRedirectionBlock:^NSURLRequest * (NSURLSession * session, NSURLSessionTask * task, NSURLResponse * response, NSURLRequest * request) {
if (response == nil) {
return request;
}
url = request.URL;
[expectation fulfill];
[redirectExpectation fulfill];
return nil;
}];

[manager GET:req.URL.absoluteString parameters:nil progress:nil success:^(NSURLSessionTask *task, id responseObject) {
XCTFail(@"Unexpected response");
// Expect the 302 response when the redirection block returns nil (don't follow redirects)
[expectation fulfill];
} failure:^(NSURLSessionTask *operation, NSError *error) {
XCTFail(@"Unexpected network failure");
Expand All @@ -162,6 +165,52 @@ -(void)test_AFHTTPRequestOperation_redirect
XCTAssertEqualObjects(url, redirectURL, @"Unexpected data received");
}

/*
* In order to establish that test_AFHTTPRequestOperation_redirect was incorrect and needed fixing, I needed
* to demonstrate identical behaviour--that is, returning the redirect response itself to the success block--
* when running without the NSURLProtocol stubbing the request. The test below, if enabled, establishes this,
* as it is identical to test_AFHTTPRequestOperation_redirect except that it does not stub the requests.
*/
#if 0
-(void)test_AFHTTPRequestOperation_redirect_baseline
{
NSURL* redirectURL = [NSURL URLWithString:@"https://httpbin.org/get"];

XCTestExpectation* redirectExpectation = [self expectationWithDescription:@"AFHTTPRequestOperation request was redirected"];
XCTestExpectation* expectation = [self expectationWithDescription:@"AFHTTPRequestOperation request finished"];

NSURLRequest* req = [NSURLRequest requestWithURL:[NSURL URLWithString:@"https://httpbin.org/redirect/1"]];
AFHTTPSessionManager *manager = [AFHTTPSessionManager manager];
AFHTTPResponseSerializer *serializer = [AFHTTPResponseSerializer serializer];
serializer.acceptableStatusCodes = [NSIndexSet indexSetWithIndex:302];
[manager setResponseSerializer:serializer];

__block __strong NSURL* url = nil;
[manager setTaskWillPerformHTTPRedirectionBlock:^NSURLRequest * (NSURLSession * session, NSURLSessionTask * task, NSURLResponse * response, NSURLRequest * request) {
if (response == nil) {
return request;
}
url = request.URL;
[redirectExpectation fulfill];
return nil;
}];

[manager GET:req.URL.absoluteString parameters:nil progress:nil success:^(NSURLSessionTask *task, id responseObject) {
// Expect the 302 response when the redirection block returns nil (don't follow redirects)
[expectation fulfill];
} failure:^(NSURLSessionTask *operation, NSError *error) {
XCTFail(@"Unexpected network failure");
[expectation fulfill];
}];

// Allow a longer timeout as this test actually hits the network
[self waitForExpectationsWithTimeout:10 handler:nil];

XCTAssertEqualObjects(url, redirectURL, @"Unexpected data received");
}
#endif


@end


Expand Down
118 changes: 117 additions & 1 deletion OHHTTPStubs/UnitTests/Test Suites/NSURLSessionTests.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,19 +38,21 @@
@import OHHTTPStubs;
#endif

@interface NSURLSessionTests : XCTestCase <NSURLSessionDataDelegate> @end
@interface NSURLSessionTests : XCTestCase <NSURLSessionDataDelegate, NSURLSessionTaskDelegate> @end

@implementation NSURLSessionTests
{
NSMutableData* _receivedData;
XCTestExpectation* _taskDidCompleteExpectation;
BOOL _shouldFollowRedirects;
}

- (void)setUp
{
[super setUp];
[OHHTTPStubs removeAllStubs];
_receivedData = nil;
_shouldFollowRedirects = YES;
}

- (void)_test_NSURLSession:(NSURLSession*)session
Expand Down Expand Up @@ -100,6 +102,67 @@ - (void)_test_NSURLSession:(NSURLSession*)session
}
}

- (void)_test_redirect_NSURLSession:(NSURLSession*)session
jsonForStub:(id)json
completion:(void(^)(NSError* errorResponse, NSHTTPURLResponse *response, id jsonResponse))completion
{
if ([NSURLSession class])
{
static const NSTimeInterval kRequestTime = 0.0;
static const NSTimeInterval kResponseTime = 0.2;

[OHHTTPStubs stubRequestsPassingTest:^BOOL(NSURLRequest *request) {
return [[[request URL] path] isEqualToString:@""];
} withStubResponse:^OHHTTPStubsResponse *(NSURLRequest *request) {
NSDictionary *headers = @{ @"Location": @"foo://unknownhost:666/elsewhere" };
return [[OHHTTPStubsResponse responseWithData:[[NSData alloc] init] statusCode:301 headers:headers]
requestTime:kRequestTime responseTime:kResponseTime];
}];

[OHHTTPStubs stubRequestsPassingTest:^BOOL(NSURLRequest *request) {
return [[[request URL] path] isEqualToString:@"/elsewhere"];
} withStubResponse:^OHHTTPStubsResponse *(NSURLRequest *request) {
return [[OHHTTPStubsResponse responseWithJSONObject:json statusCode:200 headers:nil]
requestTime:kRequestTime responseTime:kResponseTime];
}];

XCTestExpectation* expectation = [self expectationWithDescription:@"NSURLSessionDataTask completed"];

__block __strong NSHTTPURLResponse *redirectResponse = nil;
__block __strong id dataResponse = nil;
__block __strong NSError* errorResponse = nil;
NSMutableURLRequest *request = [NSMutableURLRequest requestWithURL:[NSURL URLWithString:@"foo://unknownhost:666"]];
request.HTTPMethod = @"GET";
[request setValue:@"application/json" forHTTPHeaderField:@"Content-Type"];
[request setValue:@"application/json" forHTTPHeaderField:@"Accept"];

NSURLSessionDataTask *task = [session dataTaskWithRequest:request
completionHandler:^(NSData *data, NSURLResponse *response, NSError *error)
{
errorResponse = error;
if (!error)
{
NSHTTPURLResponse *HTTPResponse = (NSHTTPURLResponse *)response;
if ([HTTPResponse statusCode] >= 300 && [HTTPResponse statusCode] < 400) {
redirectResponse = HTTPResponse;
} else {
NSError *jsonError = nil;
NSDictionary *jsonObject = [NSJSONSerialization JSONObjectWithData:data options:0 error:&jsonError];
XCTAssertNil(jsonError, @"Unexpected error deserializing JSON response");
dataResponse = jsonObject;
}
}
[expectation fulfill];
}];

[task resume];

[self waitForExpectationsWithTimeout:kRequestTime+kResponseTime+0.5 handler:nil];

completion(errorResponse, redirectResponse, dataResponse);
}
}

// The shared session use the same mechanism as NSURLConnection
// (based on protocols registered via +[NSURLProtocol registerClass:] and all)
// and no NSURLSessionConfiguration
Expand All @@ -114,6 +177,12 @@ - (void)test_SharedNSURLSession
XCTAssertNil(errorResponse, @"Unexpected error");
XCTAssertEqualObjects(jsonResponse, json, @"Unexpected data received");
}];

[self _test_redirect_NSURLSession:session jsonForStub:json completion:^(NSError *errorResponse, NSHTTPURLResponse *redirectResponse, id jsonResponse) {
XCTAssertNil(errorResponse, @"Unexpected error");
XCTAssertNil(redirectResponse, @"Unexpected redirect response received");
XCTAssertEqualObjects(jsonResponse, json, @"Unexpected data received");
}];
}
else
{
Expand All @@ -133,6 +202,35 @@ - (void)test_NSURLSessionDefaultConfig
XCTAssertNil(errorResponse, @"Unexpected error");
XCTAssertEqualObjects(jsonResponse, json, @"Unexpected data received");
}];

[self _test_redirect_NSURLSession:session jsonForStub:json completion:^(NSError *errorResponse, NSHTTPURLResponse *redirectResponse, id jsonResponse) {
XCTAssertNil(errorResponse, @"Unexpected error");
XCTAssertNil(redirectResponse, @"Unexpected redirect response received");
XCTAssertEqualObjects(jsonResponse, json, @"Unexpected data received");
}];
}
else
{
NSLog(@"/!\\ Test skipped because the NSURLSession class is not available on this OS version. Run the tests a target with a more recent OS.\n");
}
}

- (void)test_NSURLSessionDefaultConfig_notFollowingRedirects
{
_shouldFollowRedirects = NO;

if ([NSURLSessionConfiguration class] && [NSURLSession class])
{
NSURLSessionConfiguration* config = [NSURLSessionConfiguration defaultSessionConfiguration];
NSURLSession *session = [NSURLSession sessionWithConfiguration:config delegate:self delegateQueue:nil];

NSDictionary* json = @{@"Success": @"Yes"};
[self _test_redirect_NSURLSession:session jsonForStub:json completion:^(NSError *errorResponse, NSHTTPURLResponse *redirectResponse, id jsonResponse) {
XCTAssertNil(errorResponse, @"Unexpected error");
XCTAssertNotNil(redirectResponse, @"Redirect response should have been received");
XCTAssertEqual(301, [redirectResponse statusCode], @"Expected 301 redirect");
XCTAssertNil(jsonResponse, @"Unexpected data received");
}];
}
else
{
Expand All @@ -152,6 +250,12 @@ - (void)test_NSURLSessionEphemeralConfig
XCTAssertNil(errorResponse, @"Unexpected error");
XCTAssertEqualObjects(jsonResponse, json, @"Unexpected data received");
}];

[self _test_redirect_NSURLSession:session jsonForStub:json completion:^(NSError *errorResponse, NSHTTPURLResponse *redirectResponse, id jsonResponse) {
XCTAssertNil(errorResponse, @"Unexpected error");
XCTAssertNil(redirectResponse, @"Unexpected redirect response received");
XCTAssertEqualObjects(jsonResponse, json, @"Unexpected data received");
}];
}
else
{
Expand All @@ -173,6 +277,13 @@ - (void)test_NSURLSessionDefaultConfig_Disabled
XCTAssertNotNil(errorResponse, @"Expected error but none found");
XCTAssertNil(jsonResponse, @"Data should not have been received as stubs should be disabled");
}];

[self _test_redirect_NSURLSession:session jsonForStub:json completion:^(NSError *errorResponse, NSHTTPURLResponse *redirectResponse, id jsonResponse) {
// Stubs were disable for this session, so we should get an error instead of the stubs data
XCTAssertNotNil(errorResponse, @"Expected error but none found");
XCTAssertNil(redirectResponse, @"Redirect response should not have been received as stubs should be disabled");
XCTAssertNil(jsonResponse, @"Data should not have been received as stubs should be disabled");
}];
}
else
{
Expand Down Expand Up @@ -226,6 +337,11 @@ - (void)URLSession:(NSURLSession *)session task:(NSURLSessionTask *)task didComp
[_taskDidCompleteExpectation fulfill];
}

- (void)URLSession:(NSURLSession *)session task:(NSURLSessionTask *)task willPerformHTTPRedirection:(NSHTTPURLResponse *)response newRequest:(NSURLRequest *)request completionHandler:(void (^)(NSURLRequest * _Nullable))completionHandler
{
completionHandler(_shouldFollowRedirects ? request : nil);
}

@end

#else
Expand Down

0 comments on commit 46850d0

Please sign in to comment.