Repository navigation
Conversation
|
@4brunu It should fix that issue, but right now my uploads still fail because they don't have a filename or content type. |
There was a problem hiding this comment.
5 issues found and verified against the latest diff
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. 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. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="samples/client/petstore/swift6/combineDeferredLibrary/PetstoreClient/Classes/OpenAPIs/Infrastructure/APIHelper.swift">
<violation number="1" location="samples/client/petstore/swift6/combineDeferredLibrary/PetstoreClient/Classes/OpenAPIs/Infrastructure/APIHelper.swift:54">
P2: This shared conversion also base64-encodes `Data` elements in array path parameters through `mapValueToPathItem`, although the PR limits encoding to query strings and headers. Apply the base64 conversion only in the query/header serialization paths.</violation>
</file>
<file name="modules/openapi-generator/src/main/resources/swift6/Extensions.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/swift6/Extensions.mustache:91">
P3: For the Alamofire library, `application/x-www-form-urlencoded` form params of type `Data` now reach the request as raw `Data` instead of a base64 `String`. Alamofire's `URLEncoding.queryComponents` has no `Data` case, so the value falls through to the `default` branch and is stringified with `"\(value)"`, producing `Data`'s description (e.g. "5 bytes") rather than the base64 value sent before this change.</violation>
</file>
<file name="samples/client/petstore/swift6/apiNonStaticMethod/Sources/PetstoreClient/Infrastructure/Extensions.swift">
<violation number="1" location="samples/client/petstore/swift6/apiNonStaticMethod/Sources/PetstoreClient/Infrastructure/Extensions.swift:90">
P2: This also sends `Data` form parameters through `application/x-www-form-urlencoded`, where the `byte` value must remain base64 text; `URLEncoding` receives the raw `Data` instead. Preserve raw data only for multipart parts, and base64-encode data in URL-encoded form bodies.</violation>
</file>
<file name="samples/client/petstore/swift6/resultLibrary/PetstoreClient/Classes/OpenAPIs/Infrastructure/APIHelper.swift">
<violation number="1" location="samples/client/petstore/swift6/resultLibrary/PetstoreClient/Classes/OpenAPIs/Infrastructure/APIHelper.swift:55">
P2: This shared conversion also base64-encodes `Data` elements in collection path parameters, although this change is intended only for query strings and headers. Keep the `Data` conversion scoped to those placements so path serialization remains unchanged.</violation>
</file>
<file name="samples/client/petstore/swift6/alamofireLibrary/Sources/PetstoreClient/Infrastructure/Extensions.swift">
<violation number="1" location="samples/client/petstore/swift6/alamofireLibrary/Sources/PetstoreClient/Infrastructure/Extensions.swift:89">
P2: In the Alamofire-based build, Data parameters now lose their base64 encoding when placed in a query string or URL-encoded form body. The Alamofire request path passes `asParameter()` output straight to Alamofire's own `URLEncoding()`/`URLEncoding(destination: .httpBody)` (AlamofireImplementations.swift: `encoding = URLEncoding()` for GET/HEAD and for `application/x-www-form-urlencoded`), so the new base64 branch in `convertAnyToString` is never invoked for those placements; Alamofire string-interpolates the raw `Data` (e.g. "12 bytes") instead. With the old `asParameter` the value reached the encoder as a base64 String, so this is a wire-format regression for Data/`format: byte` query and form-urlencoded parameters (e.g. `testEndpointParameters`'s `byte` param) that the URLSession-based samples do not have, since they encode via `convertAnyToString`. Run Data/byte query params through `convertAnyToString` (or base64-encode at the parameter boundary) in the Alamofire request builder to match the PR's stated behavior.</violation>
</file>
Reply to a comment to ask cubic a question or push back. It learns from your replies.
View guided diff | Re-trigger cubic
| guard let value = value else { return nil } | ||
| if let value = value as? any RawRepresentable { | ||
| return "\(value.rawValue)" | ||
| } else if let data = value as? Data { |
There was a problem hiding this comment.
P2: This shared conversion also base64-encodes Data elements in array path parameters through mapValueToPathItem, although the PR limits encoding to query strings and headers. Apply the base64 conversion only in the query/header serialization paths.
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 samples/client/petstore/swift6/combineDeferredLibrary/PetstoreClient/Classes/OpenAPIs/Infrastructure/APIHelper.swift, line 54:
<comment>This shared conversion also base64-encodes `Data` elements in array path parameters through `mapValueToPathItem`, although the PR limits encoding to query strings and headers. Apply the base64 conversion only in the query/header serialization paths.</comment>
<file context>
@@ -51,6 +51,8 @@ public struct APIHelper {
guard let value = value else { return nil }
if let value = value as? any RawRepresentable {
return "\(value.rawValue)"
+ } else if let data = value as? Data {
+ return data.base64EncodedString(options: Data.Base64EncodingOptions())
} else {
</file context>
| func asParameter(codableHelper: CodableHelper) -> any Sendable { | ||
| return self.base64EncodedString(options: Data.Base64EncodingOptions()) | ||
| } | ||
| func asParameter(codableHelper: CodableHelper) -> any Sendable { self } |
There was a problem hiding this comment.
P2: This also sends Data form parameters through application/x-www-form-urlencoded, where the byte value must remain base64 text; URLEncoding receives the raw Data instead. Preserve raw data only for multipart parts, and base64-encode data in URL-encoded form bodies.
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 samples/client/petstore/swift6/apiNonStaticMethod/Sources/PetstoreClient/Infrastructure/Extensions.swift, line 88:
<comment>This also sends `Data` form parameters through `application/x-www-form-urlencoded`, where the `byte` value must remain base64 text; `URLEncoding` receives the raw `Data` instead. Preserve raw data only for multipart parts, and base64-encode data in URL-encoded form bodies.</comment>
<file context>
@@ -85,9 +85,7 @@ extension Dictionary where Key: Sendable, Value: Sendable {
- func asParameter(codableHelper: CodableHelper) -> any Sendable {
- return self.base64EncodedString(options: Data.Base64EncodingOptions())
- }
+ func asParameter(codableHelper: CodableHelper) -> any Sendable { self }
}
</file context>
| if let value = value as? any RawRepresentable { | ||
| return "\(value.rawValue)" | ||
| } else if let data = value as? Data { | ||
| return data.base64EncodedString(options: Data.Base64EncodingOptions()) |
There was a problem hiding this comment.
P2: This shared conversion also base64-encodes Data elements in collection path parameters, although this change is intended only for query strings and headers. Keep the Data conversion scoped to those placements so path serialization remains unchanged.
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 samples/client/petstore/swift6/resultLibrary/PetstoreClient/Classes/OpenAPIs/Infrastructure/APIHelper.swift, line 55:
<comment>This shared conversion also base64-encodes `Data` elements in collection path parameters, although this change is intended only for query strings and headers. Keep the `Data` conversion scoped to those placements so path serialization remains unchanged.</comment>
<file context>
@@ -51,6 +51,8 @@ internal struct APIHelper {
if let value = value as? any RawRepresentable {
return "\(value.rawValue)"
+ } else if let data = value as? Data {
+ return data.base64EncodedString(options: Data.Base64EncodingOptions())
} else {
return "\(value)"
</file context>
| func asParameter(codableHelper: CodableHelper) -> any Sendable { | ||
| return self.base64EncodedString(options: Data.Base64EncodingOptions()) | ||
| } | ||
| func asParameter(codableHelper: CodableHelper) -> any Sendable { self } |
There was a problem hiding this comment.
P2: In the Alamofire-based build, Data parameters now lose their base64 encoding when placed in a query string or URL-encoded form body. The Alamofire request path passes asParameter() output straight to Alamofire's own URLEncoding()/URLEncoding(destination: .httpBody) (AlamofireImplementations.swift: encoding = URLEncoding() for GET/HEAD and for application/x-www-form-urlencoded), so the new base64 branch in convertAnyToString is never invoked for those placements; Alamofire string-interpolates the raw Data (e.g. "12 bytes") instead. With the old asParameter the value reached the encoder as a base64 String, so this is a wire-format regression for Data/format: byte query and form-urlencoded parameters (e.g. testEndpointParameters's byte param) that the URLSession-based samples do not have, since they encode via convertAnyToString. Run Data/byte query params through convertAnyToString (or base64-encode at the parameter boundary) in the Alamofire request builder to match the PR's stated behavior.
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 samples/client/petstore/swift6/alamofireLibrary/Sources/PetstoreClient/Infrastructure/Extensions.swift, line 87:
<comment>In the Alamofire-based build, Data parameters now lose their base64 encoding when placed in a query string or URL-encoded form body. The Alamofire request path passes `asParameter()` output straight to Alamofire's own `URLEncoding()`/`URLEncoding(destination: .httpBody)` (AlamofireImplementations.swift: `encoding = URLEncoding()` for GET/HEAD and for `application/x-www-form-urlencoded`), so the new base64 branch in `convertAnyToString` is never invoked for those placements; Alamofire string-interpolates the raw `Data` (e.g. "12 bytes") instead. With the old `asParameter` the value reached the encoder as a base64 String, so this is a wire-format regression for Data/`format: byte` query and form-urlencoded parameters (e.g. `testEndpointParameters`'s `byte` param) that the URLSession-based samples do not have, since they encode via `convertAnyToString`. Run Data/byte query params through `convertAnyToString` (or base64-encode at the parameter boundary) in the Alamofire request builder to match the PR's stated behavior.</comment>
<file context>
@@ -84,9 +84,7 @@ extension Dictionary where Key: Sendable, Value: Sendable {
- func asParameter(codableHelper: CodableHelper) -> any Sendable {
- return self.base64EncodedString(options: Data.Base64EncodingOptions())
- }
+ func asParameter(codableHelper: CodableHelper) -> any Sendable { self }
}
</file context>
| func asParameter(codableHelper: CodableHelper) -> any Sendable { | ||
| return self.base64EncodedString(options: Data.Base64EncodingOptions()) | ||
| } | ||
| func asParameter(codableHelper: CodableHelper) -> any Sendable { self } |
There was a problem hiding this comment.
P3: For the Alamofire library, application/x-www-form-urlencoded form params of type Data now reach the request as raw Data instead of a base64 String. Alamofire's URLEncoding.queryComponents has no Data case, so the value falls through to the default branch and is stringified with "\(value)", producing Data's description (e.g. "5 bytes") rather than the base64 value sent before this change.
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/Extensions.mustache, line 89:
<comment>For the Alamofire library, `application/x-www-form-urlencoded` form params of type `Data` now reach the request as raw `Data` instead of a base64 `String`. Alamofire's `URLEncoding.queryComponents` has no `Data` case, so the value falls through to the `default` branch and is stringified with `"\(value)"`, producing `Data`'s description (e.g. "5 bytes") rather than the base64 value sent before this change.</comment>
<file context>
@@ -86,9 +86,7 @@ extension Dictionary where Key: Sendable, Value: Sendable {
- func asParameter(codableHelper: CodableHelper) -> any Sendable {
- return self.base64EncodedString(options: Data.Base64EncodingOptions())
- }
+ func asParameter(codableHelper: CodableHelper) -> any Sendable { self }
}
</file context>
|
@4brunu I don't currently have time to bring this PR up-to-date again. You're free to update this PR, or close it and make a different one. |
|
@x-sheep thanks |
This will make sure Data is not encoded as base64 unless it's necessary for the request, i.e. it's part of the querystring or headers. A FormData request should always preserve the data as-is, to keep the same functionality as passing in a URL.
PR checklist
Commit all changed files.
This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
These must match the expectations made by your contribution.
You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example
./bin/generate-samples.sh bin/configs/java*.IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
master(upcoming7.x.0minor release - breaking changes with fallbacks),8.0.x(breaking changes without fallbacks)"fixes #123"present in the PR description)