Fix redirect race condition

CHANGELOG.md

@@ -6,6 +6,12 @@
 * Improved documentation about `dynamicType:` vs `type(of:)`.  
   [Antondomashnev](https://github.com/Antondomashnev)
   [#221](https://github.com/AliSoftware/OHHTTPStubs/pull/221)
+* Fixed a race condition that occasionally prevented redirect callbacks.  
+  [@morrowa](https://github.com/morrowa)
+  [#224](https://github.com/AliSoftware/OHHTTPStubs/pull/224)
+* Fixed response timing for zero-length stub data.  
+  [@morrowa](https://github.com/morrowa)
+  [#224](https://github.com/AliSoftware/OHHTTPStubs/pull/224)
 
 ## [5.2.2](https://github.com/AliSoftware/OHHTTPStubs/releases/tag/5.2.2)
 

OHHTTPStubs/Sources/OHHTTPStubs.m

@@ -418,25 +418,21 @@ - (void)startLoading
         {
             redirectLocationURL = nil;
         }
-        // Notify if a redirection occurred
-        if (((responseStub.statusCode > 300) && (responseStub.statusCode < 400)) && redirectLocationURL)
-        {
-            NSURLRequest* redirectRequest = [NSURLRequest requestWithURL:redirectLocationURL];
-            [self executeOnClientRunLoopAfterDelay:responseStub.requestTime block:^{
-                if (!self.stopped)
+        [self executeOnClientRunLoopAfterDelay:responseStub.requestTime block:^{
+            if (!self.stopped)
+            {
+                // Notify if a redirection occurred
+                if (((responseStub.statusCode > 300) && (responseStub.statusCode < 400)) && redirectLocationURL)
                 {
+                    NSURLRequest* redirectRequest = [NSURLRequest requestWithURL:redirectLocationURL];
                     [client URLProtocol:self wasRedirectedToRequest:redirectRequest redirectResponse:urlResponse];
                     if (OHHTTPStubs.sharedInstance.onStubRedirectBlock)
                     {
                         OHHTTPStubs.sharedInstance.onStubRedirectBlock(request, redirectRequest, self.stub, responseStub);
                     }
                 }
-            }];
-        }
-        // Send the response (even for redirections)
-        [self executeOnClientRunLoopAfterDelay:responseStub.requestTime block:^{
-            if (!self.stopped)
-            {
+
+                // Send the response (even for redirections)
                 [client URLProtocol:self didReceiveResponse:urlResponse cacheStoragePolicy:NSURLCacheStorageNotAllowed];
                 if(responseStub.inputStream.streamStatus == NSStreamStatusNotOpen)
                 {
@@ -494,42 +490,47 @@ - (void)streamDataForClient:(id<NSURLProtocolClient>)client
            withStubResponse:(OHHTTPStubsResponse*)stubResponse
                  completion:(void(^)(NSError * error))completion
 {
-    if ((stubResponse.dataSize>0) && stubResponse.inputStream.hasBytesAvailable && (!self.stopped))
+    if (!self.stopped)
     {
-        // Compute timing data once and for all for this stub
-
-        OHHTTPStubsStreamTimingInfo timingInfo = {
-            .slotTime = kSlotTime, // Must be >0. We will send a chunk of data from the stream each 'slotTime' seconds
-            .cumulativeChunkSize = 0
-        };
-
-        if(stubResponse.responseTime < 0)
-        {
-            // Bytes send each 'slotTime' seconds = Speed in KB/s * 1000 * slotTime in seconds
-            timingInfo.chunkSizePerSlot = (fabs(stubResponse.responseTime) * 1000) * timingInfo.slotTime;
-        }
-        else if (stubResponse.responseTime < kSlotTime) // includes case when responseTime == 0
+        if ((stubResponse.dataSize>0) && stubResponse.inputStream.hasBytesAvailable)
         {
-            // We want to send the whole data quicker than the slotTime, so send it all in one chunk.
-            timingInfo.chunkSizePerSlot = stubResponse.dataSize;
-            timingInfo.slotTime = stubResponse.responseTime;
+            // Compute timing data once and for all for this stub
+
+            OHHTTPStubsStreamTimingInfo timingInfo = {
+                .slotTime = kSlotTime, // Must be >0. We will send a chunk of data from the stream each 'slotTime' seconds
+                .cumulativeChunkSize = 0
+            };
+
+            if(stubResponse.responseTime < 0)
+            {
+                // Bytes send each 'slotTime' seconds = Speed in KB/s * 1000 * slotTime in seconds
+                timingInfo.chunkSizePerSlot = (fabs(stubResponse.responseTime) * 1000) * timingInfo.slotTime;
+            }
+            else if (stubResponse.responseTime < kSlotTime) // includes case when responseTime == 0
+            {
+                // We want to send the whole data quicker than the slotTime, so send it all in one chunk.
+                timingInfo.chunkSizePerSlot = stubResponse.dataSize;
+                timingInfo.slotTime = stubResponse.responseTime;
+            }
+            else
+            {
+                // Bytes send each 'slotTime' seconds = (Whole size in bytes / response time) * slotTime = speed in bps * slotTime in seconds
+                timingInfo.chunkSizePerSlot = ((stubResponse.dataSize/stubResponse.responseTime) * timingInfo.slotTime);
+            }
+
+            [self streamDataForClient:client
+                           fromStream:stubResponse.inputStream
+                           timingInfo:timingInfo
+                           completion:completion];
         }
         else
         {
-            // Bytes send each 'slotTime' seconds = (Whole size in bytes / response time) * slotTime = speed in bps * slotTime in seconds
-            timingInfo.chunkSizePerSlot = ((stubResponse.dataSize/stubResponse.responseTime) * timingInfo.slotTime);
-        }
-
-        [self streamDataForClient:client
-                       fromStream:stubResponse.inputStream
-                       timingInfo:timingInfo
-                       completion:completion];
-    }
-    else
-    {
-        if (completion)
-        {
-            completion(nil);
+            [self executeOnClientRunLoopAfterDelay:stubResponse.responseTime block:^{
+                if (completion && !self.stopped)
+                {
+                    completion(nil);
+                }
+            }];
         }
     }
 }

OHHTTPStubs/UnitTests/Test Suites/NSURLSessionTests.m

@@ -97,9 +97,9 @@ - (void)_test_NSURLSession:(NSURLSession*)session
 
         [task resume];
 
-        [self waitForExpectationsWithTimeout:kRequestTime+kResponseTime+0.5 handler:nil];
-
-        completion(errorResponse, dataResponse);
+        [self waitForExpectationsWithTimeout:kRequestTime+kResponseTime+0.5 handler:^(NSError * _Nullable error) {
+            completion(errorResponse, dataResponse);
+        }];
     }
 }
 
@@ -109,7 +109,7 @@ - (void)_test_redirect_NSURLSession:(NSURLSession*)session
 {
     if ([NSURLSession class])
     {
-        static const NSTimeInterval kRequestTime = 0.0;
+        static const NSTimeInterval kRequestTime = 0.2;
         static const NSTimeInterval kResponseTime = 0.2;
 
         [OHHTTPStubs stubRequestsPassingTest:^BOOL(NSURLRequest *request) {
@@ -156,11 +156,22 @@ - (void)_test_redirect_NSURLSession:(NSURLSession*)session
                                            [expectation fulfill];
                                        }];
 
+        NSDate *startTime = [NSDate date];
         [task resume];
 
-        [self waitForExpectationsWithTimeout:kRequestTime+kResponseTime+0.5 handler:nil];
-
-        completion(errorResponse, redirectResponse, dataResponse);
+        [self waitForExpectationsWithTimeout:(kRequestTime+kResponseTime)*2+0.1 handler:^(NSError * _Nullable error) {
+            NSDate *finishTime = [NSDate date];
+            NSTimeInterval totalResponseTime = [finishTime timeIntervalSinceDate:startTime];
+            if (redirectResponse) {
+                XCTAssertGreaterThanOrEqual(totalResponseTime, (kRequestTime + kResponseTime), @"Redirect did not honor request/response time");
+            }
+            else if (dataResponse) {
+                // NSURLSession does not wait for the 3xx response to stream before starting the second request.
+                // Thus, the 3xx and final responses will stream in parallel.
+                XCTAssertGreaterThanOrEqual(totalResponseTime, ((2 * kRequestTime) + kResponseTime), @"Redirect or final request did not honor request/response time");
+            }
+            completion(errorResponse, redirectResponse, dataResponse);
+        }];
     }
 }