NSURLSession should throw for creating tasks with nil request or resu…

…me data (#1860) Fixes #1844 * Check for nil before trying to resume * CR feedback
microsoft · Jan 30, 2017 · 9aaa5ef · 9aaa5ef
1 parent 794fbfd
commit 9aaa5ef
Show file tree

Hide file tree

Showing 5 changed files with 69 additions and 9 deletions.
diff --git a/Frameworks/Foundation/NSURLSession.mm b/Frameworks/Foundation/NSURLSession.mm
@@ -1,6 +1,6 @@
 //******************************************************************************
 //
-// Copyright (c) 2015 Microsoft Corporation. All rights reserved.
+// Copyright (c) Microsoft. All rights reserved.
 //
 // This code is licensed under the MIT License (MIT).
 //
@@ -33,6 +33,21 @@
 
 #import <StubReturn.h>
 
+#define _THROW_IF_NULL_REQUEST(request)                                        \
+do {                                                                           \
+    if (request == nil) {                                                      \
+      @throw [NSException                                                      \
+          exceptionWithName:NSInvalidArgumentException                         \
+                     reason:[NSString                                          \
+                                stringWithFormat:@"*** %@ Cannot create data " \
+                                                 @"task without request or "   \
+                                                 @"resume data",               \
+                                                 NSStringFromSelector(_cmd)]   \
+                   userInfo:nil];                                              \
+    }                                                                          \
+}                                                                              \
+  while (false)
+
 NSString* const NSURLErrorBackgroundTaskCancelledReasonKey = @"NSURLErrorBackgroundTaskCancelledReasonKey";
 
 template <typename... Args>
@@ -305,6 +320,7 @@ - (NSURLSessionDataTask*)dataTaskWithRequest:(NSURLRequest*)request {
 @Status Interoperable
 */
 - (NSURLSessionDataTask*)dataTaskWithRequest:(NSURLRequest*)request completionHandler:(NSURLSessionTaskCompletionHandler)completionHandler {
+    _THROW_IF_NULL_REQUEST(request);
     if (_invalidating) {
         return nil;
     }
@@ -344,6 +360,7 @@ - (NSURLSessionDownloadTask*)downloadTaskWithRequest:(NSURLRequest*)request {
 */
 - (NSURLSessionDownloadTask*)downloadTaskWithRequest:(NSURLRequest*)request
                                    completionHandler:(NSURLSessionDownloadTaskCompletionHandler)completionHandler {
+    _THROW_IF_NULL_REQUEST(request);
     if (_invalidating) {
         return nil;
     }
@@ -368,6 +385,7 @@ - (NSURLSessionDownloadTask*)downloadTaskWithResumeData:(NSData*)resumeData {
 */
 - (NSURLSessionDownloadTask*)downloadTaskWithResumeData:(NSData*)resumeData
                                       completionHandler:(NSURLSessionDownloadTaskCompletionHandler)completionHandler {
+    _THROW_IF_NULL_REQUEST(resumeData);
     if (_invalidating) {
         return nil;
     }

diff --git a/build/Tests/UnitTests/Foundation/Foundation.UnitTests.vcxproj b/build/Tests/UnitTests/Foundation/Foundation.UnitTests.vcxproj
@@ -324,6 +324,7 @@
     <ClangCompile Include="$(StarboardBasePath)\tests\unittests\Foundation\ReferenceFoundation\TestNSPropertyList.mm" />
     <ClangCompile Include="$(StarboardBasePath)\tests\unittests\Foundation\ReferenceFoundation\TestNSTimeZone.mm" />
     <ClangCompile Include="$(StarboardBasePath)\tests\unittests\Foundation\NSUndoManagerTests.mm" />
+    <ClangCompile Include="$(StarboardBasePath)\tests\UnitTests\Foundation\NSURLSessionTests.mm" />
   </ItemGroup>
   <ItemGroup>
     <Text Include="..\..\..\tests\unittests\Foundation\NSFileManagerUT.txt">

diff --git a/build/Tests/UnitTests/Foundation/Foundation.UnitTests.vcxproj.filters b/build/Tests/UnitTests/Foundation/Foundation.UnitTests.vcxproj.filters
@@ -139,6 +139,8 @@
     </ClangCompile>
     <ClangCompile Include="$(StarboardBasePath)\tests\unittests\Foundation\NSURLResponseTests.mm" />
     <ClangCompile Include="$(StarboardBasePath)\tests\unittests\Foundation\NSUndoManagerTests.mm" />
+    <ClangCompile Include="$(StarboardBasePath)\tests\unittests\Foundation\NSDecimalNumberTests.mm" />
+    <ClangCompile Include="$(StarboardBasePath)\tests\UnitTests\Foundation\NSURLSessionTests.mm" />
   </ItemGroup>
   <ItemGroup>
     <Text Include="..\..\..\tests\unittests\Foundation\NSFileManagerUT.txt" />
@@ -149,4 +151,4 @@
       <UniqueIdentifier>{1beaf0a2-ff8a-415b-ac31-aeb41c89f25d}</UniqueIdentifier>
     </Filter>
   </ItemGroup>
-</Project>
+</Project>
diff --git a/tests/UnitTests/Foundation/NSURLSessionTests.mm b/tests/UnitTests/Foundation/NSURLSessionTests.mm
@@ -0,0 +1,36 @@
+//******************************************************************************
+//
+// Copyright (c) Microsoft. All rights reserved.
+//
+// This code is licensed under the MIT License (MIT).
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+// THE SOFTWARE.
+//
+//******************************************************************************
+
+#import <Foundation/NSURLSession.h>
+#import <TestFramework.h>
+#import <Starboard/SmartTypes.h>
+
+TEST(NSURLSession, ShouldThrowForNilRequest) {
+    NSURLSession* session = [NSURLSession sessionWithConfiguration:[NSURLSessionConfiguration defaultSessionConfiguration]];
+    EXPECT_ANY_THROW([session downloadTaskWithRequest:nil]);
+    EXPECT_ANY_THROW([session dataTaskWithRequest:nil]);
+}
+
+TEST(NSURLSession, ShouldThrowForNilResumeData) {
+    NSURLSession* session = [NSURLSession sessionWithConfiguration:[NSURLSessionConfiguration defaultSessionConfiguration]];
+    EXPECT_ANY_THROW([session downloadTaskWithResumeData:nil]);
+}
+
+TEST(NSURLSession, ShouldNOTThrowForNilURL) {
+    NSURLSession* session = [NSURLSession sessionWithConfiguration:[NSURLSessionConfiguration defaultSessionConfiguration]];
+    EXPECT_NO_THROW([session downloadTaskWithURL:nil]);
+    EXPECT_NO_THROW([session dataTaskWithURL:nil]);
+}
diff --git a/tests/testapps/Foundation-Dev/Foundation-Dev/URLSessionViewController.mm b/tests/testapps/Foundation-Dev/Foundation-Dev/URLSessionViewController.mm
@@ -276,14 +276,17 @@ - (void)cancelLastButtonPressed:(id)sender {
 
 - (void)resumeLastButtonPressed:(id)sender {
     NSURLSessionDownloadTask* newTask = nil;
-    if ([_blockSwitch isOn]) {
-        newTask = [_urlSession downloadTaskWithResumeData:_lastResumeData
-                                        completionHandler:^(NSURL* location, NSURLResponse* response, NSError* error) {
-                                            [self _printOutput:@"Resumed Task completed to %@: %@, %@\n", location, response, error];
-                                        }];
-    } else {
-        newTask = [_urlSession downloadTaskWithResumeData:_lastResumeData];
+    if (_lastResumeData != nil) {
+        if ([_blockSwitch isOn]) {
+            newTask = [_urlSession downloadTaskWithResumeData:_lastResumeData
+                                            completionHandler:^(NSURL* location, NSURLResponse* response, NSError* error) {
+                                                [self _printOutput:@"Resumed Task completed to %@: %@, %@\n", location, response, error];
+                                            }];
+        } else {
+            newTask = [_urlSession downloadTaskWithResumeData:_lastResumeData];
+        }
     }
+
     if (!newTask) {
         [self _printOutput:@"There wasn't anything to resume.\n"];
         return;