Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adjacently tagged enums don't work in formats not supporting string values #2881

Open
inetic opened this issue Jan 14, 2025 · 3 comments
Open

Comments

@inetic
Copy link

inetic commented Jan 14, 2025

I'm using the bencode format which doesn't differentiate between strings and bytes.

In particular I'm working with serde-bencode which currently has a test ser_de_flattened_adjacently_tagged_enum ignored due of this issue.

The test fails with the error InvalidType("Invalid Type: byte array (expected: string or map)") coming from serde/ContentDeserializer::deserialize_enum.

Before returning the error, the self.content variable contains Content::ByteBuf(b"Request") instead of a string.

Applying the below patch in that function seems to resolve the issue, but I'm not too familiar with the serde code so please let me know if you think this is or isn't an adequate solution (full diff here).

-                s @ Content::String(_) | s @ Content::Str(_) => (s, None),
+                s @ Content::String(_)
+               | s @ Content::Str(_)
+               | s @ Content::ByteBuf(_)
+               | s @ Content::Bytes(_) => (s, None),

For posterity, adjacently tagged enums worked OK in serde-bencode with serde v1.0.180 but broke with >=v1.0.181.

@Mingun
Copy link
Contributor

Mingun commented Jan 14, 2025

Could you check if #2811 solves this problem?

@inetic
Copy link
Author

inetic commented Jan 15, 2025

Could you check if #2811 solves this problem?

Sorry to say, but no. The same serde-bencode tests ([1], [2]) fail with your branch as with the upstream with a slightly different error message.

@Mingun
Copy link
Contributor

Mingun commented Jan 15, 2025

It seems that the thing that broke serde-bencode is #2496 and #2505. This change broke several protocols

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants