Ben Copsey

Fix a problem where requests would attempt to remove the temporary download file twice

@@ -492,7 +492,7 @@ static NSRecursiveLock *delegateAuthenticationLock = nil; @@ -492,7 +492,7 @@ static NSRecursiveLock *delegateAuthenticationLock = nil;
492 NSError *err = nil; 492 NSError *err = nil;
493 [self setPartialDownloadSize:[[[NSFileManager defaultManager] attributesOfItemAtPath:[self temporaryFileDownloadPath] error:&err] fileSize]]; 493 [self setPartialDownloadSize:[[[NSFileManager defaultManager] attributesOfItemAtPath:[self temporaryFileDownloadPath] error:&err] fileSize]];
494 if (err) { 494 if (err) {
495 - [self failWithError:[NSError errorWithDomain:NetworkRequestErrorDomain code:ASIFileManagementError userInfo:[NSDictionary dictionaryWithObjectsAndKeys:[NSString stringWithFormat:@"Failed to get attributes for file at path '@%'",temporaryFileDownloadPath],NSLocalizedDescriptionKey,error,NSUnderlyingErrorKey,nil]]]; 495 + [self failWithError:[NSError errorWithDomain:NetworkRequestErrorDomain code:ASIFileManagementError userInfo:[NSDictionary dictionaryWithObjectsAndKeys:[NSString stringWithFormat:@"Failed to get attributes for file at path '@%'",[self temporaryFileDownloadPath]],NSLocalizedDescriptionKey,error,NSUnderlyingErrorKey,nil]]];
496 return; 496 return;
497 } 497 }
498 [self addRequestHeader:@"Range" value:[NSString stringWithFormat:@"bytes=%llu-",[self partialDownloadSize]]]; 498 [self addRequestHeader:@"Range" value:[NSString stringWithFormat:@"bytes=%llu-",[self partialDownloadSize]]];
@@ -773,21 +773,16 @@ static NSRecursiveLock *delegateAuthenticationLock = nil; @@ -773,21 +773,16 @@ static NSRecursiveLock *delegateAuthenticationLock = nil;
773 773
774 [[self postBodyReadStream] close]; 774 [[self postBodyReadStream] close];
775 775
776 - if (rawResponseData) { 776 + if ([self rawResponseData]) {
777 [self setRawResponseData:nil]; 777 [self setRawResponseData:nil];
778 778
779 // If we were downloading to a file 779 // If we were downloading to a file
780 - } else if (temporaryFileDownloadPath) { 780 + } else if ([self temporaryFileDownloadPath]) {
781 - [fileDownloadOutputStream close]; 781 + [[self fileDownloadOutputStream] close];
782 782
783 // If we haven't said we might want to resume, let's remove the temporary file too 783 // If we haven't said we might want to resume, let's remove the temporary file too
784 if (![self allowResumeForFileDownloads]) { 784 if (![self allowResumeForFileDownloads]) {
785 - NSError *err = nil; 785 + [self removeTemporaryDownloadFile];
786 - [[NSFileManager defaultManager] removeItemAtPath:temporaryFileDownloadPath error:&err];  
787 - if (err) {  
788 - [self failWithError:[NSError errorWithDomain:NetworkRequestErrorDomain code:ASIFileManagementError userInfo:[NSDictionary dictionaryWithObjectsAndKeys:[NSString stringWithFormat:@"Failed to remove temporary file at path '@%'",temporaryFileDownloadPath],NSLocalizedDescriptionKey,error,NSUnderlyingErrorKey,nil]]];  
789 - return;  
790 - }  
791 } 786 }
792 } 787 }
793 788
@@ -804,11 +799,13 @@ static NSRecursiveLock *delegateAuthenticationLock = nil; @@ -804,11 +799,13 @@ static NSRecursiveLock *delegateAuthenticationLock = nil;
804 799
805 - (void)removeTemporaryDownloadFile 800 - (void)removeTemporaryDownloadFile
806 { 801 {
807 - if (temporaryFileDownloadPath) { 802 + if ([self temporaryFileDownloadPath]) {
  803 + if ([[NSFileManager defaultManager] fileExistsAtPath:[self temporaryFileDownloadPath]]) {
808 NSError *removeError = nil; 804 NSError *removeError = nil;
809 - [[NSFileManager defaultManager] removeItemAtPath:temporaryFileDownloadPath error:&removeError]; 805 + [[NSFileManager defaultManager] removeItemAtPath:[self temporaryFileDownloadPath] error:&removeError];
810 if (removeError) { 806 if (removeError) {
811 - [self failWithError:[NSError errorWithDomain:NetworkRequestErrorDomain code:ASIFileManagementError userInfo:[NSDictionary dictionaryWithObjectsAndKeys:[NSString stringWithFormat:@"Failed to delete file at path '%@'",temporaryFileDownloadPath],NSLocalizedDescriptionKey,removeError,NSUnderlyingErrorKey,nil]]]; 807 + [self failWithError:[NSError errorWithDomain:NetworkRequestErrorDomain code:ASIFileManagementError userInfo:[NSDictionary dictionaryWithObjectsAndKeys:[NSString stringWithFormat:@"Failed to delete file at path '%@'",[self temporaryFileDownloadPath]],NSLocalizedDescriptionKey,removeError,NSUnderlyingErrorKey,nil]]];
  808 + }
812 } 809 }
813 [self setTemporaryFileDownloadPath:nil]; 810 [self setTemporaryFileDownloadPath:nil];
814 } 811 }
@@ -1936,10 +1933,10 @@ static NSRecursiveLock *delegateAuthenticationLock = nil; @@ -1936,10 +1933,10 @@ static NSRecursiveLock *delegateAuthenticationLock = nil;
1936 append = YES; 1933 append = YES;
1937 } 1934 }
1938 1935
1939 - [self setFileDownloadOutputStream:[[[NSOutputStream alloc] initToFileAtPath:temporaryFileDownloadPath append:append] autorelease]]; 1936 + [self setFileDownloadOutputStream:[[[NSOutputStream alloc] initToFileAtPath:[self temporaryFileDownloadPath] append:append] autorelease]];
1940 - [fileDownloadOutputStream open]; 1937 + [[self fileDownloadOutputStream] open];
1941 } 1938 }
1942 - [fileDownloadOutputStream write:buffer maxLength:bytesRead]; 1939 + [[self fileDownloadOutputStream] write:buffer maxLength:bytesRead];
1943 1940
1944 //Otherwise, let's add the data to our in-memory store 1941 //Otherwise, let's add the data to our in-memory store
1945 } else { 1942 } else {
@@ -1979,19 +1976,19 @@ static NSRecursiveLock *delegateAuthenticationLock = nil; @@ -1979,19 +1976,19 @@ static NSRecursiveLock *delegateAuthenticationLock = nil;
1979 NSError *fileError = nil; 1976 NSError *fileError = nil;
1980 1977
1981 // Delete up the request body temporary file, if it exists 1978 // Delete up the request body temporary file, if it exists
1982 - if (didCreateTemporaryPostDataFile) { 1979 + if ([self didCreateTemporaryPostDataFile]) {
1983 [self removePostDataFile]; 1980 [self removePostDataFile];
1984 } 1981 }
1985 1982
1986 // Close the output stream as we're done writing to the file 1983 // Close the output stream as we're done writing to the file
1987 - if (temporaryFileDownloadPath) { 1984 + if ([self temporaryFileDownloadPath]) {
1988 - [fileDownloadOutputStream close]; 1985 + [[self fileDownloadOutputStream] close];
1989 1986
1990 // Decompress the file (if necessary) directly to the destination path 1987 // Decompress the file (if necessary) directly to the destination path
1991 if ([self isResponseCompressed]) { 1988 if ([self isResponseCompressed]) {
1992 - int decompressionStatus = [ASIHTTPRequest uncompressZippedDataFromFile:temporaryFileDownloadPath toFile:downloadDestinationPath]; 1989 + int decompressionStatus = [ASIHTTPRequest uncompressZippedDataFromFile:[self temporaryFileDownloadPath] toFile:[self downloadDestinationPath]];
1993 if (decompressionStatus != Z_OK) { 1990 if (decompressionStatus != Z_OK) {
1994 - fileError = [NSError errorWithDomain:NetworkRequestErrorDomain code:ASIFileManagementError userInfo:[NSDictionary dictionaryWithObjectsAndKeys:[NSString stringWithFormat:@"Decompression of %@ failed with code %hi",temporaryFileDownloadPath,decompressionStatus],NSLocalizedDescriptionKey,nil]]; 1991 + fileError = [NSError errorWithDomain:NetworkRequestErrorDomain code:ASIFileManagementError userInfo:[NSDictionary dictionaryWithObjectsAndKeys:[NSString stringWithFormat:@"Decompression of %@ failed with code %hi",[self temporaryFileDownloadPath],decompressionStatus],NSLocalizedDescriptionKey,nil]];
1995 } 1992 }
1996 1993
1997 [self removeTemporaryDownloadFile]; 1994 [self removeTemporaryDownloadFile];
@@ -1999,19 +1996,20 @@ static NSRecursiveLock *delegateAuthenticationLock = nil; @@ -1999,19 +1996,20 @@ static NSRecursiveLock *delegateAuthenticationLock = nil;
1999 1996
2000 //Remove any file at the destination path 1997 //Remove any file at the destination path
2001 NSError *moveError = nil; 1998 NSError *moveError = nil;
2002 - if ([[NSFileManager defaultManager] fileExistsAtPath:downloadDestinationPath]) { 1999 + if ([[NSFileManager defaultManager] fileExistsAtPath:[self downloadDestinationPath]]) {
2003 - [[NSFileManager defaultManager] removeItemAtPath:downloadDestinationPath error:&moveError]; 2000 + [[NSFileManager defaultManager] removeItemAtPath:[self downloadDestinationPath] error:&moveError];
2004 if (moveError) { 2001 if (moveError) {
2005 - fileError = [NSError errorWithDomain:NetworkRequestErrorDomain code:ASIFileManagementError userInfo:[NSDictionary dictionaryWithObjectsAndKeys:[NSString stringWithFormat:@"Unable to remove file at path '%@'",downloadDestinationPath],NSLocalizedDescriptionKey,moveError,NSUnderlyingErrorKey,nil]]; 2002 + fileError = [NSError errorWithDomain:NetworkRequestErrorDomain code:ASIFileManagementError userInfo:[NSDictionary dictionaryWithObjectsAndKeys:[NSString stringWithFormat:@"Unable to remove file at path '%@'",[self downloadDestinationPath]],NSLocalizedDescriptionKey,moveError,NSUnderlyingErrorKey,nil]];
2006 } 2003 }
2007 } 2004 }
2008 2005
2009 //Move the temporary file to the destination path 2006 //Move the temporary file to the destination path
2010 if (!fileError) { 2007 if (!fileError) {
2011 - [[NSFileManager defaultManager] moveItemAtPath:temporaryFileDownloadPath toPath:downloadDestinationPath error:&moveError]; 2008 + [[NSFileManager defaultManager] moveItemAtPath:[self temporaryFileDownloadPath] toPath:[self downloadDestinationPath] error:&moveError];
2012 if (moveError) { 2009 if (moveError) {
2013 - fileError = [NSError errorWithDomain:NetworkRequestErrorDomain code:ASIFileManagementError userInfo:[NSDictionary dictionaryWithObjectsAndKeys:[NSString stringWithFormat:@"Failed to move file from '%@' to '%@'",temporaryFileDownloadPath,downloadDestinationPath],NSLocalizedDescriptionKey,moveError,NSUnderlyingErrorKey,nil]]; 2010 + fileError = [NSError errorWithDomain:NetworkRequestErrorDomain code:ASIFileManagementError userInfo:[NSDictionary dictionaryWithObjectsAndKeys:[NSString stringWithFormat:@"Failed to move file from '%@' to '%@'",[self temporaryFileDownloadPath]],[self downloadDestinationPath],NSLocalizedDescriptionKey,moveError,NSUnderlyingErrorKey,nil]];
2014 } 2011 }
  2012 + [self setTemporaryFileDownloadPath:nil];
2015 } 2013 }
2016 } 2014 }
2017 } 2015 }
@@ -40,4 +40,5 @@ @@ -40,4 +40,5 @@
40 - (void)testThrottlingDownloadBandwidth; 40 - (void)testThrottlingDownloadBandwidth;
41 - (void)testThrottlingUploadBandwidth; 41 - (void)testThrottlingUploadBandwidth;
42 - (void)testMainThreadDelegateAuthenticationFailure; 42 - (void)testMainThreadDelegateAuthenticationFailure;
  43 +- (void)testFileCleanupWorks;
43 @end 44 @end
@@ -898,6 +898,26 @@ @@ -898,6 +898,26 @@
898 GHAssertTrue(success,@"Got wrong response status message"); 898 GHAssertTrue(success,@"Got wrong response status message");
899 } 899 }
900 900
  901 +
  902 +- (void)handleDownloadFailed:(ASIHTTPRequest *)request
  903 +{
  904 + GHFail(@"Download failed for file cleanup test");
  905 +}
  906 +
  907 +// Test for a bug that existed that would attempt to remove the temporary download file twice
  908 +- (void)testFileCleanupWorks
  909 +{
  910 + ASIHTTPRequest *request = [ASIHTTPRequest requestWithURL:[NSURL URLWithString:@"http://allseeing-i.com/i/logo.png"]];
  911 + NSString *path = [[self filePathForTemporaryTestFiles] stringByAppendingPathComponent:@"test.png"];
  912 + if ([[NSFileManager defaultManager] fileExistsAtPath:path]) {
  913 + [[NSFileManager defaultManager] removeItemAtPath:path error:nil];
  914 + }
  915 + [request setDownloadDestinationPath:path];
  916 + [request setDelegate:self];
  917 + [request setDidFailSelector:@selector(handleDownloadFailed:)];
  918 + [request start];
  919 +}
  920 +
901 @end 921 @end
902 922
903 923