fix(serde_v8): serialize objects with numeric keys correctly

Fixes: denoland#11502 Signed-off-by: Darshan Sen <raisinten@gmail.com>
RaisinTen · Sep 25, 2022 · 10f0cac · 10f0cac
1 parent d7b27ed
commit 10f0cac
Show file tree

Hide file tree

Showing 3 changed files with 113 additions and 2 deletions.
diff --git a/core/runtime.rs b/core/runtime.rs
@@ -3216,6 +3216,90 @@ assertEquals(1, notify_return_value);
     runtime.run_event_loop(false).await.unwrap();
   }
 
+  #[tokio::test]
+  async fn test_sync_op_serialize_object_with_numbers_as_keys() {
+    #[op]
+    fn op_sync_serialize_object_with_numbers_as_keys(
+      value: serde_json::Value,
+    ) -> Result<(), Error> {
+      assert_eq!(
+        value.to_string(),
+        r#"{"lines":{"100":{"unit":"m"},"200":{"unit":"cm"}}}"#
+      );
+      Ok(())
+    }
+
+    let extension = Extension::builder()
+      .ops(vec![op_sync_serialize_object_with_numbers_as_keys::decl()])
+      .build();
+
+    let mut runtime = JsRuntime::new(RuntimeOptions {
+      extensions: vec![extension],
+      ..Default::default()
+    });
+
+    runtime
+      .execute_script(
+        "op_sync_serialize_object_with_numbers_as_keys.js",
+        r#"
+Deno.core.ops.op_sync_serialize_object_with_numbers_as_keys({
+  lines: {
+    100: {
+      unit: "m"
+    },
+    200: {
+      unit: "cm"
+    }
+  }
+})
+"#,
+      )
+      .unwrap();
+    runtime.run_event_loop(false).await.unwrap();
+  }
+
+  #[tokio::test]
+  async fn test_async_op_serialize_object_with_numbers_as_keys() {
+    #[op]
+    async fn op_async_serialize_object_with_numbers_as_keys(
+      value: serde_json::Value,
+    ) -> Result<(), Error> {
+      assert_eq!(
+        value.to_string(),
+        r#"{"lines":{"100":{"unit":"m"},"200":{"unit":"cm"}}}"#
+      );
+      Ok(())
+    }
+
+    let extension = Extension::builder()
+      .ops(vec![op_async_serialize_object_with_numbers_as_keys::decl()])
+      .build();
+
+    let mut runtime = JsRuntime::new(RuntimeOptions {
+      extensions: vec![extension],
+      ..Default::default()
+    });
+
+    runtime
+      .execute_script(
+        "op_async_serialize_object_with_numbers_as_keys.js",
+        r#"
+Deno.core.opAsync('op_async_serialize_object_with_numbers_as_keys', {
+  lines: {
+    100: {
+      unit: "m"
+    },
+    200: {
+      unit: "cm"
+    }
+  }
+})
+"#,
+      )
+      .unwrap();
+    runtime.run_event_loop(false).await.unwrap();
+  }
+
   #[tokio::test]
   async fn test_set_macrotask_callback_set_next_tick_callback() {
     #[op]

diff --git a/serde_v8/de.rs b/serde_v8/de.rs
@@ -325,8 +325,12 @@ impl<'de, 'a, 'b, 's, 'x> de::Deserializer<'de>
       };
       visitor.visit_map(map)
     } else {
-      let prop_names =
-        obj.get_own_property_names(self.scope, Default::default());
+      let prop_names = obj.get_own_property_names(
+        self.scope,
+        v8::GetPropertyNamesArgsBuilder::new()
+          .key_conversion(v8::KeyConversionMode::ConvertToString)
+          .build(),
+      );
       let keys: Vec<magic::Value> = match prop_names {
         Some(names) => from_v8(self.scope, names.into()).unwrap(),
         None => vec![],

diff --git a/serde_v8/tests/de.rs b/serde_v8/tests/de.rs
@@ -184,6 +184,29 @@ fn de_map() {
   })
 }
 
+#[test]
+fn de_obj_with_numeric_keys() {
+  dedo(
+    r#"({
+  lines: {
+    100: {
+      unit: "m"
+    },
+    200: {
+      unit: "cm"
+    }
+  }
+})"#,
+    |scope, v| {
+      let json: serde_json::Value = serde_v8::from_v8(scope, v).unwrap();
+      assert_eq!(
+        json.to_string(),
+        r#"{"lines":{"100":{"unit":"m"},"200":{"unit":"cm"}}}"#
+      );
+    },
+  )
+}
+
 #[test]
 fn de_string_or_buffer() {
   dedo("'hello'", |scope, v| {