search

brycx / pasetors

PASETOrs: PASETO tokens in pure Rust
MIT License
83 stars 8 forks source link

Relax payload from_utf8 behind flag #98

Closed loehde closed 11 months ago

loehde commented 11 months ago

A minor suggestion

It would be nice if the payload could be relaxed to a non-valid utf-8 encoding, currently it restricts binary encodings. I know binary encoding, is not compliant with the PASETO RFC. As a suggestion, it could be hidden behind a feature flag to allow binary encodings and since the function signature for payload is &[u8], and a valid footer does not need a valid utf-8 encoding.

https://github.com/brycx/pasetors/blob/807d3ad3660158bac25be0f66768d84ee259323d/src/token.rs#L119C21-L119C21

brycx commented 11 months ago

Hej Kasper,

Thanks for the suggestion! As you also mention, non-valid UTF-8 data is prohibited by the spec. As the spec also suggests, if non-UTF-8 data is needed, this ought to be encoded using the base64-url encoding, which is also used throughout the rest of PASETO.

Is there anything preventing you from doing this? Encoding your binary data beforehand and then decoding it after having processed the token?

loehde commented 11 months ago

Hej Johannes,

Thank you for your fast response. There is nothing preventing me from using base64 encoding, except it introduces serialization/deserialization overhead and introduces space overhead, which is typical not an issue. But it would be nice to serialize it directly instead of introducing steps, since the function signature takes an &[u8] maybe it should be changed to a &str if it should stay complaint with PASETO? Since it introduces confusion?

It is delightful to hear your thoughts, and thank you for the awesome library, huge thumbs up!

//Kasper

brycx commented 11 months ago

"[..] takes an &[u8] maybe it should be changed to a &str if it should stay complaint with PASETO? Since it introduces confusion?"

Yes, this sounds like a good idea. I see how it is confusing, I don't want people necessarily forced to read the spec before using the library. Should be noted that this API confusion is only present in the lower-level versionN:: modules, since AFAIR the high-level interfaces for v4 should already enforce this UTF-8 input through the Claims type.

Doing this change would be SemVer breaking.

Thank you for the kind words! Glad you like it.