Skip to content

Commit

Permalink
Handle malformed content types (#339)
Browse files Browse the repository at this point in the history
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. 

#339
  • Loading branch information
czechboy0 authored Oct 24, 2023
1 parent 110de34 commit f8f88d2
Show file tree
Hide file tree
Showing 8 changed files with 76 additions and 55 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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
}
Expand All @@ -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))
Expand All @@ -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
Expand All @@ -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")
{
Expand All @@ -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))
Expand Down
50 changes: 23 additions & 27 deletions Sources/_OpenAPIGeneratorCore/Translator/Content/ContentType.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
//
//===----------------------------------------------------------------------===//
import OpenAPIKit
import Foundation

/// A content type of a request, response, and other types.
///
Expand Down Expand Up @@ -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
Expand All @@ -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]
}
Expand Down Expand Up @@ -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)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit f8f88d2

Please sign in to comment.