Ergonomics of the tls_serialize module?

[roy-work]

All of the functions in the tls_serialize module require passing a (&mut [u8], usize).

There a couple of issues:

1. I'm not entirely sure what the usize is. If it's the size of the buffer, why not use the slice's length?

2. I'm not sure how large of a buffer to pass in. I'd like to avoid arbitrary lengths/sizes in my own code, and I'd like to avoid innate knowledge about TLS's serialization layer, as that's really the responsibility of the library to know this. Ideally, I'd like to pass in a Write, or have the return be a Result<Vec<u8>, _>; I don't want to have to know, a priori, how to calculate a buffer of the correct length.

(Looking at the source code, it seems like a lot of this is pass-through to cookie_factory, and I'm not sure how to correct it. But it seems odd to me that a serialization library like that would require or use function signatures like this, but perhaps that is the case here?)

[chifflier]

TBH, this code waas mostly written as a proof-of-concept, and is not entirely satisfying.
It relies on cookie-factory, with the old API. At some point I want to upgrade it, which would also solve the strange API with the usize that you mention. I just had no time, and the serialize part was not the priority of this crate - that may change in the future.
Originally, this usize was used to differentiate the allocated size while knowing the current write offset, and be able to write at another location (like updating the size field of a structure). The newer API of cookie-factory is better, regarding this.

If that is of some help, I have written a toy example of using the parse and serialize features of tls-parser here (look at src/main.rs).

[chifflier]

I had a second look, and took the time to upgrade the API in 5439ac1

It is not easier to use the serialization function, you can now just pass Vec::new() to gen_simple (see the unit tests for many examples), there is no need to preallocate data (except to optimize and avoid Vec reallocations).

I'm considering this closed since the changes are committed - however the serialize feature is still incomplete and does not cover all messages and extensions. This will be added in the future.

[roy-work]

The update looks much better! A Write is much closer to something that I as a consumer can pass.

I'm still struggling with it, though. My expectation is still to be able to pass something like a Write, e.g., a std::io::Cursor<Vec<u8>>, and have it serialize the given TLS struct/enum into that.

The functions are still a bit odd, since they return a closure that does the serialization. Why the two-step, and not just a gen_foo<W: Write>(w: W, data: TLSSomething)?

But also because the returned Fn has a lot of trait bounds on the Write arg:

error[E0277]: the trait bound `&mut std::io::Cursor<std::vec::Vec<u8>>: std::default::Default` is not satisfied
   --> …/src/lib.rs:150:66
    |
150 |     println!("{:#?}", tls_parser::gen_tls_extensions(&ext_stuff)(write_context));
    |                                                                  ^^^^^^^^^^^^^ the trait `std::default::Default` is not implemented for `&mut std::io::Cursor<std::vec::Vec<u8>>`
    |
   ::: /…/.cargo/git/checkouts/tls-parser-a8479c5b4ed93857/eac5fe4/src/tls_serialize.rs:143:16
    |
143 |     W: Write + Default + AsRef<[u8]> + AsMut<[u8]> + 'a,
    |                ------- required by this bound in `tls_parser::tls_serialize::gen_tls_extensions`
    |
    = help: the following implementations were found:
              <std::io::Cursor<T> as std::default::Default>

What Write satisfies Default? AsRef/AsMut<[u8]>? I'm not sure if there exists a Write that I can pass here, or what it expects from those trait bounds?

Edit: it seems Vec<u8> impls Write. But I still don't understand why most of these trait bounds should be required, so I think the question stands regardless. A generic Write should suffice, no?

Edit edit: also, Vec<u8> in the new API does work for my current use case. I'd love to get it released, but if you want to consider the arguments above (which would be breaking changes) and change something and only do one breaking release, I understand that too.

[chifflier]

Thanks for your feedback!

Here are a few answers/comments:

• cookie-factory asks for Write, and is usually used with Vec or a slice
• The Default, AsRef and AsMut traits are required because of functions calculating the length using a temporary object (like length_be_u16). That was the solution found at that moment, I will have a second look and try to remove these restrictions, but this is non-trivial.
• The combinators all return closures, that is required to be able to combine them. However, the intention is to wrap that behind a trait and have for ex a method TlsMessage::to_vec()
• The unit tests are examples of how to use the serialization functions

[chifflier]

Update: I got more or less 2 solutions for the serialization functions, for ex those requiring to create something like [length (2 bytes) | buffer].

1. Construction of the buffer in one pass (quite similar to the existing). This requires the BackToTheBuffer trait, which adds Seek. Practically, this means using only Vec<u8> of &mut [u8] (not really different from what is actually in the code)
2. Using a temporary buffer to format data, then write length + buffer. This removes almost all requirements (the function must be able to work on a buffer though), but creates temporary copies of data.

The API is the same in both cases. Right now I don't have any strong opinion on which one is the best.

[chifflier]

The "tmporary buffers" approach (2) is easier to use, so I implemented it in ad1eed8 so more tests can be done.