From f4940e115fd5de059c15be78f1df66081fa9bec4 Mon Sep 17 00:00:00 2001
From: Devyn Cairns <devyn.cairns@gmail.com>
Date: Sun, 1 Sep 2024 08:33:10 -0700
Subject: [PATCH] Remove bincode and use MessagePack instead for plugin custom
 values (#13745)

# Description

This changes the serialization of custom values within the plugin
protocol to use MessagePack instead of bincode, removing the dependency
on bincode entirely.

Bincode does not seem to be very maintained anymore, and the externally
tagged enum representation doesn't seem to always work now even though
it should. Since we use MessagePack already anyway for the plugin
protocol, this seems like an obvious choice. This uses the unnamed
variant of the serialization rather than the named variant, which is
what the plugin protocol in general uses. The unnamed variant does not
include field names, which aren't really required here, so this should
give us something that's more or less as efficient as bincode is.

Should fix #13743.

# User-Facing Changes

- Will need to recompile plugins (but always do anyway)
- Doesn't technically break the plugin protocol (custom value data is a
black box / plugin implementation specific), but breaks compatibility
between `nu-plugin-engine` and `nu-plugin` so they do need to both be
updated to match.

# Tests + Formatting

All tests pass.

# After Submitting

- [ ] release notes
---
 Cargo.lock                                            | 11 +----------
 crates/nu-plugin-protocol/Cargo.toml                  |  2 +-
 .../nu-plugin-protocol/src/plugin_custom_value/mod.rs |  4 ++--
 crates/nu-plugin-protocol/src/test_util.rs            |  4 ++--
 4 files changed, 6 insertions(+), 15 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index 089938ed6d..c73ddb8f32 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -366,15 +366,6 @@ version = "0.22.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "72b3254f16251a8381aa12e40e3c4d2f0199f8c6508fbecb9d91f575e0fbb8c6"
 
-[[package]]
-name = "bincode"
-version = "1.3.3"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "b1f45e9417d87227c7a56d22e471c6206462cba514c7590c09aff4cf6d1ddcad"
-dependencies = [
- "serde",
-]
-
 [[package]]
 name = "bindgen"
 version = "0.69.4"
@@ -3364,9 +3355,9 @@ dependencies = [
 name = "nu-plugin-protocol"
 version = "0.97.2"
 dependencies = [
- "bincode",
  "nu-protocol",
  "nu-utils",
+ "rmp-serde",
  "semver",
  "serde",
  "typetag",
diff --git a/crates/nu-plugin-protocol/Cargo.toml b/crates/nu-plugin-protocol/Cargo.toml
index 45d8fe3a97..1d8aab6a1a 100644
--- a/crates/nu-plugin-protocol/Cargo.toml
+++ b/crates/nu-plugin-protocol/Cargo.toml
@@ -17,7 +17,7 @@ workspace = true
 nu-protocol = { path = "../nu-protocol", version = "0.97.2", features = ["plugin"] }
 nu-utils = { path = "../nu-utils", version = "0.97.2" }
 
-bincode = "1.3"
+rmp-serde = { workspace = true }
 serde = { workspace = true, features = ["derive"] }
 semver = "1.0"
 typetag = "0.2"
diff --git a/crates/nu-plugin-protocol/src/plugin_custom_value/mod.rs b/crates/nu-plugin-protocol/src/plugin_custom_value/mod.rs
index ea2b1551c9..db7ea733d7 100644
--- a/crates/nu-plugin-protocol/src/plugin_custom_value/mod.rs
+++ b/crates/nu-plugin-protocol/src/plugin_custom_value/mod.rs
@@ -142,7 +142,7 @@ impl PluginCustomValue {
     ) -> Result<PluginCustomValue, ShellError> {
         let name = custom_value.type_name();
         let notify_on_drop = custom_value.notify_plugin_on_drop();
-        bincode::serialize(custom_value)
+        rmp_serde::to_vec(custom_value)
             .map(|data| PluginCustomValue::new(name, data, notify_on_drop))
             .map_err(|err| ShellError::CustomValueFailedToEncode {
                 msg: err.to_string(),
@@ -156,7 +156,7 @@ impl PluginCustomValue {
         &self,
         span: Span,
     ) -> Result<Box<dyn CustomValue>, ShellError> {
-        bincode::deserialize::<Box<dyn CustomValue>>(self.data()).map_err(|err| {
+        rmp_serde::from_slice::<Box<dyn CustomValue>>(self.data()).map_err(|err| {
             ShellError::CustomValueFailedToDecode {
                 msg: err.to_string(),
                 span,
diff --git a/crates/nu-plugin-protocol/src/test_util.rs b/crates/nu-plugin-protocol/src/test_util.rs
index 579b1ee758..7d3bfb28ca 100644
--- a/crates/nu-plugin-protocol/src/test_util.rs
+++ b/crates/nu-plugin-protocol/src/test_util.rs
@@ -31,8 +31,8 @@ impl CustomValue for TestCustomValue {
 
 /// A [`TestCustomValue`] serialized as a [`PluginCustomValue`].
 pub fn test_plugin_custom_value() -> PluginCustomValue {
-    let data = bincode::serialize(&expected_test_custom_value() as &dyn CustomValue)
-        .expect("bincode serialization of the expected_test_custom_value() failed");
+    let data = rmp_serde::to_vec(&expected_test_custom_value() as &dyn CustomValue)
+        .expect("MessagePack serialization of the expected_test_custom_value() failed");
 
     PluginCustomValue::new("TestCustomValue".into(), data, false)
 }