Skip to content
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

Fix encoding of plain text bodies #9

Merged
merged 2 commits into from
May 26, 2023
Merged

Fix encoding of plain text bodies #9

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
59026c4
Fix encoding of plain text bodies
czechboy0 May 26, 2023
0cf839e
Formatting fixes
czechboy0 May 26, 2023
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
19 changes: 18 additions & 1 deletion Sources/OpenAPIRuntime/Conversion/Converter+Client.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,18 @@ extension Converter {
from data: Data,
transforming transform: (T) -> C
) throws -> C {
let decoded = try decoder.decode(type, from: data)
let decoded: T
if let myType = T.self as? _StringParameterConvertible.Type {
guard
let stringValue = String(data: data, encoding: .utf8),
Copy link
Member

Choose a reason for hiding this comment

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

Please avoid String(data:encoding:), especially in networked software as it has 2 failure modes (return nil or insert replacement characters for invalid UTF8 -- which one it chooses depends on what's broken).
The correct and faster way is String(decoding: data, as: UTF8.self) which also doesn't require Foundation and is not fallible

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, broke out into apple/swift-openapi-generator#42

let decodedValue = myType.init(stringValue)
else {
throw RuntimeError.failedToDecodePrimitiveBodyFromData
}
decoded = decodedValue as! T
} else {
decoded = try decoder.decode(type, from: data)
}
czechboy0 marked this conversation as resolved.
Show resolved Hide resolved
return transform(decoded)
}

Expand Down Expand Up @@ -128,6 +139,12 @@ extension Converter {
) throws -> Data {
let body = transform(value)
headerFields.add(name: "content-type", value: body.contentType)
if let value = value as? _StringParameterConvertible {
guard let data = value.description.data(using: .utf8) else {
Copy link
Member

Choose a reason for hiding this comment

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

Same here, the correct and fast way is Data(value.utf8) which can't fail and is faster. You should audit your code to make sure String(data and data(using aren't present

Copy link
Contributor Author

Choose a reason for hiding this comment

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

apple/swift-openapi-generator#42

throw RuntimeError.failedToEncodePrimitiveBodyIntoData
}
return data
}
return try encoder.encode(body.value)
}

Expand Down
22 changes: 20 additions & 2 deletions Sources/OpenAPIRuntime/Conversion/Converter+Server.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,18 @@ public extension Converter {
guard let data else {
return nil
}
let decoded = try decoder.decode(type, from: data)
let decoded: T
if let myType = T.self as? _StringParameterConvertible.Type {
guard
let stringValue = String(data: data, encoding: .utf8),
let decodedValue = myType.init(stringValue)
else {
throw RuntimeError.failedToDecodePrimitiveBodyFromData
}
decoded = decodedValue as! T
} else {
decoded = try decoder.decode(type, from: data)
}
return transform(decoded)
}

Expand Down Expand Up @@ -297,7 +308,14 @@ public extension Converter {
) throws -> Data {
let body = transform(value)
headerFields.add(name: "content-type", value: body.contentType)
return try encoder.encode(body.value)
let bodyValue = body.value
if let value = bodyValue as? _StringParameterConvertible {
guard let data = value.description.data(using: .utf8) else {
throw RuntimeError.failedToEncodePrimitiveBodyIntoData
}
return data
}
return try encoder.encode(bodyValue)
}

// MARK: Body - Data
Expand Down
6 changes: 6 additions & 0 deletions Sources/OpenAPIRuntime/Errors/RuntimeError.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ internal enum RuntimeError: Error, CustomStringConvertible, LocalizedError, Pret

// Body
case missingRequiredRequestBody
case failedToEncodePrimitiveBodyIntoData
case failedToDecodePrimitiveBodyFromData

// Transport/Handler
case transportFailed(Error)
Expand Down Expand Up @@ -69,6 +71,10 @@ internal enum RuntimeError: Error, CustomStringConvertible, LocalizedError, Pret
return "Failed to decode query parameter named '\(name)' to type \(type)."
case .missingRequiredRequestBody:
return "Missing required request body"
case .failedToEncodePrimitiveBodyIntoData:
return "Failed to encode a primitive body into data"
case .failedToDecodePrimitiveBodyFromData:
return "Failed to decode a primitive body from data"
case .transportFailed(let underlyingError):
return "Transport failed with error: \(underlyingError.localizedDescription)"
case .handlerFailed(let underlyingError):
Expand Down
82 changes: 82 additions & 0 deletions Tests/OpenAPIRuntimeTests/Conversion/Test_Converter+Client.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,24 @@ final class Test_ClientConverterExtensions: Test_Runtime {
XCTAssertEqual(body, testStruct)
}

func testBodyGetData_success() throws {
let body = try converter.bodyGet(
Data.self,
from: testStructData,
transforming: { $0 }
)
XCTAssertEqual(body, testStructData)
}

func testBodyGetString_success() throws {
let body = try converter.bodyGet(
String.self,
from: testStringData,
transforming: { $0 }
)
XCTAssertEqual(body, testString)
}

func testBodyAddComplexOptional_success() throws {
var headerFields: [HeaderField] = []
let data = try converter.bodyAddOptional(
Expand Down Expand Up @@ -116,4 +134,68 @@ final class Test_ClientConverterExtensions: Test_Runtime {
]
)
}

func testBodyAddDataOptional_success() throws {
var headerFields: [HeaderField] = []
let data = try converter.bodyAddOptional(
testStructPrettyData,
headerFields: &headerFields,
transforming: { .init(value: $0, contentType: "application/octet-stream") }
)
XCTAssertEqual(data, testStructPrettyData)
XCTAssertEqual(
headerFields,
[
.init(name: "content-type", value: "application/octet-stream")
]
)
}

func testBodyAddDataRequired_success() throws {
var headerFields: [HeaderField] = []
let data = try converter.bodyAddRequired(
testStructPrettyData,
headerFields: &headerFields,
transforming: { .init(value: $0, contentType: "application/octet-stream") }
)
XCTAssertEqual(data, testStructPrettyData)
XCTAssertEqual(
headerFields,
[
.init(name: "content-type", value: "application/octet-stream")
]
)
}

func testBodyAddStringOptional_success() throws {
var headerFields: [HeaderField] = []
let data = try converter.bodyAddOptional(
testString,
headerFields: &headerFields,
transforming: { .init(value: $0, contentType: "text/plain") }
)
XCTAssertEqual(data, testStringData)
XCTAssertEqual(
headerFields,
[
.init(name: "content-type", value: "text/plain")
]
)
}

func testBodyAddStringRequired_success() throws {
var headerFields: [HeaderField] = []
let data = try converter.bodyAddRequired(
testString,
headerFields: &headerFields,
transforming: { .init(value: $0, contentType: "text/plain") }
)
XCTAssertEqual(data, testStringData)
XCTAssertEqual(
headerFields,
[
.init(name: "content-type", value: "text/plain")
]
)
}
}
68 changes: 68 additions & 0 deletions Tests/OpenAPIRuntimeTests/Conversion/Test_Converter+Server.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -538,6 +538,38 @@ final class Test_ServerConverterExtensions: Test_Runtime {
)
}

func testBodyAddString() throws {
var headers: [HeaderField] = []
let data = try converter.bodyAdd(
testString,
headerFields: &headers,
transforming: { .init(value: $0, contentType: "text/plain") }
)
XCTAssertEqual(String(data: data, encoding: .utf8)!, testString)
XCTAssertEqual(
headers,
[
.init(name: "content-type", value: "text/plain")
]
)
}

func testBodyAddData() throws {
var headers: [HeaderField] = []
let data = try converter.bodyAdd(
testStructPrettyData,
headerFields: &headers,
transforming: { .init(value: $0, contentType: "application/octet-stream") }
)
XCTAssertEqual(data, testStructPrettyData)
XCTAssertEqual(
headers,
[
.init(name: "content-type", value: "application/octet-stream")
]
)
}

func testBodyGetComplexOptional_success() throws {
let body = try converter.bodyGetOptional(
TestPet.self,
Expand Down Expand Up @@ -584,4 +616,40 @@ final class Test_ServerConverterExtensions: Test_Runtime {
}
)
}

func testBodyGetDataOptional_success() throws {
let body = try converter.bodyGetOptional(
Data.self,
from: testStructPrettyData,
transforming: { $0 }
)
XCTAssertEqual(body, testStructPrettyData)
}

func testBodyGetDataRequired_success() throws {
let body = try converter.bodyGetOptional(
Data.self,
from: testStructPrettyData,
transforming: { $0 }
)
XCTAssertEqual(body, testStructPrettyData)
}

func testBodyGetStringOptional_success() throws {
let body = try converter.bodyGetOptional(
String.self,
from: testStringData,
transforming: { $0 }
)
XCTAssertEqual(body, testString)
}

func testBodyGetStringRequired_success() throws {
let body = try converter.bodyGetOptional(
String.self,
from: testStringData,
transforming: { $0 }
)
XCTAssertEqual(body, testString)
}
}
8 changes: 8 additions & 0 deletions Tests/OpenAPIRuntimeTests/Test_Runtime.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,14 @@ class Test_Runtime: XCTestCase {
"2023-01-18T10:04:11Z"
}

var testString: String {
"hello"
}

var testStringData: Data {
"hello".data(using: .utf8)!
}

var testStruct: TestPet {
.init(name: "Fluffz")
}
Expand Down