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

Edge case where RON fails but all other serde format types succeed #152

Closed
jaynus opened this issue Mar 15, 2019 · 8 comments
Closed

Edge case where RON fails but all other serde format types succeed #152

jaynus opened this issue Mar 15, 2019 · 8 comments

Comments

@jaynus
Copy link

jaynus commented Mar 15, 2019

Using any format of bitflags serialization, I seem to have found, somehow, a way in which RON fails to deserialize values, but all other serde serializers seem to handle just fine. I don't understand serde enough to understand what is incorrect about the RON implementation.

Error is: thread 'main' panicked at 'called Result::unwrap() on an Err value: Message("invalid type: string "One", expected a borrowed string")', src/libcore/result.rs:997:5

Specifically, I could at least trace the error down to this line: https://github.com/kvark/bitflags-serial/blob/master/src/lib.rs#L66
or this line
https://github.com/H2CO3/option_set/blob/master/src/lib.rs#L242

I've created a repo minimally showing the issue here: https://github.com/jaynus/ron_fail_poc

I ran into this using RON originally with bitflags_serial (https://github.com/kvark/bitflags-serial). I then swapped to option_set in an attempt to fix it, and ran into the same issue.

@kvark
Copy link
Collaborator

kvark commented Mar 15, 2019

Any idea how does this differ from a general case that is expected to work with RON?

@jaynus
Copy link
Author

jaynus commented Mar 15, 2019

How do you mean exactly? I've updated the test case showing working cases vs. non-working cases with bitflags vs. option_set. The update shows that bitflags works for both ron and serde_json, while option_set works with serde_json but NOT ron.

I'm not sure what details you need.

Update here:
jaynus/ron_fail_poc@fe5bb3d

@kvark
Copy link
Collaborator

kvark commented Mar 15, 2019

Hm weird. I mean, the bitflags-serial was specifically implemented with Ron in mind and tested on it, see bitflags/bitflags#147
So the fact it no longer works means there was a regression in one of the pieces.

@jaynus
Copy link
Author

jaynus commented Mar 15, 2019

Oh! I totally failed to connect the dots that that was you! I've added bitflags_serial to the test case.

I've added a test case for bitflags_serial, which also appears to fail for JSON and RON.

jaynus/ron_fail_poc@faa4ce5

Test Output Is:
! RON option_set FAILED!
! JSON bitflags_serial FAILED!
! RON bitflags_serial FAILED!

@jaynus
Copy link
Author

jaynus commented Mar 15, 2019

As an additional, I've noticed there are cases where it DOES work with amethyst assets RonFormat loader, though.

So it appears maybe its a difference between from_str and from_bytes? I cannot replicate from_bytes working though in my case.

@jaynus
Copy link
Author

jaynus commented Mar 15, 2019

Update: I've also confirmed it fails with stable (I am using latest nightly).

@jaynus
Copy link
Author

jaynus commented Jun 12, 2019

As referenced in that PR to bitflags-serial, I'm not sure if this is a bug in bitflags-serial or RON. it appears to be because of the call to the unimplemented visit_str, instead of the originally implemented visit_bytes. But I don't know serde or RON well enough though to say why this is the case.

juntyr added a commit to juntyr/ron that referenced this issue Dec 25, 2021
juntyr added a commit to juntyr/ron that referenced this issue Dec 25, 2021
juntyr added a commit to juntyr/ron that referenced this issue Dec 28, 2021
kvark pushed a commit that referenced this issue Dec 30, 2021
@juntyr
Copy link
Member

juntyr commented Jan 1, 2022

Fixed with #352

@juntyr juntyr closed this as completed Jan 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants