Skip to content

Conversation

@JonasKruckenberg
Copy link

@JonasKruckenberg JonasKruckenberg commented Dec 10, 2025

This change introduces the RedisKey::to_raw_parts method as a counterpart to RedisKey::from_raw_parts. We need to be able to access the inner pointer for more low-level operations. It also marks from_raw_parts as unsafe adding documentation to both.

Additionally this fixes an unsoundness bug in in KeyCursor that was leading to possible use-after free of the key name provided to the scan callback.

@JonasKruckenberg JonasKruckenberg force-pushed the push-qowsolyxxttn branch 4 times, most recently from 465a889 to 9d22033 Compare December 10, 2025 09:51
This change introduces the `to_raw_parts` method as a counterpart to `from_raw_parts`. It also marks `from_raw_parts` as unsafe adding documentation to both.

Additionally this fixes an unsoundness bug in in `KeyCursor` that was leading to possible use-after free of the key name provided to the scan callback.
@JonasKruckenberg JonasKruckenberg changed the title fix: add to_raw_parts method. fix: add to_raw_parts method & fix unsoundness bug Dec 10, 2025
Copy link

@GlenDC GlenDC left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

Only thing that concerns me, but outside scope of this PR I suppose, is the lack of sanity checks to at least have some guarnatees for these safety checks.

I also wonder if it should not be better to have a borrow type specific for these purposes which doesn't do any dropping at all so that you cannot even forget to disable the destructors for cases like this...

@JonasKruckenberg
Copy link
Author

Only thing that concerns me, but outside scope of this PR I suppose, is the lack of sanity checks to at least have some guarnatees for these safety checks.

I also wonder if it should not be better to have a borrow type specific for these purposes which doesn't do any dropping at all so that you cannot even forget to disable the destructors for cases like this...

agreed, but squarely outside the scope of this PR

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

Successfully merging this pull request may close these issues.

3 participants