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

Remove the unsound SerializedDatabase::new function #4110

Merged
merged 1 commit into from
Jul 9, 2024

Conversation

weiznich
Copy link
Member

@weiznich weiznich commented Jul 9, 2024

This commit removes the unsound SerializedDatabase::new function from the stable API. Semver explicitly allows breaking changes for critical bug fixes and I would classify fixing soundness bugs as critical so this should be fine to do.

Practical speaking I wouldn't expect anyone to use that function anyway as the only reasonable use of SerializedDatabase is to be produced bye the SqliteConnection::serialize_database_to_buffer function. The corresponding
SqliteConnection::deserialize_readonly_database_from_buffer function accepts a &[u8] buffer that can be allocated on the rust side. It looks like we just "missed" to mark this function as not-public.

Fixes #4108

This commit removes the unsound `SerializedDatabase::new` function from
the stable API. Semver explicitly allows breaking changes for critical
bug fixes and I would classify fixing soundness bugs as critical so this
should be fine to do.

Practical speaking I wouldn't expect anyone to use that function anyway
as the only reasonable use of `SerializedDatabase` is to be produced bye
the `SqliteConnection::serialize_database_to_buffer` function. The
corresponding
`SqliteConnection::deserialize_readonly_database_from_buffer` function
accepts a `&[u8]` buffer that can be allocated on the rust side. It
looks like we just "missed" to mark this function as not-public.

Fixes diesel-rs#4108
@weiznich weiznich added the maybe backport Maybe backport this pr to the latest diesel release label Jul 9, 2024
@weiznich weiznich requested review from a team July 9, 2024 13:12
@pksunkara pksunkara enabled auto-merge July 9, 2024 13:31
@pksunkara pksunkara added this pull request to the merge queue Jul 9, 2024
Merged via the queue into diesel-rs:master with commit bcbb879 Jul 9, 2024
50 checks passed
weiznich pushed a commit to weiznich/diesel that referenced this pull request Jul 16, 2024
Remove the unsound `SerializedDatabase::new` function
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maybe backport Maybe backport this pr to the latest diesel release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unsoundness with diesel::sqlite::SerializedDatabase::new()
2 participants