Ben Copsey

Stop using kCFStreamPropertyHTTPShouldAutoredirect and handle 30x redirection ourselves

This ensures cookies presented as part of the initial response can be applied to the redirected request
sessionCookies should now hang on to all cookies, and cookies will replace old versions of the same cookie
@@ -26,7 +26,8 @@ typedef enum _ASINetworkErrorType { @@ -26,7 +26,8 @@ typedef enum _ASINetworkErrorType {
26 ASIUnableToCreateRequestErrorType = 5, 26 ASIUnableToCreateRequestErrorType = 5,
27 ASIInternalErrorWhileBuildingRequestType = 6, 27 ASIInternalErrorWhileBuildingRequestType = 6,
28 ASIInternalErrorWhileApplyingCredentialsType = 7, 28 ASIInternalErrorWhileApplyingCredentialsType = 7,
29 - ASIFileManagementError = 8 29 + ASIFileManagementError = 8,
  30 + ASITooMuchRedirectionErrorType = 9
30 31
31 } ASINetworkErrorType; 32 } ASINetworkErrorType;
32 33
@@ -218,6 +219,12 @@ extern NSString* const NetworkRequestErrorDomain; @@ -218,6 +219,12 @@ extern NSString* const NetworkRequestErrorDomain;
218 // When YES, requests will automatically redirect when they get a HTTP 30x header (defaults to YES) 219 // When YES, requests will automatically redirect when they get a HTTP 30x header (defaults to YES)
219 BOOL shouldRedirect; 220 BOOL shouldRedirect;
220 221
  222 + // Used internally to tell the main loop we need to stop and retry with a new url
  223 + BOOL needsRedirect;
  224 +
  225 + // Incremented every time this request redirects. When it reaches 5, we give up
  226 + int redirectCount;
  227 +
221 // When NO, requests will not check the secure certificate is valid (use for self-signed cerficates during development, DO NOT USE IN PRODUCTION) Default is YES 228 // When NO, requests will not check the secure certificate is valid (use for self-signed cerficates during development, DO NOT USE IN PRODUCTION) Default is YES
222 BOOL validatesSecureCertificate; 229 BOOL validatesSecureCertificate;
223 230
@@ -297,7 +304,9 @@ extern NSString* const NetworkRequestErrorDomain; @@ -297,7 +304,9 @@ extern NSString* const NetworkRequestErrorDomain;
297 304
298 #pragma mark http authentication stuff 305 #pragma mark http authentication stuff
299 306
300 -// Reads the response headers to find the content length, and returns true if the request needs a username and password (or if those supplied were incorrect) 307 +// Reads the response headers to find the content length, encoding, cookies for the session
  308 +// Also initiates request redirection when shouldRedirect is true
  309 +// Returns true if the request needs a username and password (or if those supplied were incorrect)
301 - (BOOL)readResponseHeadersReturningAuthenticationFailure; 310 - (BOOL)readResponseHeadersReturningAuthenticationFailure;
302 311
303 // Apply credentials to this request 312 // Apply credentials to this request
@@ -345,6 +354,9 @@ extern NSString* const NetworkRequestErrorDomain; @@ -345,6 +354,9 @@ extern NSString* const NetworkRequestErrorDomain;
345 + (void)setSessionCookies:(NSMutableArray *)newSessionCookies; 354 + (void)setSessionCookies:(NSMutableArray *)newSessionCookies;
346 + (NSMutableArray *)sessionCookies; 355 + (NSMutableArray *)sessionCookies;
347 356
  357 +// Adds a cookie to our list of cookies we've accepted, checking first for an old version of the same cookie and removing that
  358 ++ (void)addSessionCookie:(NSHTTPCookie *)newCookie;
  359 +
348 // Dump all session data (authentication and cookies) 360 // Dump all session data (authentication and cookies)
349 + (void)clearSession; 361 + (void)clearSession;
350 362
@@ -27,6 +27,8 @@ static CFHTTPAuthenticationRef sessionAuthentication = NULL; @@ -27,6 +27,8 @@ static CFHTTPAuthenticationRef sessionAuthentication = NULL;
27 static NSMutableDictionary *sessionCredentials = nil; 27 static NSMutableDictionary *sessionCredentials = nil;
28 static NSMutableArray *sessionCookies = nil; 28 static NSMutableArray *sessionCookies = nil;
29 29
  30 +// The number of times we will allow requests to redirect before we fail with a redirection error
  31 +const int RedirectionLimit = 5;
30 32
31 static void ReadStreamClientCallBack(CFReadStreamRef readStream, CFStreamEventType type, void *clientCallBackInfo) { 33 static void ReadStreamClientCallBack(CFReadStreamRef readStream, CFStreamEventType type, void *clientCallBackInfo) {
32 [((ASIHTTPRequest*)clientCallBackInfo) handleNetworkEvent: type]; 34 [((ASIHTTPRequest*)clientCallBackInfo) handleNetworkEvent: type];
@@ -39,6 +41,8 @@ static NSError *ASIRequestCancelledError; @@ -39,6 +41,8 @@ static NSError *ASIRequestCancelledError;
39 static NSError *ASIRequestTimedOutError; 41 static NSError *ASIRequestTimedOutError;
40 static NSError *ASIAuthenticationError; 42 static NSError *ASIAuthenticationError;
41 static NSError *ASIUnableToCreateRequestError; 43 static NSError *ASIUnableToCreateRequestError;
  44 +static NSError *ASITooMuchRedirectionError;
  45 +
42 46
43 // Private stuff 47 // Private stuff
44 @interface ASIHTTPRequest () 48 @interface ASIHTTPRequest ()
@@ -64,6 +68,8 @@ static NSError *ASIUnableToCreateRequestError; @@ -64,6 +68,8 @@ static NSError *ASIUnableToCreateRequestError;
64 @property (retain, nonatomic) NSOutputStream *fileDownloadOutputStream; 68 @property (retain, nonatomic) NSOutputStream *fileDownloadOutputStream;
65 @property (assign, nonatomic) int authenticationRetryCount; 69 @property (assign, nonatomic) int authenticationRetryCount;
66 @property (assign, nonatomic) BOOL updatedProgress; 70 @property (assign, nonatomic) BOOL updatedProgress;
  71 + @property (assign, nonatomic) BOOL needsRedirect;
  72 + @property (assign, nonatomic) int redirectCount;
67 @end 73 @end
68 74
69 @implementation ASIHTTPRequest 75 @implementation ASIHTTPRequest
@@ -80,6 +86,8 @@ static NSError *ASIUnableToCreateRequestError; @@ -80,6 +86,8 @@ static NSError *ASIUnableToCreateRequestError;
80 ASIAuthenticationError = [[NSError errorWithDomain:NetworkRequestErrorDomain code:ASIAuthenticationErrorType userInfo:[NSDictionary dictionaryWithObjectsAndKeys:@"Authentication needed",NSLocalizedDescriptionKey,nil]] retain]; 86 ASIAuthenticationError = [[NSError errorWithDomain:NetworkRequestErrorDomain code:ASIAuthenticationErrorType userInfo:[NSDictionary dictionaryWithObjectsAndKeys:@"Authentication needed",NSLocalizedDescriptionKey,nil]] retain];
81 ASIRequestCancelledError = [[NSError errorWithDomain:NetworkRequestErrorDomain code:ASIRequestCancelledErrorType userInfo:[NSDictionary dictionaryWithObjectsAndKeys:@"The request was cancelled",NSLocalizedDescriptionKey,nil]] retain]; 87 ASIRequestCancelledError = [[NSError errorWithDomain:NetworkRequestErrorDomain code:ASIRequestCancelledErrorType userInfo:[NSDictionary dictionaryWithObjectsAndKeys:@"The request was cancelled",NSLocalizedDescriptionKey,nil]] retain];
82 ASIUnableToCreateRequestError = [[NSError errorWithDomain:NetworkRequestErrorDomain code:ASIUnableToCreateRequestErrorType userInfo:[NSDictionary dictionaryWithObjectsAndKeys:@"Unable to create request (bad url?)",NSLocalizedDescriptionKey,nil]] retain]; 88 ASIUnableToCreateRequestError = [[NSError errorWithDomain:NetworkRequestErrorDomain code:ASIUnableToCreateRequestErrorType userInfo:[NSDictionary dictionaryWithObjectsAndKeys:@"Unable to create request (bad url?)",NSLocalizedDescriptionKey,nil]] retain];
  89 + ASITooMuchRedirectionError = [[NSError errorWithDomain:NetworkRequestErrorDomain code:ASITooMuchRedirectionErrorType userInfo:[NSDictionary dictionaryWithObjectsAndKeys:@"The request failed because it redirected too many times",NSLocalizedDescriptionKey,nil]] retain];
  90 +
83 } 91 }
84 [super initialize]; 92 [super initialize];
85 } 93 }
@@ -436,10 +444,7 @@ static NSError *ASIUnableToCreateRequestError; @@ -436,10 +444,7 @@ static NSError *ASIUnableToCreateRequestError;
436 [self failWithError:[NSError errorWithDomain:NetworkRequestErrorDomain code:ASIInternalErrorWhileBuildingRequestType userInfo:[NSDictionary dictionaryWithObjectsAndKeys:@"Unable to create read stream",NSLocalizedDescriptionKey,nil]]]; 444 [self failWithError:[NSError errorWithDomain:NetworkRequestErrorDomain code:ASIInternalErrorWhileBuildingRequestType userInfo:[NSDictionary dictionaryWithObjectsAndKeys:@"Unable to create read stream",NSLocalizedDescriptionKey,nil]]];
437 return; 445 return;
438 } 446 }
439 - 447 +
440 - // Tell CFNetwork to automatically redirect for 30x status codes  
441 - CFReadStreamSetProperty(readStream, kCFStreamPropertyHTTPShouldAutoredirect, [self shouldRedirect] ? kCFBooleanTrue : kCFBooleanFalse);  
442 -  
443 // Tell CFNetwork not to validate SSL certificates 448 // Tell CFNetwork not to validate SSL certificates
444 if (!validatesSecureCertificate) { 449 if (!validatesSecureCertificate) {
445 CFReadStreamSetProperty(readStream, kCFStreamPropertySSLSettings, [NSMutableDictionary dictionaryWithObject:(NSString *)kCFBooleanFalse forKey:(NSString *)kCFStreamSSLValidatesCertificateChain]); 450 CFReadStreamSetProperty(readStream, kCFStreamPropertySSLSettings, [NSMutableDictionary dictionaryWithObject:(NSString *)kCFBooleanFalse forKey:(NSString *)kCFStreamSSLValidatesCertificateChain]);
@@ -517,7 +522,7 @@ static NSError *ASIUnableToCreateRequestError; @@ -517,7 +522,7 @@ static NSError *ASIUnableToCreateRequestError;
517 // See if we need to timeout 522 // See if we need to timeout
518 if (lastActivityTime && timeOutSeconds > 0 && [now timeIntervalSinceDate:lastActivityTime] > timeOutSeconds) { 523 if (lastActivityTime && timeOutSeconds > 0 && [now timeIntervalSinceDate:lastActivityTime] > timeOutSeconds) {
519 524
520 - // Prevent timeouts before 128KB has been sent when the size of data to upload is greater than 128KB 525 + // Prevent timeouts before 128KB* has been sent when the size of data to upload is greater than 128KB* (*32KB on iPhone 3.0 SDK)
521 // This is to workaround the fact that kCFStreamPropertyHTTPRequestBytesWrittenCount is the amount written to the buffer, not the amount actually sent 526 // This is to workaround the fact that kCFStreamPropertyHTTPRequestBytesWrittenCount is the amount written to the buffer, not the amount actually sent
522 // This workaround prevents erroneous timeouts in low bandwidth situations (eg iPhone) 527 // This workaround prevents erroneous timeouts in low bandwidth situations (eg iPhone)
523 if (contentLength <= uploadBufferSize || (uploadBufferSize > 0 && totalBytesSent > uploadBufferSize)) { 528 if (contentLength <= uploadBufferSize || (uploadBufferSize > 0 && totalBytesSent > uploadBufferSize)) {
@@ -528,7 +533,23 @@ static NSError *ASIUnableToCreateRequestError; @@ -528,7 +533,23 @@ static NSError *ASIUnableToCreateRequestError;
528 } 533 }
529 } 534 }
530 535
531 - // See if our NSOperationQueue told us to cancel 536 + // Do we need to redirect?
  537 + if ([self needsRedirect]) {
  538 + [self cancelLoad];
  539 + [self setNeedsRedirect:NO];
  540 + [self setRedirectCount:[self redirectCount]+1];
  541 + if ([self redirectCount] > RedirectionLimit) {
  542 + // Some naughty / badly coded website is trying to force us into a redirection loop. This is not cool.
  543 + [self failWithError:ASITooMuchRedirectionError];
  544 + [self setComplete:YES];
  545 + } else {
  546 + // Go all the way back to the beginning and build the request again, so that we can apply any new cookies
  547 + [self main];
  548 + }
  549 + break;
  550 + }
  551 +
  552 + // See if our NSOperationQueue told us to cancel or we need to redirect
532 if ([self isCancelled]) { 553 if ([self isCancelled]) {
533 break; 554 break;
534 } 555 }
@@ -944,7 +965,7 @@ static NSError *ASIUnableToCreateRequestError; @@ -944,7 +965,7 @@ static NSError *ASIUnableToCreateRequestError;
944 [self setResponseStatusCode:CFHTTPMessageGetResponseStatusCode(headers)]; 965 [self setResponseStatusCode:CFHTTPMessageGetResponseStatusCode(headers)];
945 966
946 // Is the server response a challenge for credentials? 967 // Is the server response a challenge for credentials?
947 - isAuthenticationChallenge = (responseStatusCode == 401); 968 + isAuthenticationChallenge = ([self responseStatusCode] == 401);
948 969
949 // We won't reset the download progress delegate if we got an authentication challenge 970 // We won't reset the download progress delegate if we got an authentication challenge
950 if (!isAuthenticationChallenge) { 971 if (!isAuthenticationChallenge) {
@@ -990,18 +1011,22 @@ static NSError *ASIUnableToCreateRequestError; @@ -990,18 +1011,22 @@ static NSError *ASIUnableToCreateRequestError;
990 NSArray *newCookies = [NSHTTPCookie cookiesWithResponseHeaderFields:responseHeaders forURL:url]; 1011 NSArray *newCookies = [NSHTTPCookie cookiesWithResponseHeaderFields:responseHeaders forURL:url];
991 [self setResponseCookies:newCookies]; 1012 [self setResponseCookies:newCookies];
992 1013
993 - if (useCookiePersistance) { 1014 + if ([self useCookiePersistance]) {
994 1015
995 // Store cookies in global persistent store 1016 // Store cookies in global persistent store
996 [[NSHTTPCookieStorage sharedHTTPCookieStorage] setCookies:newCookies forURL:url mainDocumentURL:nil]; 1017 [[NSHTTPCookieStorage sharedHTTPCookieStorage] setCookies:newCookies forURL:url mainDocumentURL:nil];
997 1018
998 // We also keep any cookies in the sessionCookies array, so that we have a reference to them if we need to remove them later 1019 // We also keep any cookies in the sessionCookies array, so that we have a reference to them if we need to remove them later
999 - if (!sessionCookies) { 1020 + NSHTTPCookie *cookie;
1000 - [ASIHTTPRequest setSessionCookies:[[[NSMutableArray alloc] init] autorelease]]; 1021 + for (cookie in newCookies) {
1001 - NSHTTPCookie *cookie; 1022 + [ASIHTTPRequest addSessionCookie:cookie];
1002 - for (cookie in newCookies) { 1023 + }
1003 - [[ASIHTTPRequest sessionCookies] addObject:cookie]; 1024 + }
1004 - } 1025 + // Do we need to redirect?
  1026 + if ([self shouldRedirect]) {
  1027 + if ([self responseStatusCode] > 300 && [self responseStatusCode] < 308 && [self responseStatusCode] != 304) {
  1028 + [self setURL:[[NSURL URLWithString:[responseHeaders valueForKey:@"Location"] relativeToURL:[self url]] absoluteURL]];
  1029 + [self setNeedsRedirect:YES];
1005 } 1030 }
1006 } 1031 }
1007 1032
@@ -1256,6 +1281,9 @@ static NSError *ASIUnableToCreateRequestError; @@ -1256,6 +1281,9 @@ static NSError *ASIUnableToCreateRequestError;
1256 return; 1281 return;
1257 } 1282 }
1258 } 1283 }
  1284 + if ([self needsRedirect]) {
  1285 + return;
  1286 + }
1259 int bufferSize = 2048; 1287 int bufferSize = 2048;
1260 if (contentLength > 262144) { 1288 if (contentLength > 262144) {
1261 bufferSize = 65536; 1289 bufferSize = 65536;
@@ -1307,6 +1335,9 @@ static NSError *ASIUnableToCreateRequestError; @@ -1307,6 +1335,9 @@ static NSError *ASIUnableToCreateRequestError;
1307 return; 1335 return;
1308 } 1336 }
1309 } 1337 }
  1338 + if ([self needsRedirect]) {
  1339 + return;
  1340 + }
1310 [progressLock lock]; 1341 [progressLock lock];
1311 [self setComplete:YES]; 1342 [self setComplete:YES];
1312 [self updateProgressIndicators]; 1343 [self updateProgressIndicators];
@@ -1371,8 +1402,6 @@ static NSError *ASIUnableToCreateRequestError; @@ -1371,8 +1402,6 @@ static NSError *ASIUnableToCreateRequestError;
1371 { 1402 {
1372 NSError *underlyingError = [(NSError *)CFReadStreamCopyError(readStream) autorelease]; 1403 NSError *underlyingError = [(NSError *)CFReadStreamCopyError(readStream) autorelease];
1373 1404
1374 -  
1375 -  
1376 [self cancelLoad]; 1405 [self cancelLoad];
1377 [self setComplete:YES]; 1406 [self setComplete:YES];
1378 1407
@@ -1459,19 +1488,37 @@ static NSError *ASIUnableToCreateRequestError; @@ -1459,19 +1488,37 @@ static NSError *ASIUnableToCreateRequestError;
1459 1488
1460 + (NSMutableArray *)sessionCookies 1489 + (NSMutableArray *)sessionCookies
1461 { 1490 {
  1491 + if (!sessionCookies) {
  1492 + [ASIHTTPRequest setSessionCookies:[[[NSMutableArray alloc] init] autorelease]];
  1493 + }
1462 return sessionCookies; 1494 return sessionCookies;
1463 } 1495 }
1464 1496
1465 + (void)setSessionCookies:(NSMutableArray *)newSessionCookies 1497 + (void)setSessionCookies:(NSMutableArray *)newSessionCookies
1466 { 1498 {
1467 // Remove existing cookies from the persistent store 1499 // Remove existing cookies from the persistent store
1468 - for (NSHTTPCookie *cookie in [ASIHTTPRequest sessionCookies]) { 1500 + for (NSHTTPCookie *cookie in sessionCookies) {
1469 [[NSHTTPCookieStorage sharedHTTPCookieStorage] deleteCookie:cookie]; 1501 [[NSHTTPCookieStorage sharedHTTPCookieStorage] deleteCookie:cookie];
1470 } 1502 }
1471 [sessionCookies release]; 1503 [sessionCookies release];
1472 sessionCookies = [newSessionCookies retain]; 1504 sessionCookies = [newSessionCookies retain];
1473 } 1505 }
1474 1506
  1507 ++ (void)addSessionCookie:(NSHTTPCookie *)newCookie
  1508 +{
  1509 + NSHTTPCookie *cookie;
  1510 + int i;
  1511 + int max = [[ASIHTTPRequest sessionCookies] count];
  1512 + for (i=0; i<max; i++) {
  1513 + cookie = [[ASIHTTPRequest sessionCookies] objectAtIndex:i];
  1514 + if ([[cookie domain] isEqualToString:[newCookie domain]] && [[cookie path] isEqualToString:[newCookie path]] && [[cookie name] isEqualToString:[newCookie name]]) {
  1515 + [[ASIHTTPRequest sessionCookies] removeObjectAtIndex:i];
  1516 + break;
  1517 + }
  1518 + }
  1519 + [[ASIHTTPRequest sessionCookies] addObject:newCookie];
  1520 +}
  1521 +
1475 // Dump all session data (authentication and cookies) 1522 // Dump all session data (authentication and cookies)
1476 + (void)clearSession 1523 + (void)clearSession
1477 { 1524 {
@@ -1676,4 +1723,6 @@ static NSError *ASIUnableToCreateRequestError; @@ -1676,4 +1723,6 @@ static NSError *ASIUnableToCreateRequestError;
1676 @synthesize updatedProgress; 1723 @synthesize updatedProgress;
1677 @synthesize shouldRedirect; 1724 @synthesize shouldRedirect;
1678 @synthesize validatesSecureCertificate; 1725 @synthesize validatesSecureCertificate;
  1726 +@synthesize needsRedirect;
  1727 +@synthesize redirectCount;
1679 @end 1728 @end
@@ -33,4 +33,6 @@ @@ -33,4 +33,6 @@
33 - (void)testCompressedResponse; 33 - (void)testCompressedResponse;
34 - (void)testCompressedResponseDownloadToFile; 34 - (void)testCompressedResponseDownloadToFile;
35 - (void)testSSL; 35 - (void)testSSL;
  36 +- (void)testRedirectPreservesSession;
  37 +- (void)testTooMuchRedirection;
36 @end 38 @end
@@ -639,4 +639,24 @@ @@ -639,4 +639,24 @@
639 GHAssertNil([request error],@"Failed to accept a self-signed certificate"); 639 GHAssertNil([request error],@"Failed to accept a self-signed certificate");
640 } 640 }
641 641
  642 +- (void)testRedirectPreservesSession
  643 +{
  644 + // Remove any old session cookies
  645 + [ASIHTTPRequest clearSession];
  646 + ASIHTTPRequest *request = [ASIHTTPRequest requestWithURL:[NSURL URLWithString:@"http://asi/ASIHTTPRequest/tests/session_redirect"]];
  647 + [request start];
  648 + BOOL success = [[request responseString] isEqualToString:@"Take me to your leader"];
  649 + GHAssertTrue(success,@"Failed to redirect preserving session cookies");
  650 +}
  651 +
  652 +- (void)testTooMuchRedirection
  653 +{
  654 + // This url will simply send a 302 redirect back to itself
  655 + ASIHTTPRequest *request = [ASIHTTPRequest requestWithURL:[NSURL URLWithString:@"http://asi/ASIHTTPRequest/tests/one_infinite_loop"]];
  656 + [request start];
  657 + GHAssertNotNil([request error],@"Failed to generate an error when redirection occurs too many times");
  658 + BOOL success = ([[request error] code] == ASITooMuchRedirectionErrorType);
  659 + GHAssertTrue(success,@"Generated the wrong error for a redirection loop");
  660 +}
  661 +
642 @end 662 @end