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.

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

Added headers not sent when following redirects #798

Closed
eburi opened this issue Sep 21, 2015 · 11 comments
Closed

Added headers not sent when following redirects #798

eburi opened this issue Sep 21, 2015 · 11 comments

Comments

@eburi
Copy link

@eburi eburi commented Sep 21, 2015

Xcode 7, iOS 9, Alamofire 2.0.1

When following a redirect, the headers added to the original request, will not be added to the new request.

Example (#788):

let headers = [
            "Authorization": userData.userAPIKey!,
        ]
Alamofire.request(.GET, url, parameters: nil, encoding: .URL, headers: headers).response { (request, response, data, error) -> Void in
}

If the server responds with a 301 Moved permanently, a new request is made to the location returned. But the new request will not have the Authorization-Header set.

I saw #314, #317, and #424 But I think this is a different issue. IMHO Alamofire should either follow redirects and add the headers added to the original-request or it should not follow redirects at all and report them to the user. In #788 I found out the hard way looking at what was sent over the wire.

As a user of Alamofire, I don't get any notification/log-message, when the library is following a redirect. This is very handy, but I would expect, that the my extra-headers would also be applied to the new-request.

@cnoon
Copy link
Member

@cnoon cnoon commented Sep 22, 2015

Thanks for filing this issue @eburi. I'll need to put some tests together to figure out exactly why the headers are being dropped. Are you overriding any of the session delegate closures?

@cnoon
Copy link
Member

@cnoon cnoon commented Sep 22, 2015

Okay @eburi, you're going to have to take this one up with Apple.

I've added a fairly robust test in cdeb5c1 that demonstrates the behavior. It appears that most headers are passed through from the original to the redirect request with the exception of the Authorization header. I can't find any documentation on this behavior, but the test verifies what is actually going on. If you need to pass the Authorization header through, you'll have to do it manually by overriding the closure.

func testThatRedirectedRequestContainsAllHeadersFromOriginalRequest() {
    // Given
    let redirectURLString = "https://httpbin.org/get"
    let URLString = "https://httpbin.org/redirect-to?url=\(redirectURLString)"
    let headers = [
        "Authorization": "1234",
        "Custom-Header": "foobar",
    ]

    // NOTE: It appears that most headers are maintained during a redirect with the exception of the `Authorization`
    // header. It appears that Apple's strips the `Authorization` header from the redirected URL request. If you
    // need to maintain the `Authorization` header, you need to manually append it to the redirected request.

    Alamofire.Manager.sharedInstance.delegate.taskWillPerformHTTPRedirection = { session, task, response, request in
        var redirectedRequest = request

        if let
            originalRequest = task.originalRequest,
            headers = originalRequest.allHTTPHeaderFields,
            authorizationHeaderValue = headers["Authorization"]
        {
            let mutableRequest = request.mutableCopy() as! NSMutableURLRequest
            mutableRequest.setValue(authorizationHeaderValue, forHTTPHeaderField: "Authorization")
            redirectedRequest = mutableRequest
        }

        return redirectedRequest
    }

    let expectation = expectationWithDescription("Request should redirect to \(redirectURLString)")

    var response: Response<AnyObject, NSError>?

    // When
    Alamofire.request(.GET, URLString, headers: headers)
        .responseJSON { closureResponse in
            response = closureResponse
            expectation.fulfill()
        }

    waitForExpectationsWithTimeout(defaultTimeout, handler: nil)

    // Then
    XCTAssertNotNil(response?.request, "request should not be nil")
    XCTAssertNotNil(response?.response, "response should not be nil")
    XCTAssertNotNil(response?.data, "data should not be nil")
    XCTAssertTrue(response?.result.isSuccess ?? false, "response result should be a success")

    if let
        JSON = response?.result.value as? [String: AnyObject],
        headers = JSON["headers"] as? [String: String]
    {
        XCTAssertEqual(headers["Custom-Header"], "foobar", "Custom-Header should be equal to foobar")
        XCTAssertEqual(headers["Authorization"], "1234", "Authorization header should be equal to 1234")
    }
}

All you need to do is override the taskWillPerformHTTPRedirection closure on the delegate as shown in the test. If you feel strongly about this behavior being incorrect, then I'd suggest you file a radar with Apple.

Cheers. 🍻

@eburi
Copy link
Author

@eburi eburi commented Sep 22, 2015

Thanks, for the quick reply!
I won't take this to Apple - this is most likely by design for security reasons.

And now we know how to work around it!

@AnthonyMDev
Copy link
Contributor

@AnthonyMDev AnthonyMDev commented Sep 24, 2015

@cnoon I may be missing something, but why doesn't Alamofire just implement this in the taskWillPerformHTTPRedirection internally?

Is it because, since Apple is doing it this way, we are assuming it's a security issue?

If you are willing to accept a PR that implements this, I'd be happy to make one. I think it shouldn't be too difficult. However, I also understand if you don't want to do this, as it may be seen as a way to work around Apple's own security measures.

@cnoon
Copy link
Member

@cnoon cnoon commented Sep 24, 2015

It's most likely not implemented that way for security purposes, but more for usability. Forwarding on the Authorization headers can fail the redirected request on certain servers. Some servers can throw 400s if they are sent Authorization headers when they are not expecting it. Therefore, it's good to follow Apple's lead here.

If you do need to append the headers to the redirected request, you'll need to do so manually as shown in the test logic above.

@TigerWolf
Copy link

@TigerWolf TigerWolf commented May 3, 2016

This work around does not work if you are using
request.authenticate(user: username, password: password)

Is there a way for me to avoid manually building the Authorization header?

@cnoon

@ssilvestri15
Copy link

@ssilvestri15 ssilvestri15 commented Jun 9, 2020

Okay @eburi, you're going to have to take this one up with Apple.

I've added a fairly robust test in cdeb5c1 that demonstrates the behavior. It appears that most headers are passed through from the original to the redirect request with the exception of the Authorization header. I can't find any documentation on this behavior, but the test verifies what is actually going on. If you need to pass the Authorization header through, you'll have to do it manually by overriding the closure.

func testThatRedirectedRequestContainsAllHeadersFromOriginalRequest() {
    // Given
    let redirectURLString = "https://httpbin.org/get"
    let URLString = "https://httpbin.org/redirect-to?url=\(redirectURLString)"
    let headers = [
        "Authorization": "1234",
        "Custom-Header": "foobar",
    ]

    // NOTE: It appears that most headers are maintained during a redirect with the exception of the `Authorization`
    // header. It appears that Apple's strips the `Authorization` header from the redirected URL request. If you
    // need to maintain the `Authorization` header, you need to manually append it to the redirected request.

    Alamofire.Manager.sharedInstance.delegate.taskWillPerformHTTPRedirection = { session, task, response, request in
        var redirectedRequest = request

        if let
            originalRequest = task.originalRequest,
            headers = originalRequest.allHTTPHeaderFields,
            authorizationHeaderValue = headers["Authorization"]
        {
            let mutableRequest = request.mutableCopy() as! NSMutableURLRequest
            mutableRequest.setValue(authorizationHeaderValue, forHTTPHeaderField: "Authorization")
            redirectedRequest = mutableRequest
        }

        return redirectedRequest
    }

    let expectation = expectationWithDescription("Request should redirect to \(redirectURLString)")

    var response: Response<AnyObject, NSError>?

    // When
    Alamofire.request(.GET, URLString, headers: headers)
        .responseJSON { closureResponse in
            response = closureResponse
            expectation.fulfill()
        }

    waitForExpectationsWithTimeout(defaultTimeout, handler: nil)

    // Then
    XCTAssertNotNil(response?.request, "request should not be nil")
    XCTAssertNotNil(response?.response, "response should not be nil")
    XCTAssertNotNil(response?.data, "data should not be nil")
    XCTAssertTrue(response?.result.isSuccess ?? false, "response result should be a success")

    if let
        JSON = response?.result.value as? [String: AnyObject],
        headers = JSON["headers"] as? [String: String]
    {
        XCTAssertEqual(headers["Custom-Header"], "foobar", "Custom-Header should be equal to foobar")
        XCTAssertEqual(headers["Authorization"], "1234", "Authorization header should be equal to 1234")
    }
}

All you need to do is override the taskWillPerformHTTPRedirection closure on the delegate as shown in the test. If you feel strongly about this behavior being incorrect, then I'd suggest you file a radar with Apple.

Cheers. 🍻

How replicate this with Alamofire 5?

@jshier
Copy link
Contributor

@jshier jshier commented Jun 9, 2020

You can control redirects through the RedirectHandler API and add one to a Session or a Request.

@larrylegend
Copy link

@larrylegend larrylegend commented Jul 1, 2021

Thanks @jshier for the helpful suggestion. If anyone wants a head start on the implementation, here's a quick Alamofire 5 example I put together once I got it working:

        let authorizationHeaderPreservingRedirectHandler: Redirector = {
            let behavior = Redirector.Behavior.modify {
                (task, request, response) in
                
                var redirectedRequest = request

                if let originalRequest = task.originalRequest,
                   let headers = originalRequest.allHTTPHeaderFields,
                   let authorizationHeaderValue = headers["Authorization"] {
                    redirectedRequest.setValue(authorizationHeaderValue, forHTTPHeaderField: "Authorization")
                }

                return redirectedRequest
            }

            let redirector = Redirector(behavior: behavior)
            
            return redirector
        }()

        // --- Example request
        let redirectURLString = "https://httpbingo.org/get"
        let urlString = "https://httpbingo.org/redirect-to?url=\(redirectURLString)"
        let accessToken = "1234example"
        let headers = HTTPHeaders([HTTPHeader.authorization(bearerToken: accessToken)])
        // ---
        
        AF.request(urlString, method: .get, headers: headers)
            .redirect(using: authorizationHeaderPreservingRedirectHandler)
            .response {
                response in
                debugPrint(response)
        }

@aybarsyalcin
Copy link

@aybarsyalcin aybarsyalcin commented Sep 2, 2021

@larrylegend your code is solving my problems. thanks again

@MarcSteven
Copy link

@MarcSteven MarcSteven commented Sep 6, 2021

Wow it's so long

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants