Runtime URIDecoder fails when parsing strings containing commas

[bfrearson]

Commit hash: f0589f6

Context:
URIDecoder fails with error DecodingError: typeMismatch String - at : Cannot parse a value from a node with multiple values. (underlying error: <nil>) when trying to parse a value that contains a comma (e.g. no-cache, private in a header response).

I traced this back to URIParser which is splitting elements by commas.

Steps to reproduce:

1. Attempt to decode a header response containing a comma as part of a request
2. Request fails with DecodingError.typeMismatch

$ swift --version
Apple Swift version 5.8.1 (swiftlang-5.8.0.124.5 clang-1403.0.22.11.100)
Target: arm64-apple-macosx13.0

Operating system: MacOS 13.4.1

[czechboy0]

Ah yes, can you confirm what the style and explode values are for the header, if specified?

[bfrearson]

simple and explode I think. Was part of decoding a header response.

[czechboy0]

Was explode true or false? The default is false for simple (https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.1.0.md#fixed-fields-10), was it enabled manually?

[bfrearson]

Oh yes it was false!

[czechboy0]

Ok. The reason this is happening is because the value isn't percent-encoded, which is required by https://datatracker.ietf.org/doc/html/rfc6570#section-3.2.2, which is how OpenAPI serializes simple values: https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.1.0.md#style-values

However, this is a common mistake servers make, so I'm leaning towards the runtime library correctly encoding values (with percent-encoding), but being a little more flexible while decoding values.

A way to introduce this flexibility might be as simple as making URISingleValueDecodingContainer concatenate values again with a comma and return that. I'll have to play with that next week. This logic is getting improved further with apple/swift-openapi-runtime#50, which just landed, but I don't think it contains any changes that'd fix this case.

Here's a speculative fix, can you try if it helps, please? apple/swift-openapi-runtime#51

[czechboy0]

Changing the label as the runtime library is doing the correct thing according to RFC6570, I believe, so this is not a bug our side. Marking as enhancement instead, as it'd still be nice to be a bit more flexible on the input side.

[bfrearson]

Yeah that fix works, though I had to convert the array.joined into a substring to get it to compile:

let string = array.joined(separator: ",")
return Substring(stringLiteral: string) 

RE: Handling of headers

I may be getting the wrong idea, but shouldn't headers be treated differently to other encoded parameters? My (limited) understanding is that headers are always colon-delimited, as per RFC822 e.g.:

Content-Type: application/json
Content-Length: 1428

I'm not sure these should be handled in exactly the same way that we're handling URL form encoding? Apologies if I've misunderstood what's happening here!

[czechboy0]

You're right that the header line is colon-delimited, the colon is not meant to be percent-escaped, that only applies to the header value, aka the stuff on the right of the colon.

Most headers that support multiple values simply concatenate them using a comma. For example, let's say we have a header called Greeting-Options that provides an array of greetings for the server to choose from.

For the values Hello world! and Hi, the on-the-wire representation should look like this:

Greeting-Options: Hello%20world%21, Hi

And the OpenAPI definition should look like:

name: Greeting-Options
in: header
schema:
  type: array
  items:
    type: string

From your originating example, I infer you're using the Cache-Control header, but it seems to be documented as:

type: string

instead of:

type: array
items:
  type: string

Basically, the generated code is being told it's a single value, not an array, but the simple unexploded format uses commas to separate individual values.

So you have two options here:

1. Continue to treat the header value as a single string, but then the server must return no-cache%2C%20private instead of no-cache, private.
2. Change the definition of the header in the OpenAPI document to be an array of strings, instead of a single string. Then you'll get the two values separated by a comma parsed correctly.

I'd probably recommend (2), but wanted to explain both options here.

[bfrearson]

Ok thanks, that makes sense. I think this is probably actually an issue with my Openapi document then 😅

FWIW the document I'm using was converted from Postman docs. I guess it's a bug or oversight either with the converter I was using, or with the way Postman exports collections.

That said, I guess if this is a common use case and there is an easy fix, it would make the runtime more robust to handle admittedly badly-formed open api docs.