v3.3: describe behaviour when deserializing a multipart with a Content-Type header#5391
v3.3: describe behaviour when deserializing a multipart with a Content-Type header#5391karenetheridge wants to merge 1 commit into
Conversation
bf6e746 to
7fef1ad
Compare
| When deserializing a multipart message, if a `Content-Type` header is present in the specific part, its value shall be used instead of these values. Behavior is undefined when both this header and `contentType` are defined but with different values. | ||
|
|
There was a problem hiding this comment.
@karenetheridge if I am reading this right, the only potential change here is in the case where contentType is absent and therefore assumed to have a default value, but the per-part Content-Type type contradicts that default value?
I think we can make this fit the compatibility guidelines more cleanly by switching the order of the sentences and changing "shall" to "SHOULD" (or "RECOMMENDED" which means the same thing but fit better in the sentence I'm proposing):
Something like this? But less awkward with the last part?
| When deserializing a multipart message, if a `Content-Type` header is present in the specific part, its value shall be used instead of these values. Behavior is undefined when both this header and `contentType` are defined but with different values. | |
| When deserializing a multipart message, behavior is undefined when the `Content-Type` header and the `contentType` field have different values. If the `contentType` field is absent but `Content-Type` is present, it is RECOMMENDED to use the value of `Content-Type`. However, this behavior is still technically undefined due to the possibility that the `schema` is not structured for the received `Content-Type`. |
There was a problem hiding this comment.
The only potential change here is in the case where contentType is absent and therefore assumed to have a default value, but the per-part Content-Type type contradicts that default value?
No, I am not presuming that the default value would be considered here at all -- I think the Content-Type header in the message part should be used in preference to that: a value being underspecified means that we do the best with whatever information we do have, and in this case we have a header telling us what type the message sender intends it to be.
Your edit sounds fine, except I would remove the last sentence: the schema may be just fine with the Content-Type (consider the case where encoding/contentType says "text/plain; charset=UTF-8" but the Content-Type header says "text/plain; charset=Shift_JIS"... or application/yaml instead of application/json).
The logic I'm using is:
- if there is style/explode/allowReserved present, use those;
- otherwise, check for encoding/contentType AND a Content-Type header:
- if we have just one or the other, use it;
- else if we have both, they must match, else generate an error, but still decode with the Content-Type header (because we can presume the message sender knows what they're doing);
- else (we have neither): do we have a schema for this data location?
- yes, examine the schema to see what type this should be, and select the media-type on that basis; if it's "text/plain", also apply the charset from any adjacent
_charset_message part. - no: do nothing; leave the payload as a string (and now we start unwinding our recursion as there is nothing more we can do with this part of the messge).
- yes, examine the schema to see what type this should be, and select the media-type on that basis; if it's "text/plain", also apply the charset from any adjacent
However, while looking at my test cases a little more closely, I'm wondering if it might be more sensible to simply prefer Content-Type over encoding/contentType without erroring at all when the values conflict. contentType is useful as a recipe for serializing a message, and a hint for how to deserialize when insufficient information is present in the message itself, but when Content-Type is right there in the message next to the data, surely we should just go ahead and use it? That's what an application would do in the absence of an OAD telling them what the message should look like. If the data decodes to the desired result, why should this be an error at all?
[...and after experimenting with tests, I am convinced this is correct.. it would not be useful to generate an error when we're still able to successfully deserialize the message. The schema is still there as a verification step to catch issues with the deserialized content, and there's a good chance that it came out correctly anyway even if the media-type didn't match what the OAD said the serialization process ought to be using. If a media-type was used that we don't know how to decode, then we can error on that instead.]
There was a problem hiding this comment.
(edited to fix some errors; read in web rather than in email)
There was a problem hiding this comment.
I'm wondering if it might be more sensible to simply prefer Content-Type over encoding/contentType without erroring at all when the values conflict.
It might be more sensible (in some cases I'm pretty sure it is more sensible, and it definitely fits more with the architecture of the web where you can have advance hints, but the runtime behavior is always what's real), but it's not the way the OAS is written (or more broadly, the way it is marketed as a way to define an API interface contract).
Our general paradigm is that if the runtime behavior doesn't match the OAD, then that's an error [EDIT: except I keep forgetting the "open world" approach, so maybe it's not? See below]. Whether it's an error in the API or the OAD depends on the circumstances, but the general idea is that you should be able to tell when your API and its design contract fail to match. In theory, whatever the OAD says that contradicts the Content-Type is an assertion that the application depends on, so if the OpenAPI tooling to ignores that mismatch, it is likely to cause problems downstream — precisely the sort of problem that a contract is supposed to prevent.
(note: I just like em-dashes and know how to use them. No AI was used on this comment 😛 )
This is a philosophical point that would be worth revisiting in a 4.0.
There was a problem hiding this comment.
What should we do about this? Should we add in 3.3 some stricter language that says that contentType (or one of the values, if comma-separated) MUST match the part's Content-Type, only to potentially remove it in 4.0? Or just leave it vague for now?
In my testing I also flagged another edge case: contentType says text/plain, but the Content-Type header says text/plain; charset=Shift_JIS. IMO this is not a mismatch; the header is simply being specific about the character encoding, and contentType is saying "any text/plain type will do".
There was a problem hiding this comment.
You're definitely correct about the text/plain not being a mismatch.
Regarding the rest of it... I have to apologize for shifting my views a bit again. I'm even frustrating myself with this, and it's a good bit of evidence for why we should really nail down the philosophy of the OAS more clearly (which various folks here have attempted to do over the years but we've never quite managed it).
This scenario is nearly analogous to matching (or not) the keys under the content field of a Response Object (as that one specifically allows media ranges). We don't consider failure to match there an error, because many things in OpenAPI take an "open world" view (which is what I was forgetting before): an OAD describes things that can happen, and if they happen, what they should look like. But other things can also happen, and are not necessarily an error. So if you get back application/xml but don't have a content key for that in the Response Object, it's not automatically an error. It's just a thing you have no information about.
Or at least, this is how it should work in an "open world" view. But we're not very clear about that being the view, I think.
It's not quite perfectly analogous because at the Response Object level the content keys can be related to the Accept request header, and there's no way to specify individual part media types in Accept: multipart/whatever AFAIK. And the server is allowed to disregard Accept anyway.
So if we look at contentType as analogous to the Response Object (or Request Body Object)'s content keys, I guess a failed match is not an error? It's just a thing we don't know much about?
I feel like I've gotten myself confused at this point 😵💫
There was a problem hiding this comment.
So if you get back application/xml but don't have a content key for that in the Response Object, it's not automatically an error. It's just a thing you have no information about.
Interesting, that hasn't been my interpretation -- in my implementation, the way to get past a "wrong Content-Type" error is to explicitly provide a media-type entry for "/" (which matches anything that didn't match anything else).
I also error if there is no defined decoding for the media-type that matched under content, unless the schema is empty ({} or true) or missing entirely -- in these cases the string value of the content is returned without alteration.
| ##### Handling Multiple `contentType` Values | ||
|
|
||
| When multiple values are provided for `contentType`, parsing remains straightforward as the part's actual `Content-Type` is included in the document. | ||
| When multiple values are provided for `contentType`, parsing remains straightforward as the part's actual `Content-Type` is included in the document; it SHOULD match one of the media types in `contentType`, if it is present. |
There was a problem hiding this comment.
I think that with the re-ordered sentences above, this is not necessary as it is simply a consequence of the other statement. It's not entirely clear to me that we can put a SHOULD on the API behavior, though. I think we can only put SHOULDs on our side of things. We should maybe look and see if we have other SHOULDs that impact API behavior rather than OAS tool behavior — If we already do, then I'm perfectly happy for us to add another.
There was a problem hiding this comment.
It's not entirely clear to me that we can put a SHOULD on the API behavior, though. I think we can only put SHOULDs on our side of things
I'm not sure what you mean? What is "our side"?
There was a problem hiding this comment.
but anyway, yes, I agree with removing this addition, as it is unneeded; of course we want the Content-Type header to match encoding/contentType, but if they don't there's nothing useful the deserializer can do about it now (as above, generating an error about this is not helpful assuming we were able to deserialize anyway).
(Also, I feel good that the original statement here is consistent with my assumption that the Content-Type header is the value we should prefer over anything else) :)
There was a problem hiding this comment.
"our side" -> OAS tooling, the stuff that implements our requirements
"not our side" -> includes API behavior, although honestly I'm not sure I agree with my earlier point given the overall perspective of an OAD-as-contract. There's a subtle distinction in that with OAD-as-contract it's the mismatch that causes the error, but the error could be in either the contract or the API. So technically the OAS (by way of the OAD) is not imposing a requirement on the API. It's documenting an expectation, which could be erroneous either because the API is wrong or the expectation is wrong.
As you can tell, I'm thinking through this as I'm typing, so don't take it as any sort of settled law here.
There was a problem hiding this comment.
I think there's definitely room for a philosophical discussion here as to what we consider our scope and what we expect the OAD to actually prescribe. I've been coming at things mostly from a "deserialize incoming requests and outgoing responses, and validate as much as we can" perspective, as that aligned with my previous JSON Schema work, but there's a lot more than that (and once I exhaust myself with deserializing, I'll be pivoting to thinking more about serializing and client generation).. but other tools have done a lot more various things with the OAD and perhaps made some wildly different interpretations on some points.
There was a problem hiding this comment.
one more thought: I do think we should call out the mismatch, but maybe an actual error (which in my tooling would cause a request to be rejected with an HTTP 400 error) might be too severe? It's an interesting distinction between "we were able to deserialize this message completely using the data provided in the message, AND its deserialized form validated against the schema" and "something about the message's structure didn't match what the OAD said it should do".
This is not really a breaking change, since I think what's described here is intuitively the right thing to do anyway, but as it's a new directive I'd still only add it in 3.3 and not backport it to 3.2.1.