header marshaling should be case-insensitive and more robust #1403
Conversation
ahl
requested review from
davepacheco and
hawkw
and removed request for
davepacheco
dropshot/src/extractor/header.rs
Outdated
| /// OpenAPI document output. Case-insensitive name conflicts may lead to | ||
| /// unexpected behavior, and should be avoided. For example, only one of the |
There was a problem hiding this comment.
Trying to make sense of "Case-insensitive name conflicts may lead to unexpected behavior, and should be avoided".
Who is this advice for? Why not just say "Since HTTP headers are case-insensitive, we automatically treat headers as lowercase. Upper-case characters are still allowed for OpenAPI documents, but they are converted to lowercase when deserializing and dealing with extraction conflicts".
There was a problem hiding this comment.
I improved the phrasing; thanks.
There was a problem hiding this comment.
I felt similarly about that line but couldn't figure how to state the problem
dropshot/src/from_map.rs
Outdated
| fn rename(fields: &'static [&'static str]) -> BTreeMap<String, String> { | ||
| fields | ||
| .iter() | ||
| .map(|field| (field.to_lowercase(), field.to_string())) |
There was a problem hiding this comment.
Is the lifetime too much of a pain to return the "value" here as a &'static str? seems unnecessary to perform the string allocation (via field.to_string()) on the case-unchanged string here
Fixes #1380, #1373.
The
Headerextractor (which I wrote) was very poor. In particular it didn't consider the case of required header (or of deserialization failing for any reason). This PR intends to address the issues of robustness and add proper handling for the case-insensitivity of headers. We do so while preserving desired casing for the OpenAPI output.An alternative approach would have been to require lower-case naming for headers with the ability to use
schemarsattributes to direct the OpenAPI output (e.g.#[serde(rename = "x-my-header")] #[schemars(rename = "X-My-Header")]). But this seemed confusing and error-prone.