-
Notifications
You must be signed in to change notification settings - Fork 474
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
turning on serde_json
's arbitrary_precision
feature is infectious to importers
#2970
Comments
Hi @robjtede The Given a JSON response body like this: {
"integer": 1,
"float": 1.0,
"small_float1": 0.1,
"small_float2": 0.100000000000000005,
"big_float1": 1000000000000000000000.0,
"big_float2": 1000000000000000000000.5,
"big_integer": 1000000000000000000000
} I want to be able to test this response correctly with this Hurl file:
(you can see our integration test here https://github.com/Orange-OpenSource/hurl/blob/4a67bd5cb00c5bc942ac330138c0179e19191359/integration/hurl/tests_ok/predicates_number.hurl) Because JSON body parsing is done with serde, we've activated serde's flag Regarding the binary compatibility break, we try to be better but it's still not perfect. I've noted that we should check and upgrade version when we activate dependencies flag. |
You could potentially make Our situation is that we want to use Hurl to test an API that uses an HMAC authentication scheme. To do this, we use Hurl as a library so we can compute and inject the MAC header in each hurl entry (request). |
@RapidPencil I understand your point but I'm really really reluctant to do this. I'm not at all happy with having a different Hurl's output for a given input. Our priority is the command line and I don't want to expose/support the crate as a liability for us. We need to discuss it with the other maintainer @fabricereix @lepapareil and see if there are others alternatives. |
@robjtede @RapidPencil we have looked around this issue (we still want to have a minimal reproductible sample if you have something like that it would be cool). If we want to not activate
That implies:
The bad news is that it is not a small task. I'm particularly worried on regression on JSON parsing . The good news is that we already have a JSON parser: we use it for request body because we're supporting "JSON-like" body with Hurl templates:
So we can use our own parser everywhere, provided:
Any bug we'll identify will be a bug that we could have for request body so this is rather virtuous. We can begin work on it, for a target version of 5.1.0 (5.0.0 has already a lot of changes and we don't want to delay it). This may lands in october/november given out track records of previous version. Any idea, remarks, suggestions? |
Appreciate the look into it. For sure this is non-trivial to solve and I understand that lib consumers aren't your primary use case. Really the problem lies in serde_json and it'd be far too much to ask you folks to implement a whole separate parser. That said, I think we can work around it for now with some cargo patching. |
What is the current bug behavior?
It's a well known rough edge that
serde_json
's arbitary precision feature changes the behavior for all users of the crate; affecting uses of 3rd party data types the most.In our projects, we've gone to great lengths to avoid turning this feature on because it breaks some (or all) of our integration test code, instead opting for
rust_decimal
in all numeric cases which has support for using custom deserializers on each field to precisely control when string vs number types are expected in numeric fields.While trying to upgrade our wrapper library from
hurl
/hurl_core
v4.1 -> v4.3 (forisIsoDate
support) we noticed that CI was failing in a non-obvious way (which isSteps to reproduce
For a project with
rust_decimal
(withserde-with-float
feature to make the custom deserializer available, it's not important for this bug), using this snippet:... if we were to introduce
hurl
v4.3 dependency this test would spontaneously fail, without changes.What is the expected correct behavior?
hurl
as a library without being forced to use thearbitrary_precision
feature onserde_json
.Execution context
hurl
hurl
Version: 4.3.0Possible fixes
Not sure yet.
It's possible that an optional crate feature could solve this, which is always used by the CLI binary but gives library importers control over their context. Though, I haven't yet scanned the codebase for exactly how or why this feature is being used, so it may be non-trivial to do this.
The text was updated successfully, but these errors were encountered: