From 0e8a8e98f938ec5a81eacbebc12b3993c1eb84d3 Mon Sep 17 00:00:00 2001 From: Honza Dvorsky Date: Tue, 13 Aug 2024 17:06:56 +0200 Subject: [PATCH 1/4] [Runtime] Improve parameter handling of MIME types in content types --- .../Conversion/Converter+Server.swift | 31 ++++++++++++++++--- .../OpenAPIRuntime/Errors/RuntimeError.swift | 2 ++ .../Conversion/Test_Converter+Server.swift | 9 ++++-- 3 files changed, 36 insertions(+), 6 deletions(-) diff --git a/Sources/OpenAPIRuntime/Conversion/Converter+Server.swift b/Sources/OpenAPIRuntime/Conversion/Converter+Server.swift index 75b0f52..cf4bff2 100644 --- a/Sources/OpenAPIRuntime/Conversion/Converter+Server.swift +++ b/Sources/OpenAPIRuntime/Conversion/Converter+Server.swift @@ -56,11 +56,34 @@ extension Converter { // Drop everything after the optional semicolon (q, extensions, ...) value.split(separator: ";")[0].trimmingCharacters(in: .whitespacesAndNewlines).lowercased() } - if acceptValues.isEmpty { return } - if acceptValues.contains("*/*") { return } - if acceptValues.contains("\(substring.split(separator: "/")[0].lowercased())/*") { return } - if acceptValues.contains(where: { $0.localizedCaseInsensitiveContains(substring) }) { return } + guard let parsedSubstring = OpenAPIMIMEType(substring) else { + throw RuntimeError.invalidAcceptSubstring(substring) + } + guard case .concrete(let substringType, let substringSubtype) = parsedSubstring.kind else { + // If the substring content type has a wildcard, just let it through. + // It's not well defined how such a case should behave, so be permissive. + return + } + + // Look for the first match. + for acceptValue in acceptValues { + // Fast path. + if acceptValue == substring { return } + guard let parsedAcceptValue = OpenAPIMIMEType(acceptValue) else { + throw RuntimeError.invalidExpectedContentType(acceptValue) + } + switch parsedAcceptValue.kind { + case .any: return + case .anySubtype(type: let type): if substringType.lowercased() == type.lowercased() { return } + case .concrete(type: let type, subtype: let subtype): + if type.lowercased() == substringType.lowercased() + && subtype.lowercased() == substringSubtype.lowercased() + { + return + } + } + } throw RuntimeError.unexpectedAcceptHeader(acceptHeader) } diff --git a/Sources/OpenAPIRuntime/Errors/RuntimeError.swift b/Sources/OpenAPIRuntime/Errors/RuntimeError.swift index b0c776e..549e3a1 100644 --- a/Sources/OpenAPIRuntime/Errors/RuntimeError.swift +++ b/Sources/OpenAPIRuntime/Errors/RuntimeError.swift @@ -21,6 +21,7 @@ internal enum RuntimeError: Error, CustomStringConvertible, LocalizedError, Pret case invalidServerURL(String) case invalidServerVariableValue(name: String, value: String, allowedValues: [String]) case invalidExpectedContentType(String) + case invalidAcceptSubstring(String) case invalidHeaderFieldName(String) case invalidBase64String(String) @@ -85,6 +86,7 @@ internal enum RuntimeError: Error, CustomStringConvertible, LocalizedError, Pret return "Invalid server variable named: '\(name)', which has the value: '\(value)', but the only allowed values are: \(allowedValues.map { "'\($0)'" }.joined(separator: ", "))" case .invalidExpectedContentType(let string): return "Invalid expected content type: '\(string)'" + case .invalidAcceptSubstring(let string): return "Invalid Accept header content type: '\(string)'" case .invalidHeaderFieldName(let name): return "Invalid header field name: '\(name)'" case .invalidBase64String(let string): return "Invalid base64-encoded string (first 128 bytes): '\(string.prefix(128))'" diff --git a/Tests/OpenAPIRuntimeTests/Conversion/Test_Converter+Server.swift b/Tests/OpenAPIRuntimeTests/Conversion/Test_Converter+Server.swift index b2305a0..aece51c 100644 --- a/Tests/OpenAPIRuntimeTests/Conversion/Test_Converter+Server.swift +++ b/Tests/OpenAPIRuntimeTests/Conversion/Test_Converter+Server.swift @@ -39,12 +39,13 @@ final class Test_ServerConverterExtensions: Test_Runtime { .accept: "text/html, application/xhtml+xml, application/xml;q=0.9, image/webp, */*;q=0.8" ] let multiple: HTTPFields = [.accept: "text/plain, application/json"] + let params: HTTPFields = [.accept: "application/json; foo=bar"] let cases: [(HTTPFields, String, Bool)] = [ // No Accept header, any string validates successfully (emptyHeaders, "foobar", true), - // Accept: */*, any string validates successfully - (wildcard, "foobar", true), + // Accept: */*, any MIME type validates successfully + (wildcard, "foobaz/bar", true), // Accept: text/*, so text/plain succeeds, application/json fails (partialWildcard, "text/plain", true), (partialWildcard, "application/json", false), @@ -58,6 +59,10 @@ final class Test_ServerConverterExtensions: Test_Runtime { // Multiple values (multiple, "text/plain", true), (multiple, "application/json", true), (multiple, "application/xml", false), + + // Params + (params, "application/json; foo=bar", true), (params, "application/json; charset=utf-8; foo=bar", true), + (params, "application/json", true), (params, "text/plain", false), ] for (headers, contentType, success) in cases { if success { From 71849ae603372288c291c72d7cbd48daa044481c Mon Sep 17 00:00:00 2001 From: Honza Dvorsky Date: Mon, 23 Sep 2024 14:26:54 +0200 Subject: [PATCH 2/4] WIP on rewriting, doesn't build yet --- .../Conversion/Converter+Server.swift | 37 ++++++++++++++----- .../Conversion/Test_Converter+Server.swift | 1 + 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/Sources/OpenAPIRuntime/Conversion/Converter+Server.swift b/Sources/OpenAPIRuntime/Conversion/Converter+Server.swift index cf4bff2..702797b 100644 --- a/Sources/OpenAPIRuntime/Conversion/Converter+Server.swift +++ b/Sources/OpenAPIRuntime/Conversion/Converter+Server.swift @@ -60,12 +60,6 @@ extension Converter { guard let parsedSubstring = OpenAPIMIMEType(substring) else { throw RuntimeError.invalidAcceptSubstring(substring) } - guard case .concrete(let substringType, let substringSubtype) = parsedSubstring.kind else { - // If the substring content type has a wildcard, just let it through. - // It's not well defined how such a case should behave, so be permissive. - return - } - // Look for the first match. for acceptValue in acceptValues { // Fast path. @@ -73,9 +67,34 @@ extension Converter { guard let parsedAcceptValue = OpenAPIMIMEType(acceptValue) else { throw RuntimeError.invalidExpectedContentType(acceptValue) } - switch parsedAcceptValue.kind { - case .any: return - case .anySubtype(type: let type): if substringType.lowercased() == type.lowercased() { return } + switch (parsedAcceptValue.kind, parsedSubstring.kind) { + case (.any, _): + // Accept: */* always matches + return + case (.anySubtype(type: let acceptType), let substring): + switch substring { + case .any: + // */* as a concrete content type is NOT a match for an Accept header of foo/* + break + case .anySubtype(type: let substringType): + if substringType.lowercased() == acceptType.lowercased() { + return + } + case .concrete(type: let substringType, subtype: let substringSubtype): + if + substringType.lowercased() == acceptType.lowercased() && + + { + return + } + + if accept.lowercased() == substringType.lowercased() + && subtype.lowercased() == substringSubtype.lowercased() + { + return + } + } + case .concrete(type: let type, subtype: let subtype): if type.lowercased() == substringType.lowercased() && subtype.lowercased() == substringSubtype.lowercased() diff --git a/Tests/OpenAPIRuntimeTests/Conversion/Test_Converter+Server.swift b/Tests/OpenAPIRuntimeTests/Conversion/Test_Converter+Server.swift index aece51c..a830eaa 100644 --- a/Tests/OpenAPIRuntimeTests/Conversion/Test_Converter+Server.swift +++ b/Tests/OpenAPIRuntimeTests/Conversion/Test_Converter+Server.swift @@ -52,6 +52,7 @@ final class Test_ServerConverterExtensions: Test_Runtime { // Accept: text/plain, text/plain succeeds, application/json fails (short, "text/plain", true), (short, "application/json", false), + (short, "application/*", false), (short, "*/*", false), // A bunch of acceptable content types (long, "text/html", true), (long, "application/xhtml+xml", true), (long, "application/xml", true), From 6604f964874b8d01c347a8afd138c10709a16e07 Mon Sep 17 00:00:00 2001 From: Honza Dvorsky Date: Fri, 4 Oct 2024 16:50:22 +0200 Subject: [PATCH 3/4] Fix up the new approach based on PR feedback --- .../Conversion/Converter+Server.swift | 23 +++++++------------ 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/Sources/OpenAPIRuntime/Conversion/Converter+Server.swift b/Sources/OpenAPIRuntime/Conversion/Converter+Server.swift index 702797b..c07cdb9 100644 --- a/Sources/OpenAPIRuntime/Conversion/Converter+Server.swift +++ b/Sources/OpenAPIRuntime/Conversion/Converter+Server.swift @@ -77,30 +77,23 @@ extension Converter { // */* as a concrete content type is NOT a match for an Accept header of foo/* break case .anySubtype(type: let substringType): + // Only match if the types match. if substringType.lowercased() == acceptType.lowercased() { return } - case .concrete(type: let substringType, subtype: let substringSubtype): - if - substringType.lowercased() == acceptType.lowercased() && - - { + case .concrete(type: let substringType, _): + if substringType.lowercased() == acceptType.lowercased() { return } - - if accept.lowercased() == substringType.lowercased() - && subtype.lowercased() == substringSubtype.lowercased() + } + case (.concrete(type: let acceptType, subtype: let acceptSubtype), let substring): + if case let .concrete(substringType, substringSubtype) = substring { + if acceptType.lowercased() == substringType.lowercased() + && acceptSubtype.lowercased() == substringSubtype.lowercased() { return } } - - case .concrete(type: let type, subtype: let subtype): - if type.lowercased() == substringType.lowercased() - && subtype.lowercased() == substringSubtype.lowercased() - { - return - } } } throw RuntimeError.unexpectedAcceptHeader(acceptHeader) From b240a07edf2d2a4c2b39e363d49fa3ae0aa22fb3 Mon Sep 17 00:00:00 2001 From: Honza Dvorsky Date: Fri, 4 Oct 2024 17:04:12 +0200 Subject: [PATCH 4/4] Reformat --- Sources/OpenAPIRuntime/Conversion/Converter+Server.swift | 8 ++------ .../Conversion/Test_Converter+Server.swift | 4 ++-- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/Sources/OpenAPIRuntime/Conversion/Converter+Server.swift b/Sources/OpenAPIRuntime/Conversion/Converter+Server.swift index c07cdb9..185711b 100644 --- a/Sources/OpenAPIRuntime/Conversion/Converter+Server.swift +++ b/Sources/OpenAPIRuntime/Conversion/Converter+Server.swift @@ -78,13 +78,9 @@ extension Converter { break case .anySubtype(type: let substringType): // Only match if the types match. - if substringType.lowercased() == acceptType.lowercased() { - return - } + if substringType.lowercased() == acceptType.lowercased() { return } case .concrete(type: let substringType, _): - if substringType.lowercased() == acceptType.lowercased() { - return - } + if substringType.lowercased() == acceptType.lowercased() { return } } case (.concrete(type: let acceptType, subtype: let acceptSubtype), let substring): if case let .concrete(substringType, substringSubtype) = substring { diff --git a/Tests/OpenAPIRuntimeTests/Conversion/Test_Converter+Server.swift b/Tests/OpenAPIRuntimeTests/Conversion/Test_Converter+Server.swift index a830eaa..9e16fbf 100644 --- a/Tests/OpenAPIRuntimeTests/Conversion/Test_Converter+Server.swift +++ b/Tests/OpenAPIRuntimeTests/Conversion/Test_Converter+Server.swift @@ -51,8 +51,8 @@ final class Test_ServerConverterExtensions: Test_Runtime { (partialWildcard, "text/plain", true), (partialWildcard, "application/json", false), // Accept: text/plain, text/plain succeeds, application/json fails - (short, "text/plain", true), (short, "application/json", false), - (short, "application/*", false), (short, "*/*", false), + (short, "text/plain", true), (short, "application/json", false), (short, "application/*", false), + (short, "*/*", false), // A bunch of acceptable content types (long, "text/html", true), (long, "application/xhtml+xml", true), (long, "application/xml", true),