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

Consider replacing safe-encode feature by unsafe APIs #31

Open
jorgecarleitao opened this issue Nov 10, 2021 · 3 comments
Open

Consider replacing safe-encode feature by unsafe APIs #31

jorgecarleitao opened this issue Nov 10, 2021 · 3 comments

Comments

@jorgecarleitao
Copy link

Used with lz4_flex = { version = "0.9.0", default-features = false }, we currently violate Rust's premise that code without unsafe must not result in UB/unsound code.

Specifically, it is surprising that I lose soundness guarantees by changing from

lz4_flex = { version = "0.9.0" }

to

lz4_flex = { version = "0.9.0", default-features = false }

since it is quite difficult to detect that such a change has soundness implications.

One mechanism to avoid this issue is to offer two APIs: an unsafe that does not perform checks, and a safe one, that performs the checks (instead of a feature-based safety checking).

@PSeitz
Copy link
Owner

PSeitz commented Dec 3, 2021

Sorry for the late reply.

I agree that two different API has some advantages for soundness and documentation.
Initially I kept them together to minimize the code size and add checks without runtime overhead, but I think two APIs may be better. One serious downside is, that it is a breaking API change.

@PSeitz
Copy link
Owner

PSeitz commented Feb 12, 2023

I think I will go forward with this by inverting the feature flags.

e.g. unsafe-decode and unsafe-encode will not be default features and can be enabled to get more performance. Those will not cause UB on any input.

Additionally there will be unchecked-decode which will be unsound. It can be used on trusted sources or when targeting wasm.

That means default-features = false will still result in code that uses no unsafe.
It will require deliberate action to enable the features that use unsafe.

@PSeitz
Copy link
Owner

PSeitz commented Jun 18, 2023

With version 0.11 I inverted the feature flag from checked-decode to unchecked-decode. So the API will be sound with:

lz4_flex = { version = "0.11.0", default-features = false }

I didn't invert the other feature flags as I want to minimize the API breakage.

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

2 participants