Skip to content

[swift6] Fail requests instead of crashing, and don't send a body that failed to encode #25189

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

Open
spigo wants to merge 5 commits into
base: master
Choose a base branch
from
+664 −263
Open

[swift6] Fail requests instead of crashing, and don't send a body that failed to encode #25189

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b01271b
[swift6] Fail requests instead of crashing, and don't send a body tha…
spigo Oct 8, 2026
1f5c654
[swift6][alamofire] Check the response before writing a downloaded file
spigo Oct 8, 2026
8e1892c
[swift6][alamofire] Check the download response before its data
spigo Oct 8, 2026
e807d49
Merge remote-tracking branch 'origin/master' into swift6-fix-crashes-…
spigo Oct 9, 2026
59126b6
[swift6][alamofire] Fail on unsupported multipart values while buildi…
spigo Oct 9, 2026
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
4 changes: 3 additions & 1 deletion modules/openapi-generator/src/main/resources/swift6/JSONEncodingHelper.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import Foundation

{{#nonPublicApi}}internal{{/nonPublicApi}}{{^nonPublicApi}}open{{/nonPublicApi}} class JSONEncodingHelper {

static let encodingErrorParameterKey = "{{projectName}}.RequestBuilderError"

{{#nonPublicApi}}internal{{/nonPublicApi}}{{^nonPublicApi}}open{{/nonPublicApi}} class func encodingParameters<T: Encodable>(forEncodableObject encodableObj: T?, codableHelper: CodableHelper) -> [String: any Sendable]? {
var params: [String: any Sendable]?

Expand All @@ -19,7 +21,7 @@ import Foundation
let data = try encodeResult.get()
params = JSONDataEncoding.encodingParameters(jsonData: data)
} catch {
print(error.localizedDescription)
params = [encodingErrorParameterKey: RequestBuilderError.bodyEncodingFailed(error)]
}
}

Expand Down
8 changes: 8 additions & 0 deletions modules/openapi-generator/src/main/resources/swift6/Models.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,14 @@ where RawValue: Decodable, AllCases: BidirectionalCollection {}
case requestMissingURL
}

{{#nonPublicApi}}internal{{/nonPublicApi}}{{^nonPublicApi}}public{{/nonPublicApi}} enum RequestBuilderError: Error, Sendable {
case bodyEncodingFailed(any Error)
case unsupportedHTTPMethod(String)
case unsupportedMediaType(String)
case unsupportedParameterValue(key: String)
case unsupportedResponseType(String)
}

{{#nonPublicApi}}internal{{/nonPublicApi}}{{^nonPublicApi}}public{{/nonPublicApi}} enum DecodableRequestBuilderError: Error, Sendable {
case emptyDataResponse
case nilHTTPResponse
Expand Down
101 changes: 70 additions & 31 deletions ...generator/src/main/resources/swift6/libraries/alamofire/AlamofireImplementations.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ fileprivate class AlamofireRequestBuilderConfiguration: @unchecked Sendable {
encoding = JSONDataEncoding()

default:
fatalError("Unsupported HTTPMethod - \(xMethod.rawValue)")
return nil

@4brunu 4brunu Oct 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@spigo shouldn't we do something similar do the URLSessionImplementations instead of returning nil?
return failBeforeSending(RequestBuilderError.unsupportedHTTPMethod(method), urlSession: urlSession, completion: completion)

@spigo spigo Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

createURLRequest() is a separate open helper that returns URLRequest? and has no completion to report to. It already signals failure with nil (the try? URLRequest(...) right below). execute() doesn't use it: on the real request path an unsupported method already fails through failBeforeSending(.unsupportedHTTPMethod), as in URLSessionImplementations. I kept nil because making it throws would change the signature of an open method and break subclasses that override it. Happy to switch it to throws if you prefer.

}

guard let originalRequest = try? URLRequest(url: URLString, method: xMethod, headers: HTTPHeaders(buildHeaders())) else { return nil }
Expand Down Expand Up @@ -92,6 +92,10 @@ fileprivate class AlamofireRequestBuilderConfiguration: @unchecked Sendable {
let manager = createAlamofireSession()
AlamofireRequestBuilderConfiguration.shared.managerStore[managerId] = manager

if let bodyEncodingError = parameters?[JSONEncodingHelper.encodingErrorParameterKey] as? RequestBuilderError {
return failBeforeSending(bodyEncodingError, managerId: managerId, completion: completion)
}

let xMethod = Alamofire.HTTPMethod(rawValue: method)

let encoding: ParameterEncoding?
Expand All @@ -108,30 +112,31 @@ fileprivate class AlamofireRequestBuilderConfiguration: @unchecked Sendable {
} else if contentType.hasPrefix("multipart/form-data") {
encoding = nil

let upload = manager.upload(multipartFormData: { mpForm in
for (k, v) in self.parameters! {
for v in (v as? Array ?? [v]) {
switch v {
case let fileURL as URL:
if let mimeType = self.contentTypeForFormPart(fileURL: fileURL) {
mpForm.append(fileURL, withName: k, fileName: fileURL.lastPathComponent, mimeType: mimeType)
} else {
mpForm.append(fileURL, withName: k)
}
case let string as String:
mpForm.append(string.data(using: String.Encoding.utf8)!, withName: k)
case let number as NSNumber:
mpForm.append(number.stringValue.data(using: String.Encoding.utf8)!, withName: k)
case let data as Data:
mpForm.append(data, withName: k, fileName: k, mimeType: "application/octet-stream")
case let uuid as UUID:
mpForm.append(uuid.uuidString.data(using: String.Encoding.utf8)!, withName: k)
default:
fatalError("Unprocessable value \(v) with key \(k)")
let mpForm = MultipartFormData()
for (k, v) in self.parameters ?? [:] {
for v in (v as? Array ?? [v]) {
switch v {
case let fileURL as URL:
if let mimeType = self.contentTypeForFormPart(fileURL: fileURL) {
mpForm.append(fileURL, withName: k, fileName: fileURL.lastPathComponent, mimeType: mimeType)
} else {
mpForm.append(fileURL, withName: k)
}
case let string as String:
mpForm.append(string.data(using: String.Encoding.utf8)!, withName: k)
Comment on lines +125 to +126

@cubic-dev-ai cubic-dev-ai Bot Oct 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: These rewritten multipart parameter lines still force-unwrap data(using: .utf8)!, which can crash the request instead of failing it. String.data(using: .utf8) returns nil when the string can't be represented in UTF-8 (e.g., "\u{D800}" — pathological but user-reachable), and NSNumber.stringValue.data(...)! / uuid.uuidString.data(...)! on the next cases follow the same pattern. This is exactly the crash class the PR removes elsewhere in this builder, so the String case (the only realistically-nil one) should fail with RequestBuilderError.unsupportedParameterValue like the default: case does.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At modules/openapi-generator/src/main/resources/swift6/libraries/alamofire/AlamofireImplementations.mustache, line 125:

<comment>These rewritten multipart parameter lines still force-unwrap `data(using: .utf8)!`, which can crash the request instead of failing it. `String.data(using: .utf8)` returns nil when the string can't be represented in UTF-8 (e.g., `"\u{D800}"` — pathological but user-reachable), and `NSNumber.stringValue.data(...)!` / `uuid.uuidString.data(...)!` on the next cases follow the same pattern. This is exactly the crash class the PR removes elsewhere in this builder, so the String case (the only realistically-nil one) should fail with `RequestBuilderError.unsupportedParameterValue` like the `default:` case does.</comment>

<file context>
@@ -112,34 +112,31 @@ fileprivate class AlamofireRequestBuilderConfiguration: @unchecked Sendable {
+                            } else {
+                                mpForm.append(fileURL, withName: k)
                             }
+                        case let string as String:
+                            mpForm.append(string.data(using: String.Encoding.utf8)!, withName: k)
+                        case let number as NSNumber:
</file context>
Suggested change
case let string as String:
mpForm.append(string.data(using: String.Encoding.utf8)!, withName: k)
case let string as String:
guard let stringData = string.data(using: .utf8) else {
return failBeforeSending(.unsupportedParameterValue(key: k), managerId: managerId, completion: completion)
}
mpForm.append(stringData, withName: k)

case let number as NSNumber:
mpForm.append(number.stringValue.data(using: String.Encoding.utf8)!, withName: k)
case let data as Data:
mpForm.append(data, withName: k, fileName: k, mimeType: "application/octet-stream")
case let uuid as UUID:
mpForm.append(uuid.uuidString.data(using: String.Encoding.utf8)!, withName: k)
default:
return failBeforeSending(.unsupportedParameterValue(key: k), managerId: managerId, completion: completion)
}
}
}, to: URLString, method: xMethod, headers: nil)
}

let upload = manager.upload(multipartFormData: mpForm, to: URLString, method: xMethod, headers: nil)
.uploadProgress { progress in
if let onProgressReady = self.onProgressReady {
onProgressReady(progress)
Expand All @@ -144,11 +149,11 @@ fileprivate class AlamofireRequestBuilderConfiguration: @unchecked Sendable {
} else if contentType.hasPrefix("application/x-www-form-urlencoded") {
encoding = URLEncoding(destination: .httpBody)
} else {
fatalError("Unsupported Media Type - \(contentType)")
return failBeforeSending(.unsupportedMediaType(contentType), managerId: managerId, completion: completion)
}

default:
fatalError("Unsupported HTTPMethod - \(xMethod.rawValue)")
return failBeforeSending(.unsupportedHTTPMethod(xMethod.rawValue), managerId: managerId, completion: completion)
}

if let encoding = encoding {
Expand Down Expand Up @@ -183,17 +188,34 @@ fileprivate class AlamofireRequestBuilderConfiguration: @unchecked Sendable {

switch voidResponse.result {
case .success:
completion(.success(Response(response: voidResponse.response!, body: () as! T, bodyData: voidResponse.data)))
if let httpResponse = voidResponse.response {
completion(.success(Response(response: httpResponse, body: () as! T, bodyData: voidResponse.data)))
} else {
completion(.failure(ErrorResponse.error(-2, voidResponse.data, nil, DecodableRequestBuilderError.nilHTTPResponse)))
}
case let .failure(error):
completion(.failure(ErrorResponse.error(voidResponse.response?.statusCode ?? -1, voidResponse.data, voidResponse.response, error)))
}

})
default:
fatalError("Unsupported Response Body Type - \(String(describing: T.self))")
request.cancel()
cleanupRequest()
let error = RequestBuilderError.unsupportedResponseType(String(describing: T.self))
apiConfiguration.apiResponseQueue.async {
completion(.failure(ErrorResponse.error(415, nil, nil, error)))
}
}
}

fileprivate func failBeforeSending(_ error: RequestBuilderError, managerId: String, completion: @Sendable @escaping (_ result: Swift.Result<Response<T>, ErrorResponse>) -> Void) -> RequestTask {
AlamofireRequestBuilderConfiguration.shared.managerStore[managerId] = nil
apiConfiguration.apiResponseQueue.async {
completion(.failure(ErrorResponse.error(415, nil, nil, error)))
}
return requestTask
}

{{#nonPublicApi}}internal{{/nonPublicApi}}{{^nonPublicApi}}open{{/nonPublicApi}} func buildHeaders() -> [String: String] {
var httpHeaders = Alamofire.HTTPHeaders.default.dictionary
for (key, value) in headers {
Expand Down Expand Up @@ -277,7 +299,11 @@ fileprivate class AlamofireRequestBuilderConfiguration: @unchecked Sendable {

switch stringResponse.result {
case let .success(value):
completion(.success(Response(response: stringResponse.response!, body: value as! T, bodyData: stringResponse.data)))
if let httpResponse = stringResponse.response {
completion(.success(Response(response: httpResponse, body: value as! T, bodyData: stringResponse.data)))
} else {
completion(.failure(ErrorResponse.error(-2, stringResponse.data, nil, DecodableRequestBuilderError.nilHTTPResponse)))
}
case let .failure(error):
completion(.failure(ErrorResponse.error(stringResponse.response?.statusCode ?? -1, stringResponse.data, stringResponse.response, error)))
}
Expand All @@ -296,6 +322,11 @@ fileprivate class AlamofireRequestBuilderConfiguration: @unchecked Sendable {
return
}

guard let httpResponse = dataResponse.response else {
completion(.failure(ErrorResponse.error(-2, dataResponse.data, nil, DecodableRequestBuilderError.nilHTTPResponse)))
return
}

guard let data = dataResponse.data else {
throw DownloadException.responseDataMissing
}
Expand All @@ -311,7 +342,7 @@ fileprivate class AlamofireRequestBuilderConfiguration: @unchecked Sendable {

var requestPath = try self.getPath(from: requestURL)

if let headerFileName = self.getFileName(fromContentDisposition: dataResponse.response?.allHeaderFields["Content-Disposition"] as? String) {
if let headerFileName = self.getFileName(fromContentDisposition: httpResponse.allHeaderFields["Content-Disposition"] as? String) {
requestPath = requestPath.appending("/\(headerFileName)")
} else {
requestPath = requestPath.appending("/tmp.{{projectName}}.\(UUID().uuidString)")
Expand All @@ -323,7 +354,7 @@ fileprivate class AlamofireRequestBuilderConfiguration: @unchecked Sendable {
try fileManager.createDirectory(atPath: directoryPath, withIntermediateDirectories: true, attributes: nil)
try data.write(to: filePath, options: .atomic)

completion(.success(Response(response: dataResponse.response!, body: filePath as! T, bodyData: data)))
completion(.success(Response(response: httpResponse, body: filePath as! T, bodyData: data)))

} catch let requestParserError as DownloadException {
completion(.failure(ErrorResponse.error(400, dataResponse.data, dataResponse.response, requestParserError)))
Expand All @@ -340,7 +371,11 @@ fileprivate class AlamofireRequestBuilderConfiguration: @unchecked Sendable {

switch voidResponse.result {
case .success:
completion(.success(Response(response: voidResponse.response!, body: () as! T, bodyData: voidResponse.data)))
if let httpResponse = voidResponse.response {
completion(.success(Response(response: httpResponse, body: () as! T, bodyData: voidResponse.data)))
} else {
completion(.failure(ErrorResponse.error(-2, voidResponse.data, nil, DecodableRequestBuilderError.nilHTTPResponse)))
}
case let .failure(error):
completion(.failure(ErrorResponse.error(voidResponse.response?.statusCode ?? -1, voidResponse.data, voidResponse.response, error)))
}
Expand All @@ -354,7 +389,11 @@ fileprivate class AlamofireRequestBuilderConfiguration: @unchecked Sendable {

switch dataResponse.result {
case .success:
completion(.success(Response(response: dataResponse.response!, body: dataResponse.data as! T, bodyData: dataResponse.data)))
if let httpResponse = dataResponse.response {
completion(.success(Response(response: httpResponse, body: (dataResponse.data ?? Data()) as! T, bodyData: dataResponse.data)))
} else {
completion(.failure(ErrorResponse.error(-2, dataResponse.data, nil, DecodableRequestBuilderError.nilHTTPResponse)))
}
case let .failure(error):
completion(.failure(ErrorResponse.error(dataResponse.response?.statusCode ?? -1, dataResponse.data, dataResponse.response, error)))
}
Expand Down
38 changes: 25 additions & 13 deletions ...nerator/src/main/resources/swift6/libraries/urlsession/URLSessionImplementations.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ fileprivate class URLSessionRequestBuilderConfiguration: @unchecked Sendable {
let urlSession = createURLSession()

guard let xMethod = HTTPMethod(rawValue: method) else {
fatalError("Unsupported Http method - \(method)")
return failBeforeSending(RequestBuilderError.unsupportedHTTPMethod(method), urlSession: urlSession, completion: completion)
}

let encoding: ParameterEncoding
Expand All @@ -146,11 +146,15 @@ fileprivate class URLSessionRequestBuilderConfiguration: @unchecked Sendable {
} else if contentType.hasPrefix("application/octet-stream") || contentType.hasPrefix("image/") {
encoding = OctetStreamEncoding()
} else {
fatalError("Unsupported Media Type - \(contentType)")
return failBeforeSending(RequestBuilderError.unsupportedMediaType(contentType), urlSession: urlSession, completion: completion)
}
}

do {
if let bodyEncodingError = parameters?[JSONEncodingHelper.encodingErrorParameterKey] as? RequestBuilderError {
throw bodyEncodingError
}

let request = try createURLRequest(urlSession: urlSession, method: xMethod, encoding: encoding, headers: headers)

apiConfiguration.interceptor.intercept(urlRequest: request, urlSession: urlSession, requestBuilder: self) { result in
Expand Down Expand Up @@ -222,16 +226,22 @@ fileprivate class URLSessionRequestBuilderConfiguration: @unchecked Sendable {
}
}
} catch {
// Request creation failed - create a minimal request for error reporting
let failedURL = URL(string: URLString) ?? URL(string: "about:blank")!
var failedRequest = URLRequest(url: failedURL)
failedRequest.httpMethod = method
return failBeforeSending(error, urlSession: urlSession, completion: completion)
}

return requestTask
}

self.apiConfiguration.interceptor.didComplete(urlRequest: failedRequest, urlSession: urlSession, requestBuilder: self, data: nil, response: nil, result: .failure(error))
private func failBeforeSending(_ error: Error, urlSession: URLSessionProtocol, completion: @Sendable @escaping (_ result: Swift.Result<Response<T>, ErrorResponse>) -> Void) -> RequestTask {
// Request creation failed - create a minimal request for error reporting
let failedURL = URL(string: URLString) ?? URL(string: "about:blank")!
var failedRequest = URLRequest(url: failedURL)
failedRequest.httpMethod = method

self.apiConfiguration.apiResponseQueue.async {
completion(.failure(ErrorResponse.error(415, nil, nil, error)))
}
self.apiConfiguration.interceptor.didComplete(urlRequest: failedRequest, urlSession: urlSession, requestBuilder: self, data: nil, response: nil, result: .failure(error))

self.apiConfiguration.apiResponseQueue.async {
completion(.failure(ErrorResponse.error(415, nil, nil, error)))
}

return requestTask
Expand Down Expand Up @@ -267,7 +277,9 @@ fileprivate class URLSessionRequestBuilderConfiguration: @unchecked Sendable {
completion(.success(Response(response: httpResponse, body: result, bodyData: data)))

default:
fatalError("Unsupported Response Body Type - \(String(describing: T.self))")
let error = RequestBuilderError.unsupportedResponseType(String(describing: T.self))
apiConfiguration.interceptor.didComplete(urlRequest: urlRequest, urlSession: urlSession, requestBuilder: self, data: data, response: httpResponse, result: .failure(error))
completion(.failure(ErrorResponse.error(httpResponse.statusCode, data, httpResponse, error)))
}

}
Expand Down Expand Up @@ -556,7 +568,7 @@ private class FormDataEncoding: ParameterEncoding {
}

default:
fatalError("Unprocessable value \(value) with key \(key)")
throw RequestBuilderError.unsupportedParameterValue(key: key)
}
}
}
Expand Down Expand Up @@ -710,7 +722,7 @@ private class OctetStreamEncoding: ParameterEncoding {
case let data as Data:
urlRequest.httpBody = data
default:
fatalError("Unprocessable body \(body)")
throw RequestBuilderError.unsupportedParameterValue(key: "body")
}

return urlRequest
Expand Down
Loading
Loading