From f8f88d2660eaee43d90f27fa7c8232470dea189b Mon Sep 17 00:00:00 2001 From: Honza Dvorsky Date: Tue, 24 Oct 2023 08:33:04 +0200 Subject: [PATCH] Handle malformed content types (#339) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Handle malformed content types ### Motivation Before this PR, when a malformed content type (one that wasn't in the format `foo/bar`) was provided in the OpenAPI document, the generator would crash on a precondition failure, making it difficult to debug. ### Modifications Turn the precondition failure into a thrown error. ### Result Malformed content types now produce a thrown error instead of a crash. ### Test Plan All tests pass. Reviewed by: dnadoba Builds: ✔︎ pull request validation (5.10) - Build finished. ✔︎ pull request validation (5.8) - Build finished. ✔︎ pull request validation (5.9) - Build finished. ✔︎ pull request validation (compatibility test) - Build finished. ✔︎ pull request validation (docc test) - Build finished. ✔︎ pull request validation (integration test) - Build finished. ✔︎ pull request validation (nightly) - Build finished. ✔︎ pull request validation (soundness) - Build finished. https://github.com/apple/swift-openapi-generator/pull/339 --- .../Renderer/TextBasedRenderer.swift | 7 ++- .../translateStringEnum.swift | 7 ++- .../Translator/Content/ContentInspector.swift | 49 ++++++++++-------- .../Translator/Content/ContentType.swift | 50 +++++++++---------- .../Responses/acceptHeaderContentTypes.swift | 2 +- .../Renderer/Test_TextBasedRenderer.swift | 8 +++ .../Content/Test_ContentSwiftName.swift | 6 +-- .../Translator/Content/Test_ContentType.swift | 2 +- 8 files changed, 76 insertions(+), 55 deletions(-) diff --git a/Sources/_OpenAPIGeneratorCore/Renderer/TextBasedRenderer.swift b/Sources/_OpenAPIGeneratorCore/Renderer/TextBasedRenderer.swift index ffd4b1a5..e5b268ff 100644 --- a/Sources/_OpenAPIGeneratorCore/Renderer/TextBasedRenderer.swift +++ b/Sources/_OpenAPIGeneratorCore/Renderer/TextBasedRenderer.swift @@ -346,7 +346,12 @@ struct TextBasedRenderer: RendererProtocol { func renderedLiteral(_ literal: LiteralDescription) -> String { switch literal { case let .string(string): - return "\"\(string)\"" + // Use a raw literal if the string contains a quote/backslash. + if string.contains("\"") || string.contains("\\") { + return "#\"\(string)\"#" + } else { + return "\"\(string)\"" + } case let .int(int): return "\(int)" case let .bool(bool): diff --git a/Sources/_OpenAPIGeneratorCore/Translator/CommonTranslations/translateStringEnum.swift b/Sources/_OpenAPIGeneratorCore/Translator/CommonTranslations/translateStringEnum.swift index 144d5b72..68e1aa32 100644 --- a/Sources/_OpenAPIGeneratorCore/Translator/CommonTranslations/translateStringEnum.swift +++ b/Sources/_OpenAPIGeneratorCore/Translator/CommonTranslations/translateStringEnum.swift @@ -60,7 +60,12 @@ extension FileTranslator { let caseName = swiftSafeName(for: rawValue) return (caseName, .string(rawValue)) case .integer: - guard let rawValue = anyValue as? Int else { + let rawValue: Int + if let intRawValue = anyValue as? Int { + rawValue = intRawValue + } else if let stringRawValue = anyValue as? String, let intRawValue = Int(stringRawValue) { + rawValue = intRawValue + } else { throw GenericError(message: "Disallowed value for an integer enum '\(typeName)': \(anyValue)") } let caseName = rawValue < 0 ? "_n\(abs(rawValue))" : "_\(rawValue)" diff --git a/Sources/_OpenAPIGeneratorCore/Translator/Content/ContentInspector.swift b/Sources/_OpenAPIGeneratorCore/Translator/Content/ContentInspector.swift index 105890b8..86b667bd 100644 --- a/Sources/_OpenAPIGeneratorCore/Translator/Content/ContentInspector.swift +++ b/Sources/_OpenAPIGeneratorCore/Translator/Content/ContentInspector.swift @@ -38,7 +38,7 @@ extension FileTranslator { inParent parent: TypeName ) throws -> TypedSchemaContent? { guard - let content = bestSingleContent( + let content = try bestSingleContent( map, excludeBinary: excludeBinary, foundIn: parent.description @@ -78,7 +78,7 @@ extension FileTranslator { excludeBinary: Bool = false, inParent parent: TypeName ) throws -> [TypedSchemaContent] { - let contents = supportedContents( + let contents = try supportedContents( map, excludeBinary: excludeBinary, foundIn: parent.description @@ -111,18 +111,19 @@ extension FileTranslator { /// - foundIn: The location where this content is parsed. /// - Returns: the detected content type + schema, nil if no supported /// schema found or if empty. + /// - Throws: If parsing of any of the contents throws. func supportedContents( _ contents: OpenAPI.Content.Map, excludeBinary: Bool = false, foundIn: String - ) -> [SchemaContent] { + ) throws -> [SchemaContent] { guard !contents.isEmpty else { return [] } return - contents + try contents .compactMap { key, value in - parseContentIfSupported( + try parseContentIfSupported( contentKey: key, contentValue: value, excludeBinary: excludeBinary, @@ -145,11 +146,12 @@ extension FileTranslator { /// - foundIn: The location where this content is parsed. /// - Returns: the detected content type + schema, nil if no supported /// schema found or if empty. + /// - Throws: If a malformed content type string is encountered. func bestSingleContent( _ map: OpenAPI.Content.Map, excludeBinary: Bool = false, foundIn: String - ) -> SchemaContent? { + ) throws -> SchemaContent? { guard !map.isEmpty else { return nil } @@ -159,19 +161,25 @@ extension FileTranslator { foundIn: foundIn ) } - let chosenContent: (SchemaContent, OpenAPI.Content)? - if let (contentKey, contentValue) = map.first(where: { $0.key.isJSON }) { - let contentType = contentKey.asGeneratorContentType + let mapWithContentTypes = try map.map { key, content in + try (type: key.asGeneratorContentType, value: content) + } + + let chosenContent: (type: ContentType, schema: SchemaContent, content: OpenAPI.Content)? + if let (contentType, contentValue) = mapWithContentTypes.first(where: { $0.type.isJSON }) { chosenContent = ( + contentType, .init( contentType: contentType, schema: contentValue.schema ), contentValue ) - } else if !excludeBinary, let (contentKey, contentValue) = map.first(where: { $0.key.isBinary }) { - let contentType = contentKey.asGeneratorContentType + } else if !excludeBinary, + let (contentType, contentValue) = mapWithContentTypes.first(where: { $0.type.isBinary }) + { chosenContent = ( + contentType, .init( contentType: contentType, schema: .b(.string(format: .binary)) @@ -186,18 +194,18 @@ extension FileTranslator { chosenContent = nil } if let chosenContent { - let contentType = chosenContent.0.contentType + let contentType = chosenContent.type if contentType.lowercasedType == "multipart" || contentType.lowercasedTypeAndSubtype.contains("application/x-www-form-urlencoded") { diagnostics.emitUnsupportedIfNotNil( - chosenContent.1.encoding, + chosenContent.content.encoding, "Custom encoding for multipart/formEncoded content", foundIn: "\(foundIn), content \(contentType.originallyCasedTypeAndSubtype)" ) } } - return chosenContent?.0 + return chosenContent?.schema } /// Returns a wrapped version of the provided content if supported, returns @@ -215,14 +223,15 @@ extension FileTranslator { /// type should be skipped, for example used when encoding headers. /// - foundIn: The location where this content is parsed. /// - Returns: The detected content type + schema, nil if unsupported. + /// - Throws: If a malformed content type string is encountered. func parseContentIfSupported( contentKey: OpenAPI.ContentType, contentValue: OpenAPI.Content, excludeBinary: Bool = false, foundIn: String - ) -> SchemaContent? { - if contentKey.isJSON { - let contentType = contentKey.asGeneratorContentType + ) throws -> SchemaContent? { + let contentType = try contentKey.asGeneratorContentType + if contentType.isJSON { if contentType.lowercasedType == "multipart" || contentType.lowercasedTypeAndSubtype.contains("application/x-www-form-urlencoded") { @@ -237,15 +246,13 @@ extension FileTranslator { schema: contentValue.schema ) } - if contentKey.isUrlEncodedForm { - let contentType = ContentType(contentKey.typeAndSubtype) + if contentType.isUrlEncodedForm { return .init( contentType: contentType, schema: contentValue.schema ) } - if !excludeBinary, contentKey.isBinary { - let contentType = contentKey.asGeneratorContentType + if !excludeBinary, contentType.isBinary { return .init( contentType: contentType, schema: .b(.string(format: .binary)) diff --git a/Sources/_OpenAPIGeneratorCore/Translator/Content/ContentType.swift b/Sources/_OpenAPIGeneratorCore/Translator/Content/ContentType.swift index 78368fd2..8b40f71d 100644 --- a/Sources/_OpenAPIGeneratorCore/Translator/Content/ContentType.swift +++ b/Sources/_OpenAPIGeneratorCore/Translator/Content/ContentType.swift @@ -12,6 +12,7 @@ // //===----------------------------------------------------------------------===// import OpenAPIKit +import Foundation /// A content type of a request, response, and other types. /// @@ -147,12 +148,24 @@ struct ContentType: Hashable { } /// Creates a new content type by parsing the specified MIME type. - /// - Parameter rawValue: A MIME type, for example "application/json". Must + /// - Parameter string: A MIME type, for example "application/json". Must /// not be empty. - init(_ rawValue: String) { - precondition(!rawValue.isEmpty, "rawValue of a ContentType cannot be empty.") + /// - Throws: If a malformed content type string is encountered. + init(string: String) throws { + struct InvalidContentTypeString: Error, LocalizedError, CustomStringConvertible { + var string: String + var description: String { + "Invalid content type string: '\(string)', must have 2 components separated by a slash." + } + var errorDescription: String? { + description + } + } + guard !string.isEmpty else { + throw InvalidContentTypeString(string: "") + } var semiComponents = - rawValue + string .split(separator: ";") let typeAndSubtypeComponent = semiComponents.removeFirst() self.originallyCasedParameterPairs = semiComponents.map { component in @@ -168,10 +181,9 @@ struct ContentType: Hashable { rawTypeAndSubtype .split(separator: "/") .map(String.init) - precondition( - typeAndSubtype.count == 2, - "Invalid ContentType string, must have 2 components separated by a slash." - ) + guard typeAndSubtype.count == 2 else { + throw InvalidContentTypeString(string: rawTypeAndSubtype) + } self.originallyCasedType = typeAndSubtype[0] self.originallyCasedSubtype = typeAndSubtype[1] } @@ -251,27 +263,11 @@ struct ContentType: Hashable { extension OpenAPI.ContentType { - /// A Boolean value that indicates whether the content type - /// is a type of JSON. - var isJSON: Bool { - asGeneratorContentType.isJSON - } - - /// A Boolean value that indicates whether the content type - /// is a URL-encoded form. - var isUrlEncodedForm: Bool { - asGeneratorContentType.isUrlEncodedForm - } - - /// A Boolean value that indicates whether the content type - /// is just binary data. - var isBinary: Bool { - asGeneratorContentType.isBinary - } - /// Returns the content type wrapped in the generator's representation /// of a content type, as opposed to the one from OpenAPIKit. var asGeneratorContentType: ContentType { - ContentType(rawValue) + get throws { + try ContentType(string: rawValue) + } } } diff --git a/Sources/_OpenAPIGeneratorCore/Translator/Responses/acceptHeaderContentTypes.swift b/Sources/_OpenAPIGeneratorCore/Translator/Responses/acceptHeaderContentTypes.swift index 5a45adb4..faa3a797 100644 --- a/Sources/_OpenAPIGeneratorCore/Translator/Responses/acceptHeaderContentTypes.swift +++ b/Sources/_OpenAPIGeneratorCore/Translator/Responses/acceptHeaderContentTypes.swift @@ -31,7 +31,7 @@ extension FileTranslator { .responseOutcomes .flatMap { outcome in let response = try outcome.response.resolve(in: components) - return supportedContents(response.content, foundIn: description.operationID) + return try supportedContents(response.content, foundIn: description.operationID) } .map { content in content.contentType diff --git a/Tests/OpenAPIGeneratorCoreTests/Renderer/Test_TextBasedRenderer.swift b/Tests/OpenAPIGeneratorCoreTests/Renderer/Test_TextBasedRenderer.swift index fb8317c3..e0c9d72b 100644 --- a/Tests/OpenAPIGeneratorCoreTests/Renderer/Test_TextBasedRenderer.swift +++ b/Tests/OpenAPIGeneratorCoreTests/Renderer/Test_TextBasedRenderer.swift @@ -187,6 +187,14 @@ final class Test_TextBasedRenderer: XCTestCase { "hi" """# ) + try _test( + .string("this string: \"foo\""), + renderedBy: renderer.renderedLiteral, + rendersAs: + #""" + #"this string: "foo""# + """# + ) try _test( .nil, renderedBy: renderer.renderedLiteral, diff --git a/Tests/OpenAPIGeneratorCoreTests/Translator/Content/Test_ContentSwiftName.swift b/Tests/OpenAPIGeneratorCoreTests/Translator/Content/Test_ContentSwiftName.swift index f2b334f0..6bd52b4f 100644 --- a/Tests/OpenAPIGeneratorCoreTests/Translator/Content/Test_ContentSwiftName.swift +++ b/Tests/OpenAPIGeneratorCoreTests/Translator/Content/Test_ContentSwiftName.swift @@ -45,9 +45,9 @@ final class Test_ContentSwiftName: Test_Core { ("application/foo; bar=baz; boo=foo", "application_foo_bar_baz_boo_foo"), ("application/foo; bar = baz", "application_foo_bar_baz"), ] - for item in cases { - let contentType = try XCTUnwrap(ContentType(item.0)) - XCTAssertEqual(nameMaker(contentType), item.1, "Case \(item.0) failed") + for (string, name) in cases { + let contentType = try XCTUnwrap(ContentType(string: string)) + XCTAssertEqual(nameMaker(contentType), name, "Case \(string) failed") } } } diff --git a/Tests/OpenAPIGeneratorCoreTests/Translator/Content/Test_ContentType.swift b/Tests/OpenAPIGeneratorCoreTests/Translator/Content/Test_ContentType.swift index db7b51de..3c845ee4 100644 --- a/Tests/OpenAPIGeneratorCoreTests/Translator/Content/Test_ContentType.swift +++ b/Tests/OpenAPIGeneratorCoreTests/Translator/Content/Test_ContentType.swift @@ -170,7 +170,7 @@ final class Test_ContentType: Test_Core { originallyCasedTypeAndSubtype, originallyCasedOutputWithParameters ) in cases { - let contentType = ContentType(rawValue) + let contentType = try ContentType(string: rawValue) XCTAssertEqual(contentType.category, category) XCTAssertEqual(contentType.lowercasedType, type) XCTAssertEqual(contentType.lowercasedSubtype, subtype)