Merge pull request #639 from benhaney/serde-ok-error

Serde: Restrict special ok/error handling to variant names

CHANGELOG.md

@@ -22,6 +22,9 @@ now (#638)
 
 ### Changed
 
+- The special ok/error handling for atoms in serde is now restricted to variant
+  names (#639)
+
 ### Removed
 
 ## [0.34.0] - 2024-07-09

rustler/src/serde/atoms.rs

@@ -3,9 +3,6 @@
 use crate::serde::Error;
 use crate::{types::atom::Atom, Encoder, Env, Term};
 
-pub static OK: &str = "Ok";
-pub static ERROR: &str = "Err";
-
 atoms! {
     nil,
     ok,
@@ -23,28 +20,18 @@ atoms! {
  * Attempts to create an atom term from the provided string (if the atom already exists in the atom table). If not, returns a string term.
  */
 pub fn str_to_term<'a>(env: &Env<'a>, string: &str) -> Result<Term<'a>, Error> {
-    if string == "Ok" {
-        Ok(ok().encode(*env))
-    } else if string == "Err" {
-        Ok(error().encode(*env))
-    } else {
-        match Atom::try_from_bytes(*env, string.as_bytes()) {
-            Ok(Some(term)) => Ok(term.encode(*env)),
-            Ok(None) => Err(Error::InvalidStringable),
-            _ => Err(Error::InvalidStringable),
-        }
+    match Atom::try_from_bytes(*env, string.as_bytes()) {
+        Ok(Some(term)) => Ok(term.encode(*env)),
+        Ok(None) => Ok(string.encode(*env)),
+        _ => Err(Error::InvalidStringable),
     }
 }
 
 /**
  * Attempts to create a `String` from the term.
  */
 pub fn term_to_string(term: &Term) -> Result<String, Error> {
-    if ok().eq(term) {
-        Ok(OK.to_string())
-    } else if error().eq(term) {
-        Ok(ERROR.to_string())
-    } else if term.is_atom() {
+    if term.is_atom() {
         term.atom_to_string().or(Err(Error::InvalidAtom))
     } else {
         Err(Error::InvalidStringable)

rustler/src/serde/de.rs

@@ -728,8 +728,13 @@ impl<'de, 'a: 'de> de::Deserializer<'de> for VariantNameDeserializer<'a> {
     {
         match self.variant.get_type() {
             TermType::Atom => {
-                let string =
-                    atoms::term_to_string(&self.variant).or(Err(Error::InvalidVariantName))?;
+                let string = atoms::term_to_string(&self.variant)
+                    .map(|s| match s.as_str() {
+                        "ok" => "Ok".to_string(),
+                        "error" => "Err".to_string(),
+                        _ => s,
+                    })
+                    .or(Err(Error::InvalidVariantName))?;
                 visitor.visit_string(string)
             }
             TermType::Binary => visitor.visit_string(util::term_to_str(&self.variant)?),

rustler/src/serde/mod.rs

@@ -62,7 +62,7 @@ with added support for 128-bit integers.
 | <sup>[3](#atom)</sup> unit variant | `E::A` in `enum UnitVariant { A }` | `:A` | `"A"` |
 | <sup>[3](#atom)</sup> newtype struct | `struct Millimeters(u8)` | `{:Millimeters, u8}` | `["Millimeters", u8]` |
 | <sup>[3](#atom)</sup> newtype variant | `E::N` in `enum E { N(u8) }` | `{:N, u8}` | `["N", u8]` |
-| <sup>[3](#atom)</sup> newtype variant (any `Ok` and `Err` tagged enum) | `enum R<T, E> { Ok(T), Err(E) }` | `{:ok, T}` or `{:error, E}` | `["Ok", T]` or `["Err", E]` |
+| <sup>[3](#atom)</sup> newtype variant (any `Ok` and `Err` tagged enum) | `enum R<T, E> { Ok(T), Err(E) }` | `{:ok, T}` or `{:error, E}` | `["ok", T]` or `["error", E]` |
 | seq | `Vec<T>` | `[T,]` | `[T,]` |
 | tuple | `(u8,)` | `{u8,}` | `[u8,]` |
 | <sup>[3](#atom)</sup> tuple struct | `struct Rgb(u8, u8, u8)` | `{:Rgb, u8, u8, u8}` | `["Rgb", u8, u8, u8]` |

rustler/src/serde/ser.rs

@@ -212,17 +212,17 @@ impl<'a> ser::Serializer for Serializer<'a> {
     /// `enum Result { Ok(u8), Err(_) }` into `{:ok, u8}` or `{:err, _}`.
     fn serialize_newtype_variant<T>(
         self,
-        _name: &'static str,
+        name: &'static str,
         _variant_index: u32,
         variant: &'static str,
         value: &T,
     ) -> Result<Self::Ok, Self::Error>
     where
         T: ?Sized + ser::Serialize,
     {
-        match variant {
-            "Ok" => self.serialize_newtype_struct("ok", value),
-            "Err" => self.serialize_newtype_struct("error", value),
+        match (name, variant) {
+            ("Result", "Ok") => self.serialize_newtype_struct("ok", value),
+            ("Result", "Err") => self.serialize_newtype_struct("error", value),
             _ => self.serialize_newtype_struct(variant, value),
         }
     }

rustler_tests/test/serde_rustler_tests_test.exs

@@ -253,14 +253,14 @@ defmodule SerdeRustlerTests.NifTest do
 
     test "newtype variant (Result::Ok(T), or {:ok, T})", ctx do
       test_case = {:ok, 255}
-      transcoded = ["Ok", 255]
+      transcoded = ["ok", 255]
       run_tests("newtype variant (ok tuple)", test_case, Helpers.skip(ctx, :transcode))
       Helpers.run_transcode("newtype variant (ok tuple)", test_case, transcoded)
     end
 
     test "newtype variant (Result::Err(T), or {:error, T}", ctx do
       test_case = {:error, "error reason"}
-      transcoded = ["Err", "error reason"]
+      transcoded = ["error", "error reason"]
       run_tests("newtype variant (error tuple)", test_case, Helpers.skip(ctx, :transcode))
       Helpers.run_transcode("newtype variant (error tuple)", test_case, transcoded)
     end