Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drop special headers from redirected requests #289

Merged
merged 5 commits into from
Mar 9, 2019
Merged

Drop special headers from redirected requests #289

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
119c7ff
Drop special headers from redirected requests
sberrevoets Sep 15, 2018
2fecbda
Update CHANGELOG.md
jeffctown Mar 8, 2019
ed13794
Put clearing of auth headers in it’s own method and using a for loop.
jeffctown Mar 8, 2019
4d0f05b
Re-introduce header retention policy tests for 3xx
sberrevoets Mar 8, 2019
29ff8d8
fixup! Re-introduce header retention policy tests for 3xx
sberrevoets Mar 9, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,15 @@
## [Future release]
* Enabled application extension API only.
[@lightsprint09](https://github.com/lightsprint09)
* Disabled a flaky redirect test and adding the known issue with redirects to the README.
* Disabled a flaky redirect test and adding the known issue with redirects to the README.
[@jeffctown](https://github.com/jeffctown)
[#301](https://github.com/AliSoftware/OHHTTPStubs/pull/301)
* Added `isMethodHEAD()` to the `Swift` helpers.
[@Simon-Kaz](https://github.com/Simon-Kaz)
[#294](https://github.com/AliSoftware/OHHTTPStubs/pull/294)
* Fixed issue with not preserving correct headers when following 3xx
redirects.
[@sberrevoets](https://github.com/sberrevoets)

## [6.1.0](https://github.com/AliSoftware/OHHTTPStubs/releases/tag/6.1.0)

Expand Down
29 changes: 25 additions & 4 deletions OHHTTPStubs/Sources/OHHTTPStubs.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -360,12 +360,30 @@ - (id)initWithRequest:(NSURLRequest *)request cachedResponse:(NSCachedURLRespons

+ (NSURLRequest *)canonicalRequestForRequest:(NSURLRequest *)request
{
return request;
return request;
}

- (NSCachedURLResponse *)cachedResponse
{
return nil;
return nil;
}

/** Drop certain headers in accordance with
* https://developer.apple.com/documentation/foundation/urlsessionconfiguration/1411532-httpadditionalheaders
*/
- (NSMutableURLRequest *)clearAuthHeadersForRequest:(NSMutableURLRequest *)request {
NSArray* authHeadersToRemove = @[
@"Authorization",
@"Connection",
@"Host",
@"Proxy-Authenticate",
@"Proxy-Authorization",
@"WWW-Authenticate"
];
for (NSString* header in authHeadersToRemove) {
[request setValue:nil forHTTPHeaderField:header];
}
return request;
}

- (void)startLoading
Expand Down Expand Up @@ -442,13 +460,16 @@ - (void)startLoading
case 301:
case 302:
case 307:
case 308:
case 308: {
//Preserve the original request method and body, and set the new location URL
mReq = [self.request mutableCopy];
[mReq setURL:redirectLocationURL];

mReq = [self clearAuthHeadersForRequest:mReq];

redirectRequest = (NSURLRequest*)[mReq copy];
break;

}
default:
redirectRequest = [NSURLRequest requestWithURL:redirectLocationURL];
break;
Expand Down
81 changes: 65 additions & 16 deletions OHHTTPStubs/UnitTests/Test Suites/NSURLSessionTests.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,17 +105,19 @@ - (void)_test_NSURLSession:(NSURLSession*)session

- (void)_test_redirect_NSURLSession:(NSURLSession*)session
httpMethod:(NSString *)requestHTTPMethod
headers:(NSDictionary *)headers
jsonBody:(NSDictionary*)json
delays:(NSTimeInterval)delay
redirectStatusCode:(int)redirectStatusCode
completion:(void(^)(NSString* redirectedRequestMethod, id redirectedRequestJSONBody, NSHTTPURLResponse *redirectHTTPResponse, id finalJSONResponse, NSError *errorResponse))completion
completion:(void(^)(NSString* redirectedRequestMethod, NSDictionary * redirectedRequestHeaders, id redirectedRequestJSONBody, NSHTTPURLResponse *redirectHTTPResponse, id finalJSONResponse, NSError *errorResponse))completion
{
if ([NSURLSession class])
{
const NSTimeInterval requestTime = delay;
const NSTimeInterval responseTime = delay;

__block __strong NSString* capturedRedirectedRequestMethod = nil;
__block __strong NSDictionary* capturedRedirectedRequestHeaders = nil;
__block __strong id capturedRedirectedRequestJSONBody = nil;
__block __strong NSHTTPURLResponse* capturedRedirectHTTPResponse = nil;
__block __strong id capturedResponseJSONBody = nil;
Expand All @@ -139,6 +141,7 @@ - (void)_test_redirect_NSURLSession:(NSURLSession*)session
return [[[request URL] path] isEqualToString:@"/newlocation"];
} withStubResponse:^OHHTTPStubsResponse *(NSURLRequest *redirectedRequest) {
capturedRedirectedRequestMethod = redirectedRequest.HTTPMethod;
capturedRedirectedRequestHeaders = redirectedRequest.allHTTPHeaderFields;
if (redirectedRequest.OHHTTPStubs_HTTPBody) {
capturedRedirectedRequestJSONBody = [NSJSONSerialization JSONObjectWithData:redirectedRequest.OHHTTPStubs_HTTPBody options:0 error:NULL];
} else {
Expand All @@ -155,6 +158,7 @@ - (void)_test_redirect_NSURLSession:(NSURLSession*)session
// Building the initial request.
NSMutableURLRequest *request = [NSMutableURLRequest requestWithURL:[NSURL URLWithString:@"foo://unknownhost:666/oldlocation"]];
request.HTTPMethod = requestHTTPMethod;
request.allHTTPHeaderFields = headers;
if (requestBody)
{
request.HTTPBody = requestBody;
Expand Down Expand Up @@ -193,7 +197,7 @@ - (void)_test_redirect_NSURLSession:(NSURLSession*)session
[task resume];

[self waitForExpectationsWithTimeout:(requestTime+responseTime)*2+0.1 handler:nil];
completion(capturedRedirectedRequestMethod, capturedRedirectedRequestJSONBody,
completion(capturedRedirectedRequestMethod, capturedRedirectedRequestHeaders, capturedRedirectedRequestJSONBody,
capturedRedirectHTTPResponse,
capturedResponseJSONBody, capturedResponseError);
}
Expand All @@ -214,8 +218,8 @@ - (void)test_SharedNSURLSession
XCTAssertEqualObjects(jsonResponse, json, @"Unexpected data received");
}];

[self _test_redirect_NSURLSession:session httpMethod:@"GET" jsonBody:nil delays:0.1 redirectStatusCode:301
completion:^(NSString *redirectedRequestMethod, id redirectedRequestJSONBody, NSHTTPURLResponse *redirectHTTPResponse, id finalJSONResponse, NSError *errorResponse)
[self _test_redirect_NSURLSession:session httpMethod:@"GET" headers:nil jsonBody:nil delays:0.1 redirectStatusCode:301
completion:^(NSString *redirectedRequestMethod, NSDictionary *redirectedRequestHeaders, id redirectedRequestJSONBody, NSHTTPURLResponse *redirectHTTPResponse, id finalJSONResponse, NSError *errorResponse)
{
XCTAssertEqualObjects(redirectedRequestMethod, @"GET", @"Expected redirected request to use GET method");
XCTAssertNil(redirectedRequestJSONBody, @"Expected redirected request to have empty body");
Expand Down Expand Up @@ -245,8 +249,8 @@ - (void)test_NSURLSessionDefaultConfig
XCTAssertEqualObjects(jsonResponse, json, @"Unexpected data received");
}];

[self _test_redirect_NSURLSession:session httpMethod:@"GET" jsonBody:nil delays:0.1 redirectStatusCode:301
completion:^(NSString *redirectedRequestMethod, id redirectedRequestJSONBody, NSHTTPURLResponse *redirectHTTPResponse, id finalJSONResponse, NSError *errorResponse)
[self _test_redirect_NSURLSession:session httpMethod:@"GET" headers:nil jsonBody:nil delays:0.1 redirectStatusCode:301
completion:^(NSString *redirectedRequestMethod, NSDictionary *redirectedRequestHeaders, id redirectedRequestJSONBody, NSHTTPURLResponse *redirectHTTPResponse, id finalJSONResponse, NSError *errorResponse)
{
XCTAssertEqualObjects(redirectedRequestMethod, @"GET", @"Expected redirected request to use GET method");
XCTAssertNil(redirectedRequestJSONBody, @"Expected redirected request to have empty body");
Expand All @@ -272,8 +276,8 @@ - (void)test_NSURLSessionDefaultConfig_notFollowingRedirects
NSURLSessionConfiguration* config = [NSURLSessionConfiguration defaultSessionConfiguration];
NSURLSession *session = [NSURLSession sessionWithConfiguration:config delegate:delegate delegateQueue:nil];

[self _test_redirect_NSURLSession:session httpMethod:@"GET" jsonBody:nil delays:0.1 redirectStatusCode:301
completion:^(NSString *redirectedRequestMethod, id redirectedRequestJSONBody, NSHTTPURLResponse *redirectHTTPResponse, id finalJSONResponse, NSError *errorResponse)
[self _test_redirect_NSURLSession:session httpMethod:@"GET" headers:nil jsonBody:nil delays:0.1 redirectStatusCode:301
completion:^(NSString *redirectedRequestMethod, NSDictionary *redirectedRequestHeaders, id redirectedRequestJSONBody, NSHTTPURLResponse *redirectHTTPResponse, id finalJSONResponse, NSError *errorResponse)
{
XCTAssertNil(redirectedRequestMethod, @"Expected no redirected request to fire");
XCTAssertNil(redirectedRequestJSONBody, @"Expected no redirected request to fire");
Expand Down Expand Up @@ -313,8 +317,8 @@ - (void)test_NSURLSessionDefaultConfig_MethodAndDataRetentionOnRedirect
NSURLSessionTestDelegate* delegate = [NSURLSessionTestDelegate delegateFollowingRedirects:YES fulfillOnCompletion:nil];
NSURLSession *session = [NSURLSession sessionWithConfiguration:config delegate:delegate delegateQueue:nil];

[self _test_redirect_NSURLSession:session httpMethod:method jsonBody:json delays:0.0 redirectStatusCode:statusCode
completion:^(NSString *redirectedRequestMethod, id redirectedRequestJSONBody, NSHTTPURLResponse *redirectHTTPResponse, id finalJSONResponse, NSError *errorResponse)
[self _test_redirect_NSURLSession:session httpMethod:method headers:nil jsonBody:json delays:0.0 redirectStatusCode:statusCode
completion:^(NSString *redirectedRequestMethod, NSDictionary *redirectedRequestHeaders, id redirectedRequestJSONBody, NSHTTPURLResponse *redirectHTTPResponse, id finalJSONResponse, NSError *errorResponse)
{
XCTAssertEqualObjects(redirectedRequestMethod, method,
@"Expected the HTTP method to be unchanged after %d redirect", statusCode);
Expand All @@ -338,8 +342,8 @@ - (void)test_NSURLSessionDefaultConfig_MethodAndDataRetentionOnRedirect
NSURLSessionTestDelegate* delegate = [NSURLSessionTestDelegate delegateFollowingRedirects:YES fulfillOnCompletion:nil];
NSURLSession *session = [NSURLSession sessionWithConfiguration:config delegate:delegate delegateQueue:nil];

[self _test_redirect_NSURLSession:session httpMethod:method jsonBody:json delays:0.0 redirectStatusCode:303
completion:^(NSString *redirectedRequestMethod, id redirectedRequestJSONBody, NSHTTPURLResponse *redirectHTTPResponse, id finalJSONResponse, NSError *errorResponse)
[self _test_redirect_NSURLSession:session httpMethod:method headers:nil jsonBody:json delays:0.0 redirectStatusCode:303
completion:^(NSString *redirectedRequestMethod, NSDictionary *redirectedRequestHeaders, id redirectedRequestJSONBody, NSHTTPURLResponse *redirectHTTPResponse, id finalJSONResponse, NSError *errorResponse)
{
XCTAssertEqualObjects(redirectedRequestMethod, @"GET", @"Expected 303 redirected request HTTP method to be reset to GET");
XCTAssertNil(redirectedRequestJSONBody, @"Expected 303-redirected request to have empty body");
Expand All @@ -356,6 +360,51 @@ - (void)test_NSURLSessionDefaultConfig_MethodAndDataRetentionOnRedirect
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_HeaderRetentionPolicyOnRedirect {
if ([NSURLSessionConfiguration class] && [NSURLSession class])
{
NSArray<NSString*>* allMethods = @[@"GET", @"HEAD", @"POST", @"PATCH", @"PUT"];

/** 301, 302, 307, 308: GET, HEAD, POST, PATCH, PUT should all maintain most HTTP headers unchanged **/
for (NSNumber* redirectStatusCode in @[@301, @302, @307, @308]) {
int statusCode = redirectStatusCode.intValue;
for (NSString* method in allMethods) {

NSURLSessionConfiguration* config = [NSURLSessionConfiguration defaultSessionConfiguration];
NSURLSessionTestDelegate* delegate = [NSURLSessionTestDelegate delegateFollowingRedirects:YES fulfillOnCompletion:nil];
NSURLSession *session = [NSURLSession sessionWithConfiguration:config delegate:delegate delegateQueue:nil];

NSDictionary *headers = @{
@"Authorization": @"authorization",
@"Connection": @"connection",
@"Host": @"host",
@"Proxy-Authenticate": @"proxy-authenticate",
@"Proxy-Authorization": @"proxy-authorization",
@"WWW-Authenticate": @"www-authenticate",
@"Preserved": @"preserved"
Copy link
Owner

@AliSoftware AliSoftware Mar 8, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just because if, say, we change the implementation one day and accidentally make the new implementation behave ( wrongly) as "remove all headers but the last" or "only keep one header" (instead of doing what it's supposed to do like today), I would try to put one or two more "headers to be preserved" in there and intertwine them in the middle of that list instead of all in the end 😉 #paranoiaModeOn

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But since it's a dictionary order isn't guaranteed anyway? Is that different under the NSURLSession hood?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh good point!

Though I suspect that dictionary literals, even though they don't guarantee order by contract as it's not a expected requirement for dicts, might end up always being iterated in the same deterministic order in practice (because of current implantation details of dictionary literals in ObjC)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it doesn't hurt to add - done. (In a separate commit for reviewing purposes, happy to rebase at the end if you'd like.)

};
[self _test_redirect_NSURLSession:session httpMethod:method headers:headers jsonBody:nil delays:0.0 redirectStatusCode:statusCode
completion:^(NSString *redirectedRequestMethod, NSDictionary *redirectedRequestHeaders, id redirectedRequestJSONBody, NSHTTPURLResponse *redirectHTTPResponse, id finalJSONResponse, NSError *errorResponse)
{
XCTAssertNil(redirectedRequestHeaders[@"Authorization"], @"Authorization header is preserved when following redirects");
XCTAssertNil(redirectedRequestHeaders[@"Connection"], @"Connection header is preserved when following redirects");
XCTAssertNil(redirectedRequestHeaders[@"Host"], @"Host header is preserved when following redirects");
XCTAssertNil(redirectedRequestHeaders[@"Proxy-Authenticate"], @"Proxy-Authenticate header is preserved when following redirects");
XCTAssertNil(redirectedRequestHeaders[@"Proxy-Authorization"], @"Proxy-Authorization header is preserved when following redirects");
XCTAssertNil(redirectedRequestHeaders[@"WWW-Authenticate"], @"WWW-Authenticate header is preserved when following redirects");
XCTAssertEqual(redirectedRequestHeaders[@"Preserved"], @"preserved", @"Regular header is not preserved when following redirects");
}];

[session finishTasksAndInvalidate];
}
}
}
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");
}
}
#endif

- (void)test_NSURLSessionEphemeralConfig
Expand All @@ -371,8 +420,8 @@ - (void)test_NSURLSessionEphemeralConfig
XCTAssertEqualObjects(jsonResponse, json, @"Unexpected data received");
}];

[self _test_redirect_NSURLSession:session httpMethod:@"GET" jsonBody:json delays:0.1 redirectStatusCode:301
completion:^(NSString *redirectedRequestMethod, id redirectedRequestJSONBody, NSHTTPURLResponse *redirectHTTPResponse, id finalJSONResponse, NSError *errorResponse)
[self _test_redirect_NSURLSession:session httpMethod:@"GET" headers:nil jsonBody:json delays:0.1 redirectStatusCode:301
completion:^(NSString *redirectedRequestMethod, NSDictionary *redirectedRequestHeaders, id redirectedRequestJSONBody, NSHTTPURLResponse *redirectHTTPResponse, id finalJSONResponse, NSError *errorResponse)
{
XCTAssertEqualObjects(redirectedRequestMethod, @"GET", @"Expected the HTTP method of redirected request to be GET");
XCTAssertEqualObjects(redirectedRequestJSONBody, json, @"Expected redirected request to have the same body as the original request");
Expand Down Expand Up @@ -404,8 +453,8 @@ - (void)test_NSURLSessionDefaultConfig_Disabled
XCTAssertNil(jsonResponse, @"Data should not have been received as stubs should be disabled");
}];

[self _test_redirect_NSURLSession:session httpMethod:@"GET" jsonBody:json delays:0.1 redirectStatusCode:301
completion:^(NSString *redirectedRequestMethod, id redirectedRequestJSONBody, NSHTTPURLResponse *finalHTTPResponse, id finalJSONResponse, NSError *errorResponse)
[self _test_redirect_NSURLSession:session httpMethod:@"GET" headers:nil jsonBody:json delays:0.1 redirectStatusCode:301
completion:^(NSString *redirectedRequestMethod, NSDictionary *redirectedRequestHeaders, id redirectedRequestJSONBody, NSHTTPURLResponse *finalHTTPResponse, id finalJSONResponse, NSError *errorResponse)
{
// Stubs were disabled for this session, so we should get an error instead of the stubs data
XCTAssertNotNil(errorResponse, @"Expected error but none found");
Expand Down