Skip to content

Commit ba1d03d

Browse files
hamzahrmalikfabianfett
hamzahrmalik
and
fabianfett
authored
Add option to retain request method on 301/302/303 redirects (#887)
Add a configuration option to retain the HTTP method and body receiving 301 or 302 responses. Currently we automatically change the method to GET, and remove the body, before following a 301 or 302. This is compliant with the fetch specification: https://fetch.spec.whatwg.org/#http-redirect-fetch However, it is useful to be able to override this behaviour and retain the method and body. Changes - Add a new struct to encapsulate the (now 4) arguments of the follow case of the redirect mode - Add new options `retainHTTPMethodAndBodyOn301` and `retainHTTPMethodAndBodyOn302`. Defaults to false because thats the existing behaviour today - When it is true, do not convert requests to GET after following a redirect - Note: this does not affect 307/308 (or any other) redirects. They always preserve their method --------- Co-authored-by: Fabian Fett <fabianfett@apple.com>
1 parent 101258d commit ba1d03d

11 files changed

+312
-28
lines changed

‎Sources/AsyncHTTPClient/AsyncAwait/HTTPClient+execute.swift‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,8 @@ extension HTTPClient {
140140
let newRequest = currentRequest.followingRedirect(
141141
from: preparedRequest.url,
142142
to: redirectURL,
143-
status: response.status
143+
status: response.status,
144+
config: redirectState.config
144145
)
145146

146147
guard newRequest.body.canBeConsumedMultipleTimes else {

‎Sources/AsyncHTTPClient/AsyncAwait/HTTPClientRequest+Prepared.swift‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,15 +124,17 @@ extension HTTPClientRequest {
124124
func followingRedirect(
125125
from originalURL: URL,
126126
to redirectURL: URL,
127-
status: HTTPResponseStatus
127+
status: HTTPResponseStatus,
128+
config: HTTPClient.Configuration.RedirectConfiguration.FollowConfiguration
128129
) -> HTTPClientRequest {
129130
let (method, headers, body) = transformRequestForRedirect(
130131
from: originalURL,
131132
method: self.method,
132133
headers: self.headers,
133134
body: self.body,
134135
to: redirectURL,
135-
status: status
136+
status: status,
137+
config: config
136138
)
137139
var newRequest = HTTPClientRequest(url: redirectURL.absoluteString)
138140
newRequest.method = method

‎Sources/AsyncHTTPClient/HTTPClient.swift‎

Lines changed: 58 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1269,13 +1269,54 @@ extension HTTPClient.Configuration {
12691269
/// Redirects are not followed.
12701270
case disallow
12711271
/// Redirects are followed with a specified limit.
1272-
case follow(max: Int, allowCycles: Bool)
1272+
case follow(FollowConfiguration)
1273+
}
1274+
1275+
/// Configuration for following redirects.
1276+
public struct FollowConfiguration: Hashable, Sendable {
1277+
/// The maximum number of allowed redirects.
1278+
public var max: Int
1279+
/// Whether cycles are allowed.
1280+
public var allowCycles: Bool
1281+
/// Whether to retain the HTTP method and body when following a 301 redirect on a POST request.
1282+
/// This should be false as per the fetch spec, but may be true according to RFC 9110.
1283+
/// This does not affect non-POST requests.
1284+
public var retainHTTPMethodAndBodyOn301: Bool
1285+
/// Whether to retain the HTTP method and body when following a 302 redirect on a POST request.
1286+
/// This should be false as per the fetch spec, but may be true according to RFC 9110.
1287+
/// This does not affect non-POST requests.
1288+
public var retainHTTPMethodAndBodyOn302: Bool
1289+
1290+
/// Create a new ``FollowConfiguration``
1291+
/// - Parameters:
1292+
/// - max: The maximum number of allowed redirects.
1293+
/// - allowCycles: Whether cycles are allowed.
1294+
/// - retainHTTPMethodAndBodyOn301: Whether to retain the HTTP method and body when following a 301 redirect on a POST request. This should be false as per the fetch spec, but may be true according to RFC 9110. This does not affect non-POST requests.
1295+
/// - retainHTTPMethodAndBodyOn302: Whether to retain the HTTP method and body when following a 302 redirect on a POST request. This should be false as per the fetch spec, but may be true according to RFC 9110. This does not affect non-POST requests.
1296+
public init(
1297+
max: Int,
1298+
allowCycles: Bool,
1299+
retainHTTPMethodAndBodyOn301: Bool,
1300+
retainHTTPMethodAndBodyOn302: Bool
1301+
) {
1302+
self.max = max
1303+
self.allowCycles = allowCycles
1304+
self.retainHTTPMethodAndBodyOn301 = retainHTTPMethodAndBodyOn301
1305+
self.retainHTTPMethodAndBodyOn302 = retainHTTPMethodAndBodyOn302
1306+
}
12731307
}
12741308

12751309
var mode: Mode
12761310

12771311
init() {
1278-
self.mode = .follow(max: 5, allowCycles: false)
1312+
self.mode = .follow(
1313+
.init(
1314+
max: 5,
1315+
allowCycles: false,
1316+
retainHTTPMethodAndBodyOn301: false,
1317+
retainHTTPMethodAndBodyOn302: false
1318+
)
1319+
)
12791320
}
12801321

12811322
init(configuration: Mode) {
@@ -1293,7 +1334,21 @@ extension HTTPClient.Configuration {
12931334
///
12941335
/// - warning: Cycle detection will keep all visited URLs in memory which means a malicious server could use this as a denial-of-service vector.
12951336
public static func follow(max: Int, allowCycles: Bool) -> RedirectConfiguration {
1296-
.init(configuration: .follow(max: max, allowCycles: allowCycles))
1337+
.follow(
1338+
configuration: .init(
1339+
max: max,
1340+
allowCycles: allowCycles,
1341+
retainHTTPMethodAndBodyOn301: false,
1342+
retainHTTPMethodAndBodyOn302: false
1343+
)
1344+
)
1345+
}
1346+
1347+
/// Redirects are followed.
1348+
///
1349+
/// - Parameter: configuration: Configure how redirects are followed.
1350+
public static func follow(configuration: FollowConfiguration) -> RedirectConfiguration {
1351+
.init(configuration: .follow(configuration))
12971352
}
12981353
}
12991354

‎Sources/AsyncHTTPClient/HTTPClientConfiguration+SwiftConfiguration.swift‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,8 @@ extension HTTPClient.Configuration.RedirectConfiguration {
6666
/// - `mode` (string, optional, default: "follow"): Redirect handling mode ("follow" or "disallow").
6767
/// - `maxRedirects` (int, optional, default: 5): Maximum allowed redirects when mode is "follow".
6868
/// - `allowCycles` (bool, optional, default: false): Allow cyclic redirects when mode is "follow".
69+
/// - `retainHTTPMethodAndBodyOn301` (bool, optional, default: false): Whether to retain the HTTP method and body when following a 301 redirect on a POST request. This should be false as per the fetch spec, but may be true according to RFC 9110. This does not affect non-POST requests.
70+
/// - `retainHTTPMethodAndBodyOn302` (bool, optional, default: false): Whether to retain the HTTP method and body when following a 302 redirect on a POST request. This should be false as per the fetch spec, but may be true according to RFC 9110. This does not affect non-POST requests.
6971
///
7072
/// - Throws: `HTTPClientError.invalidRedirectConfiguration` if mode is specified but invalid.
7173
public init(configReader: ConfigReader) throws {
@@ -77,7 +79,16 @@ extension HTTPClient.Configuration.RedirectConfiguration {
7779
if mode == "follow" {
7880
let maxRedirects = configReader.int(forKey: "maxRedirects", default: 5)
7981
let allowCycles = configReader.bool(forKey: "allowCycles", default: false)
80-
self = .follow(max: maxRedirects, allowCycles: allowCycles)
82+
let retainHTTPMethodAndBodyOn301 = configReader.bool(forKey: "retainHTTPMethodAndBodyOn301", default: false)
83+
let retainHTTPMethodAndBodyOn302 = configReader.bool(forKey: "retainHTTPMethodAndBodyOn302", default: false)
84+
self = .follow(
85+
configuration: .init(
86+
max: maxRedirects,
87+
allowCycles: allowCycles,
88+
retainHTTPMethodAndBodyOn301: retainHTTPMethodAndBodyOn301,
89+
retainHTTPMethodAndBodyOn302: retainHTTPMethodAndBodyOn302
90+
)
91+
)
8192
} else if mode == "disallow" {
8293
self = .disallow
8394
} else {

‎Sources/AsyncHTTPClient/HTTPHandler.swift‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1076,7 +1076,8 @@ internal struct RedirectHandler<ResponseType: Sendable> {
10761076
headers: self.request.headers,
10771077
body: self.request.body,
10781078
to: redirectURL,
1079-
status: status
1079+
status: status,
1080+
config: self.redirectState.config
10801081
)
10811082

10821083
let newRequest = try HTTPClient.Request(

‎Sources/AsyncHTTPClient/RedirectState.swift‎

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,10 @@ import struct Foundation.URL
1919
typealias RedirectMode = HTTPClient.Configuration.RedirectConfiguration.Mode
2020

2121
struct RedirectState {
22-
/// number of redirects we are allowed to follow.
23-
private var limit: Int
22+
var config: HTTPClient.Configuration.RedirectConfiguration.FollowConfiguration
2423

2524
/// All visited URLs.
2625
private var visited: [String]
27-
28-
/// if true, `redirect(to:)` will throw an error if a cycle is detected.
29-
private let allowCycles: Bool
3026
}
3127

3228
extension RedirectState {
@@ -40,8 +36,8 @@ extension RedirectState {
4036
switch configuration {
4137
case .disallow:
4238
return nil
43-
case .follow(let maxRedirects, let allowCycles):
44-
self.init(limit: maxRedirects, visited: [initialURL], allowCycles: allowCycles)
39+
case .follow(let config):
40+
self.init(config: config, visited: [initialURL])
4541
}
4642
}
4743
}
@@ -52,11 +48,11 @@ extension RedirectState {
5248
/// - Parameter redirectURL: the new URL to redirect the request to
5349
/// - Throws: if it reaches the redirect limit or detects a redirect cycle if and `allowCycles` is false
5450
mutating func redirect(to redirectURL: String) throws {
55-
guard self.visited.count <= limit else {
51+
guard self.visited.count <= config.max else {
5652
throw HTTPClientError.redirectLimitReached
5753
}
5854

59-
guard allowCycles || !self.visited.contains(redirectURL) else {
55+
guard config.allowCycles || !self.visited.contains(redirectURL) else {
6056
throw HTTPClientError.redirectCycleDetected
6157
}
6258
self.visited.append(redirectURL)
@@ -111,13 +107,16 @@ func transformRequestForRedirect<Body>(
111107
headers requestHeaders: HTTPHeaders,
112108
body requestBody: Body?,
113109
to redirectURL: URL,
114-
status responseStatus: HTTPResponseStatus
110+
status responseStatus: HTTPResponseStatus,
111+
config: HTTPClient.Configuration.RedirectConfiguration.FollowConfiguration
115112
) -> (HTTPMethod, HTTPHeaders, Body?) {
116113
let convertToGet: Bool
117114
if responseStatus == .seeOther, requestMethod != .HEAD {
118115
convertToGet = true
119-
} else if responseStatus == .movedPermanently || responseStatus == .found, requestMethod == .POST {
120-
convertToGet = true
116+
} else if responseStatus == .movedPermanently, requestMethod == .POST {
117+
convertToGet = !config.retainHTTPMethodAndBodyOn301
118+
} else if responseStatus == .found, requestMethod == .POST {
119+
convertToGet = !config.retainHTTPMethodAndBodyOn302
121120
} else {
122121
convertToGet = false
123122
}

‎Tests/AsyncHTTPClientTests/AsyncAwaitEndToEndTests.swift‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1019,6 +1019,95 @@ final class AsyncAwaitEndToEndTests: XCTestCase {
10191019
}
10201020
}
10211021
}
1022+
1023+
// MARK: - POST to GET conversion on redirects
1024+
1025+
@available(macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *)
1026+
private func _testPostConvertedToGetOnRedirect(
1027+
statusPath: String,
1028+
expectedStatus: HTTPResponseStatus
1029+
) {
1030+
XCTAsyncTest {
1031+
let bin = HTTPBin(.http2(compress: false))
1032+
defer { XCTAssertNoThrow(try bin.shutdown()) }
1033+
let client = makeDefaultHTTPClient()
1034+
defer { XCTAssertNoThrow(try client.syncShutdown()) }
1035+
let logger = Logger(label: "HTTPClient", factory: StreamLogHandler.standardOutput(label:))
1036+
1037+
var request = HTTPClientRequest(url: "https://localhost:\(bin.port)\(statusPath)")
1038+
request.method = .POST
1039+
request.body = .bytes(ByteBuffer(string: "test body"))
1040+
1041+
guard
1042+
let response = await XCTAssertNoThrowWithResult(
1043+
try await client.execute(request, deadline: .now() + .seconds(10), logger: logger)
1044+
)
1045+
else { return }
1046+
1047+
XCTAssertEqual(response.status, .ok)
1048+
XCTAssertEqual(response.history.count, 2, "Expected 2 entries in history for \(statusPath)")
1049+
XCTAssertEqual(
1050+
response.history[0].request.method,
1051+
.POST,
1052+
"Original request should be POST for \(statusPath)"
1053+
)
1054+
XCTAssertEqual(
1055+
response.history[1].request.method,
1056+
.GET,
1057+
"Redirected request should be converted to GET for \(statusPath)"
1058+
)
1059+
XCTAssertEqual(
1060+
response.history[0].responseHead.status,
1061+
expectedStatus,
1062+
"Expected \(expectedStatus) for \(statusPath)"
1063+
)
1064+
}
1065+
}
1066+
1067+
func testPostConvertedToGetOn301Redirect() {
1068+
self._testPostConvertedToGetOnRedirect(
1069+
statusPath: "/redirect/301",
1070+
expectedStatus: .movedPermanently
1071+
)
1072+
}
1073+
1074+
func testPostConvertedToGetOn302Redirect() {
1075+
self._testPostConvertedToGetOnRedirect(
1076+
statusPath: "/redirect/302",
1077+
expectedStatus: .found
1078+
)
1079+
}
1080+
1081+
func testPostConvertedToGetOn303Redirect() {
1082+
self._testPostConvertedToGetOnRedirect(
1083+
statusPath: "/redirect/303",
1084+
expectedStatus: .seeOther
1085+
)
1086+
}
1087+
1088+
func testGetMethodUnchangedOnRedirect() {
1089+
XCTAsyncTest {
1090+
let bin = HTTPBin(.http2(compress: false))
1091+
defer { XCTAssertNoThrow(try bin.shutdown()) }
1092+
let client = makeDefaultHTTPClient()
1093+
defer { XCTAssertNoThrow(try client.syncShutdown()) }
1094+
let logger = Logger(label: "HTTPClient", factory: StreamLogHandler.standardOutput(label:))
1095+
1096+
// Test that non-POST methods remain unchanged
1097+
let requestGet = HTTPClientRequest(url: "https://localhost:\(bin.port)/redirect/302")
1098+
1099+
guard
1100+
let responseGet = await XCTAssertNoThrowWithResult(
1101+
try await client.execute(requestGet, deadline: .now() + .seconds(10), logger: logger)
1102+
)
1103+
else { return }
1104+
1105+
XCTAssertEqual(responseGet.status, .ok)
1106+
XCTAssertEqual(responseGet.history.count, 2)
1107+
XCTAssertEqual(responseGet.history[0].request.method, .GET, "Original request should be GET")
1108+
XCTAssertEqual(responseGet.history[1].request.method, .GET, "Redirected request should remain GET")
1109+
}
1110+
}
10221111
}
10231112

10241113
struct AnySendableSequence<Element>: @unchecked Sendable {

‎Tests/AsyncHTTPClientTests/HTTPClientTestUtils.swift‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -984,11 +984,21 @@ internal final class HTTPBinHandler: ChannelInboundHandler {
984984
}
985985
self.resps.append(HTTPResponseBuilder(status: .ok, responseBodyIsRequestBodyByteCount: true))
986986
return
987+
case "/redirect/301":
988+
var headers = self.responseHeaders
989+
headers.add(name: "location", value: "/ok")
990+
self.resps.append(HTTPResponseBuilder(status: .movedPermanently, headers: headers))
991+
return
987992
case "/redirect/302":
988993
var headers = self.responseHeaders
989994
headers.add(name: "location", value: "/ok")
990995
self.resps.append(HTTPResponseBuilder(status: .found, headers: headers))
991996
return
997+
case "/redirect/303":
998+
var headers = self.responseHeaders
999+
headers.add(name: "location", value: "/ok")
1000+
self.resps.append(HTTPResponseBuilder(status: .seeOther, headers: headers))
1001+
return
9921002
case "/redirect/https":
9931003
let port = self.value(for: "port", from: urlComponents.query!)
9941004
var headers = self.responseHeaders

0 commit comments

Comments
 (0)