Skip empty query parameters

[thomaseizinger]

Currently, a key without a value fails to deserialize, even if the field is wrapped in an Option. Semantically, no key and a key with an empty value is the same (I think?). This patch filters keys without values before the deserialization starts.

This allows us to deserialize things like value= if the value is an Option. If value is not an option, it will fails as expected that it is missing.

Let me know what you think.

I stumbled across this when implementing a form that has optional number fields. It would fail to deserialize with "cannot parse float from an empty string".

[jplatte]

I don't agree that empty values should be treated as None or skipped when deserializing a Vec of values, sorry. This would also break deserialization of non-optional String fields that are sometimes empty.

[thomaseizinger]

I don't agree that empty values should be treated as None or skipped when deserializing a Vec of values, sorry.

Okay, how would you handle the case of an optional number field then? Currently, deserialization of my form (in axum) fails if the field is empty. I'd have to make it a Option<String> and then manually parse as f64 because I'd get Some("") then.

This would also break deserialization of non-optional String fields that are sometimes empty.

Does it? You can just put #[serde(default)] on it, right?

I tried to come up with a case where an absent parameter (no key & value) is semantically different from just the key being present but I couldn't think of anything. Perhaps boolean values like "path/path/path?foo" but I am not even sure whether that deserializes today!

[jplatte]

I don't agree that empty values should be treated as None or skipped when deserializing a Vec of values, sorry.

Okay, how would you handle the case of an optional number field then? Currently, deserialization of my form (in axum) fails if the field is empty. I'd have to make it a Option<String> and then manually parse as f64 because I'd get Some("") then.

You can use a custom deserialization function for that, like this one.

This would also break deserialization of non-optional String fields that are sometimes empty.

Does it? You can just put #[serde(default)] on it, right?

Yes, it wouldn't make it impossible to let people work around it but I would definitely call it a bug if this didn't work by default.

[thomaseizinger]

I don't agree that empty values should be treated as None or skipped when deserializing a Vec of values, sorry.

Okay, how would you handle the case of an optional number field then? Currently, deserialization of my form (in axum) fails if the field is empty. I'd have to make it a Option<String> and then manually parse as f64 because I'd get Some("") then.

You can use a custom deserialization function for that, like this one.

Thanks, will try that!

This would also break deserialization of non-optional String fields that are sometimes empty.

Does it? You can just put #[serde(default)] on it, right?

Yes, it wouldn't make it impossible to let people work around it but I would definitely call it a bug if this didn't work by default.

Fair. Would you accept a PR that fixes the bug I am experiencing without this downside?

[jplatte]

Would you accept a PR that fixes the bug I am experiencing without this downside?

You would first have to convince me that it's a bug at all. My current stance is that this is working as it should be.

[thomaseizinger]

Would you accept a PR that fixes the bug I am experiencing without this downside?

You would first have to convince me that it's a bug at all. My current stance is that this is working as it should be.

Okay, this is my attempt 😁

• I have an HTML form with an optional input of type number
• My typed form in axum uses Option<f64> for this field
• Submitting the form without a value makes the browser send value=
• The request fails with "cannot deserialize f64 from an empty string"

I don't see how this is working as it should be. How am I meant to handle optional fields if not by wrapping them in Option?

[jplatte]

Alright, convinced 😄

How about just changing the new Option deserialization code from #3 to call visit_none if the value is empty? (although I wonder what will happen with Option<Vec<u8>> and value=&value=2 then)

[thomaseizinger]

Alright, convinced :smile:

Yey!

How about just changing the new Option deserialization code from #3 to call visit_none if the value is empty? (although I wonder what will happen with Option<Vec<u8>> and value=&value=2 then)

That code branch unfortunately never gets called. The problem is within the MapDeserializer where we have a key but the value is an empty string. I am not a serde expert but by the time we are within our code again, we no longer have knowledge of the Option I think.

[thomaseizinger]

False alarm, it does work actually!

[thomaseizinger]

I opened a new PR here: https://github.com/jplatte/serde_html_form/pull/5