search

instructure / paseto

A paseto implementation in rust.
MIT License
151 stars 14 forks source link

Convert parameter passing from value to reference for token procedures #15

Closed akesling closed 4 years ago

akesling commented 4 years ago

Motivation

Switching from passing String to passing &str does a few things, namely:

1) denoting that ownership is not being passed into the procedure, making the relationship to the calling code more readable. 1) avoiding unnecessary clones. The provided procedures only read the messages and keys in order to construct the token. 1) cleaning up many calling sites. Taking &str allows callers to pass through a &str from another call (without having to call .to_string()) or use 'static string references (just take a look at how many of the tests were cleaned up of superfluous String::from()).

Test Plan

Existing tests have been updated. As this is mostly a parameter type change, existing tests should be sufficient. Since Rust validates parameter lifetimes for use, the switch will be safe.

Mythra commented 4 years ago

Wow, thank you doing this work! It has been something I have been meaning to do for a bit, but haven't gotten around too.

This all looks fine to me, but can we get a changelog update as well? Thanks!

akesling commented 4 years ago

Oh, apologies. I'll go right ahead and do that. I'm unsure the etiquette, but should I also update the version in Cargo.toml? Does this project use semver? If so, this is a backwards-incompatible interface change and we should probably bump it to 2.0.

Mythra commented 4 years ago

We'll want to keep the version the same for now. I am planning on doing a release, however I've been working on getting no_std support in, and I'd like that included (and tested) before cutting 2.0. However, yes we'll want to bump into the 2.0 range.

Mythra commented 4 years ago

Thanks again for this PR!

Mythra commented 4 years ago

Hey @akesling ,

We've bumped to v1.0.7, since no_std support was taking awhile. We bumped to v1.0.7 specifically because we've in the past preferred the major and minor numbers to be synchronized with the upstream paseto example, while we reserve the patch version to ourselves. So I decided to stick with what we did do previously.