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

Rename unsafe Artifact deserialize functions #3727

Closed
theduke opened this issue Mar 31, 2023 · 2 comments · Fixed by #3894
Closed

Rename unsafe Artifact deserialize functions #3727

theduke opened this issue Mar 31, 2023 · 2 comments · Fixed by #3894
Assignees
Labels
🎉 enhancement New feature! 📦 lib-engine About wasmer-engine priority-medium Medium priority issue
Milestone

Comments

@theduke
Copy link
Contributor

theduke commented Mar 31, 2023

The unsafe methods and functions for deserializing rkyv serialized artifacts should be renamed to more clearly express their dangerous nature, especially since we added deserialize_checked methods recently.

Preferred name: dangerous_deserialize_unchecked().

See #3693 for related discussion.

@theduke theduke added the 🎉 enhancement New feature! label Mar 31, 2023
@theduke theduke added this to the v4.0 milestone Mar 31, 2023
@Michael-F-Bryan Michael-F-Bryan added 📦 lib-engine About wasmer-engine priority-medium Medium priority issue labels Apr 11, 2023
@Michael-F-Bryan
Copy link
Contributor

We agreed to make this change when we do the next breaking change.

@theduke
Copy link
Contributor Author

theduke commented May 17, 2023

Also need to make the checked functions unsafe again, because loading executable code is inherently unsafe.

theduke added a commit that referenced this issue May 22, 2023
Rework the deserialize/deserialize_checked methods for module
deserialization.

* Make deserialize() methods use artifact validation
* Make checked methods unsafe again, because loading executable memory
  is inherently unsafe
* Rename methods that skip artifact validation to deserialize_unchecked

Closes #3727
theduke added a commit that referenced this issue May 22, 2023
Rework the deserialize/deserialize_checked methods for module
deserialization.

* Make deserialize() methods use artifact validation
* Make checked methods unsafe again, because loading executable memory
  is inherently unsafe
* Rename methods that skip artifact validation to deserialize_unchecked

Closes #3727
theduke added a commit that referenced this issue May 22, 2023
Rework the deserialize/deserialize_checked methods for module
deserialization.

* Make deserialize() methods use artifact validation
* Make checked methods unsafe again, because loading executable memory
  is inherently unsafe
* Rename methods that skip artifact validation to deserialize_unchecked

Closes #3727
theduke added a commit that referenced this issue May 22, 2023
Rework the deserialize/deserialize_checked methods for module
deserialization.

* Make deserialize() methods use artifact validation
* Make checked methods unsafe again, because loading executable memory
  is inherently unsafe
* Rename methods that skip artifact validation to deserialize_unchecked

Closes #3727
theduke added a commit that referenced this issue May 23, 2023
Rework the deserialize/deserialize_checked methods for module
deserialization.

* Make deserialize() methods use artifact validation
* Make checked methods unsafe again, because loading executable memory
  is inherently unsafe
* Rename methods that skip artifact validation to deserialize_unchecked

Closes #3727
theduke added a commit that referenced this issue May 23, 2023
Rework the deserialize/deserialize_checked methods for module
deserialization.

* Make deserialize() methods use artifact validation
* Make checked methods unsafe again, because loading executable memory
  is inherently unsafe
* Rename methods that skip artifact validation to deserialize_unchecked

Closes #3727
theduke added a commit that referenced this issue May 23, 2023
Rework the deserialize/deserialize_checked methods for module
deserialization.

* Make deserialize() methods use artifact validation
* Make checked methods unsafe again, because loading executable memory
  is inherently unsafe
* Rename methods that skip artifact validation to deserialize_unchecked

Closes #3727
theduke added a commit that referenced this issue May 23, 2023
Rework the deserialize/deserialize_checked methods for module
deserialization.

* Make deserialize() methods use artifact validation
* Make checked methods unsafe again, because loading executable memory
  is inherently unsafe
* Rename methods that skip artifact validation to deserialize_unchecked

Closes #3727
theduke added a commit that referenced this issue May 23, 2023
Rework the deserialize/deserialize_checked methods for module
deserialization.

* Make deserialize() methods use artifact validation
* Make checked methods unsafe again, because loading executable memory
  is inherently unsafe
* Rename methods that skip artifact validation to deserialize_unchecked

Closes #3727
theduke added a commit that referenced this issue May 23, 2023
Rework the deserialize/deserialize_checked methods for module
deserialization.

* Make deserialize() methods use artifact validation
* Make checked methods unsafe again, because loading executable memory
  is inherently unsafe
* Rename methods that skip artifact validation to deserialize_unchecked

Closes #3727
theduke added a commit that referenced this issue May 23, 2023
Rework the deserialize/deserialize_checked methods for module
deserialization.

* Make deserialize() methods use artifact validation
* Make checked methods unsafe again, because loading executable memory
  is inherently unsafe
* Rename methods that skip artifact validation to deserialize_unchecked

Closes #3727
theduke added a commit that referenced this issue May 24, 2023
Rework the deserialize/deserialize_checked methods for module
deserialization.

* Make deserialize() methods use artifact validation
* Make checked methods unsafe again, because loading executable memory
  is inherently unsafe
* Rename methods that skip artifact validation to deserialize_unchecked

Closes #3727
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎉 enhancement New feature! 📦 lib-engine About wasmer-engine priority-medium Medium priority issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants