Add option to retain request method on 301/302/303 redirects#887
Add option to retain request method on 301/302/303 redirects#887hamzahrmalik wants to merge 4 commits intoswift-server:mainfrom
Conversation
| /// - Parameters: | ||
| /// - max: The maximum number of allowed redirects. | ||
| /// - allowCycles: Whether cycles are allowed. | ||
| /// - convertToGet: Whether to convert POST requests into GET requests when following a 301, 302 or 303 redirect. This does not apply to 307 or 308. Those always retain the original method. |
There was a problem hiding this comment.
I think we should call out that this should be true except, if users have clear needs.
| /// Redirects are followed with a specified limit. | ||
| /// | ||
| /// - parameters | ||
| /// | ||
| /// - warning: Cycle detection will keep all visited URLs in memory which means a malicious server could use this as a denial-of-service vector. |
There was a problem hiding this comment.
i copied it from the existing one above
fabianfett
left a comment
fabianfett
left a comment
There was a problem hiding this comment.
Love the tests. Let the naming bike-shed begin.
| /// Whether to convert POST requests into GET requests when following a 301, 302 or 303 redirect. | ||
| /// This does not apply to 307 or 308. Those always retain the original method. | ||
| public var convertToGet: Bool |
There was a problem hiding this comment.
I like that curls specifically calls out 301, 302 and 303. I wonder if we should do the same here? convertToGet sounds too broad for me.
| } | ||
|
|
||
| /// Configuration for following redirects. | ||
| public struct FollowConfiguration: Hashable, Sendable { |
There was a problem hiding this comment.
I wonder if it is worth adding this new struct now? If I see this correctly, we don't pass it around at all. Lets just add another property on the enum for now.
There was a problem hiding this comment.
given that we're doing 3 properties now, imo we should keep the struct. We can actually take advantage of it by holding the struct inside the RedirectState
There was a problem hiding this comment.
see last commit
2 participants
Add a configuration option to not use GET when receiving 301/302/303 a redirect on a POST request.
Changes