Continued interledger-packet fuzzability #720
Conversation
| .unwrap(); | ||
|
|
||
| if roundtripped != read_expires_at { | ||
| return Err(ParseError::InvalidPacket( |
There was a problem hiding this comment.
Question:
Not sure if we need to align error types / have roundtrip error. I used a different type in the stream crate.
There was a problem hiding this comment.
At the moment all of the code I've seen either depends on:
- just any error value (or "non-ok") coming from methods which return
Result<T, ParseError> - the string formatting (the tests, maybe some logs)
So I'd be inclined not to change this now but leave it for #708, since it's pretty much the same -- both will need that static string as allocated one which I'd very much prefer not to have!
It's a good point however. Perhaps we should just go ahead and add a #[cfg(feature = "roundtrip-only")] variant there, and maybe make the whole thing #[non_exhaustive]. But I'm hoping the #708 will end up with more than one single massive error type.
|
Oh yeah, new stable rust! I'll do a separate PR for the clippy errors if possible. |
|
I accidentially dropped the last commit once as I failed to understand what did this mean: Maybe that error message should say that there are new commits or something :) Last commit is now restored. |
Signed-off-by: Joonas Koivunen <joonas.koivunen@gmail.com>
by temporarily bringing back the roundtrip check. Signed-off-by: Joonas Koivunen <joonas.koivunen@gmail.com>
do this by moving the check from fuzz_target over to src/lib.rs. Signed-off-by: Joonas Koivunen <joonas.koivunen@gmail.com>
not only the fulfill. this does allow a big simplification to the lenient_packet_roundtrips, which can now check that the packet bytesmut containers are equal instead of the per-component and hunting down the trailing bytes position. Signed-off-by: Joonas Koivunen <joonas.koivunen@gmail.com>
this allows it to be used in assert_eq for more uniform view into the data. Signed-off-by: Joonas Koivunen <joonas.koivunen@gmail.com>
that is, on "strict" reject the inner *and* outer bytes, similar to how *outer* were already rejected when "strict". for fulfill this changes how inner trailing bytes were always rejected back to rejecting when "strict". refactor out the "outer" bytes check as well, to have the "trailing bytes" functionality together for more visibility into how this is done consistently. following the refactor, messages are also unified with inner/outer. this follows change from trying to keep the "strict" as adhering as strict as possible to the RFCs, and perhaps eventually enabling it by default. Signed-off-by: Joonas Koivunen <joonas.koivunen@gmail.com>
- prefer to_string over format! - reorder added assert_eq args - prefer cfg! macro with if/else Signed-off-by: Joonas Koivunen <joonas.koivunen@gmail.com>
this follows from finding a case where the parsed timestamp doesn't match the output timestamp, even with the fix made in #707. the strict feature is included in the "rountrip-only". Signed-off-by: Joonas Koivunen <joonas.koivunen@gmail.com>
Signed-off-by: Joonas Koivunen <joonas.koivunen@gmail.com>
fuzzed_0, fuzzed_1 instead of splitting the fuzzed_0 into two. Co-authored-by: bwty <email-me@belsy.space> Signed-off-by: Joonas Koivunen <joonas.koivunen@gmail.com>
Cc: #705
This is a follow-up to #716 with:
interledger-packet/roundtrip-onlyfeature for denying ILP timestamps which don't roundtrip 1interledger-packet/strictfor Prepare and Reject (continues disallow extra trailing bytes in Fulfillment packet #710 which did this for Fulfill) 2.to_string()calls,if cfg!(...) { } else { })src/lib.rswhere it can be used with tests as wellPartialEqderivation forinterledger-packet::hex::HexStringfor use inassert_eq!(HexString(...), HexString(...));Footnotes
Unsure if this should be the default; it'd seem that the
60as the%Sfield is accepted for every date, which might not match the idea in the RFCs. "Every date" as in best to my knowledge, 2000-05-31 was not any special day w.r.t. leap seconds. ↩Changes include string changes for consistent "Unexpected {inner,outer} trailing bytes", where outer means outside the interledger-packet envelope and inner means inside the envelope. ↩