avitex / rust-dangerous

Rust library for safely and explicitly parsing untrusted data
MIT License
51 stars 4 forks source link

Input trait #28

Closed avitex closed 3 years ago

avitex commented 3 years ago

Make input as trait, removing the need for a string flag

avitex commented 3 years ago

Thought I would cc you @Byron and @crutchcorn to show you some of the changes I have in mind.

In summary there will be a split between BytesReader and StringReader (both just type aliases of Reader with their corresponding types). For your use case you don't need to be able to retry input so that can be disabled. I've yet to add functionality to StringReader. BytesReader is functionally the same. This split disambiguates semantics and optimizations between parsing bytes and/or string input. It would be possible to make String and Byte readers separate features if only one specific Reader type was required.

As far as the retry mechanics go, the library prevents a couple of bad uses that would end in dead-locks when retrying. The semantics behind bound input is now stable but more documentation is required.

I'll be merging this into master soon once I've finished the TODOs. Would like to know any concerns.

On a side-note @Byron I'm feeling more confident in rust-zc however I'm going to be requesting an additional review from someone that touches that domain more than I. There is an example in master of how that library can be used (see zerocopy.rs).

avitex commented 3 years ago

Will roll more coverage and better benches in with more string functionality

Byron commented 3 years ago

Thanks a lot for keeping me in the loop! By the looks of it, I will soon get to finally implement the parser 😁 (which technically isn't the longest delay I have had in a project, knowing the gitoxide itself laid bare for 2 years before I finally picked it up again). Here are my thoughts.

It would be possible to make String and Byte readers separate features if only one specific Reader type was required.

I use features only if they help avoiding additional dependencies, as the added complexity is usually not worth it if only a couple of lines of crate code are excluded otherwise.

Would like to know any concerns.

That's hard to tell without diving in - the latter will probably happen when the merge is done. Once I use it myself I will gladly share my experiences or downright ask for a review here :). I will definitely give rust-zc as shot before going down the rabbit hole with ranges as it certainly may make everything a little simpler, and seeing an example on how it interacts with the parser and all the inbound changes would certainly help and the existing example will certainly help a lot! I still wonder how rust-zc would relate to the parsing of structures with fields which are then stored in some sort of Token.