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

More useful trait #4

Open
immanuelhume opened this issue Aug 5, 2023 · 0 comments
Open

More useful trait #4

immanuelhume opened this issue Aug 5, 2023 · 0 comments

Comments

@immanuelhume
Copy link
Contributor

immanuelhume commented Aug 5, 2023

It dawned upon me that we should aim to make the dissection-related trait more useful.

Currently, we form an internal model of field types in a "stringy" way: https://github.com/ghpr-asia/wsdf/blob/0fc4333dc6f783101567b46a484e12f2863bbdf3/wsdf-derive/src/types.rs#L140C1-L148C61

This is working well now, but brittle. A user may have type Port = u16. In which case we would wrongly read Port as a struct instead of a u16.

I am proposing a re-write of the main dissection trait such that it can be implemented directly on basic types u8, Vec<u8>, etc. We can then directly leverage the trait in the generated code, i.e. we no longer need to care about what the type of each field is. We just assume the trait is implemented and use the methods.

Progress will be in https://github.com/immanuelhume/wsdf/tree/proper-impls. As a initial cut, I have designed the trait like so:

trait Dissect<'tvb, MaybeBytes: ?Sized> {
    /// We would like to query the value of some fields, e.g. `u8`. If the type supports this
    /// querying, we set its `Emit` type. Otherwise, `Emit` can be set to `()`.
    type Emit;

    /// Adds the field to the protocol tree. Must return the new packet offset.
    fn add_to_tree(args: &DissectorArgs, fields: &mut FieldsStore) -> usize;

    /// Registers the field. It is the responsibility of the implementor to save the hf index
    /// and possibly the ett index into the two maps.
    fn register(
        args: RegisterArgs,
        hf_indices: &mut HashMap<String, c_int>,
        etts: &mut HashMap<String, c_int>,
    );

    /// Returns the value associated with the field, if any.
    fn emit(args: &DissectorArgs<'_, 'tvb>) -> Self::Emit;
}

trait Primitive<'tvb, T: ?Sized>: Dissect<'tvb, T> {
    /// Adds the field to the protocol tree using a custom string. Must return the new packet
    /// offset.
    fn add_to_tree_format_value(
        args: &DissectorArgs<'_, 'tvb>,
        s: &impl std::fmt::Display,
        nr_bytes: usize,
    ) -> usize;

    /// Saves the field into the fields store.
    fn save(args: &DissectorArgs<'_, 'tvb>, store: &mut FieldsStore<'tvb>);
}

We can avoid breaking changes by offering the new API alongside the old one. Once the new one is stable enough, we can remove the old Protocol and ProtocolField API. In any case, there should be minimal changes from the usage perspective as this changes addresses the underlying framework design.

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

No branches or pull requests

1 participant