Fix numeric key serialization

lxsmnsyc · Jan 6, 2024 · e492ee5 · e492ee5
1 parent 25ab39c
commit e492ee5
Show file tree

Hide file tree

Showing 10 changed files with 162 additions and 112 deletions.
diff --git a/packages/seroval/src/core/context/serializer.ts b/packages/seroval/src/core/context/serializer.ts
@@ -446,12 +446,24 @@ export default abstract class BaseSerializerContext
   ): string {
     if (typeof key === 'string') {
       const check = Number(key);
-      // Test if key is a valid number or JS identifier
-      // so that we don't have to serialize the key and wrap with brackets
-      const isIdentifier = check >= 0 || isValidIdentifier(key);
+      const isIdentifier = (
+        // Test if key is a valid positive number or JS identifier
+        // so that we don't have to serialize the key and wrap with brackets
+        check >= 0
+        // It's also important to consider that if the key is
+        // indeed numeric, we need to make sure that when
+        // converted back into a string, it's still the same
+        // to the original key. This allows us to differentiate
+        // keys that has numeric formats but in a different
+        // format, which can cause unintentional key declaration
+        // Example: { 0x1: 1 } vs { '0x1': 1 }
+        && check.toString() === key
+      ) || isValidIdentifier(key);
       if (this.isIndexedValueInStack(val)) {
         const refParam = this.getRefParam((val as SerovalIndexedValueNode).i);
         this.markRef(source.i);
+        // Strict identifier check, make sure
+        // that it isn't numeric (except NaN)
         if (isIdentifier && check !== check) {
           this.createObjectAssign(source.i, key, refParam);
         } else {
@@ -516,10 +528,22 @@ export default abstract class BaseSerializerContext
   ): void {
     const serialized = this.serialize(value);
     const check = Number(key);
-    // Test if key is a valid number or JS identifier
-    // so that we don't have to serialize the key and wrap with brackets
-    const isIdentifier = check >= 0 || isValidIdentifier(key);
+    const isIdentifier = (
+      // Test if key is a valid positive number or JS identifier
+      // so that we don't have to serialize the key and wrap with brackets
+      check >= 0
+      // It's also important to consider that if the key is
+      // indeed numeric, we need to make sure that when
+      // converted back into a string, it's still the same
+      // to the original key. This allows us to differentiate
+      // keys that has numeric formats but in a different
+      // format, which can cause unintentional key declaration
+      // Example: { 0x1: 1 } vs { '0x1': 1 }
+      && check.toString() === key
+    ) || isValidIdentifier(key);
     if (this.isIndexedValueInStack(value)) {
+      // Strict identifier check, make sure
+      // that it isn't numeric (except NaN)
       if (isIdentifier && check !== check) {
         this.createObjectAssign(source.i, key, serialized);
       } else {
@@ -532,7 +556,7 @@ export default abstract class BaseSerializerContext
     } else {
       const parentAssignment = this.assignments;
       this.assignments = mainAssignments;
-      if (isIdentifier) {
+      if (isIdentifier && check !== check) {
         this.createObjectAssign(source.i, key, serialized);
       } else {
         this.createArrayAssign(

diff --git a/packages/seroval/test/__snapshots__/frozen-object.test.ts.snap b/packages/seroval/test/__snapshots__/frozen-object.test.ts.snap
diff --git a/packages/seroval/test/__snapshots__/null-constructor.test.ts.snap b/packages/seroval/test/__snapshots__/null-constructor.test.ts.snap
diff --git a/packages/seroval/test/__snapshots__/object.test.ts.snap b/packages/seroval/test/__snapshots__/object.test.ts.snap
diff --git a/packages/seroval/test/__snapshots__/sealed-object.test.ts.snap b/packages/seroval/test/__snapshots__/sealed-object.test.ts.snap
diff --git a/packages/seroval/test/frozen-object.test.ts b/packages/seroval/test/frozen-object.test.ts
@@ -15,7 +15,15 @@ import {
   toJSONAsync,
 } from '../src';
 
-const EXAMPLE = Object.freeze({ hello: 'world' });
+const EXAMPLE = Object.freeze({
+  example: 'valid identifier',
+  '%example': 'invalid identifier',
+  '0x1': 'hexadecimal',
+  '0b1': 'binary',
+  '0o1': 'octal',
+  '1_000': 'numeric separator',
+  '1.7976931348623157e+308': 'exponentiation',
+});
 
 const RECURSIVE = {} as Record<string, unknown>;
 RECURSIVE.a = RECURSIVE;
@@ -52,7 +60,7 @@ describe('frozen object', () => {
       const back = deserialize<typeof EXAMPLE>(result);
       expect(Object.isFrozen(back)).toBe(true);
       expect(back.constructor).toBe(Object);
-      expect(back.hello).toBe(EXAMPLE.hello);
+      expect(back.example).toBe(EXAMPLE.example);
     });
     it('supports self-recursion', () => {
       const result = serialize(RECURSIVE);
@@ -78,7 +86,7 @@ describe('frozen object', () => {
       const back = await deserialize<Promise<typeof EXAMPLE>>(result);
       expect(Object.isFrozen(back)).toBe(true);
       expect(back.constructor).toBe(Object);
-      expect(back.hello).toBe(EXAMPLE.hello);
+      expect(back.example).toBe(EXAMPLE.example);
     });
     it('supports self-recursion', async () => {
       const result = await serializeAsync(ASYNC_RECURSIVE);
@@ -112,7 +120,7 @@ describe('frozen object', () => {
       const back = fromJSON<typeof EXAMPLE>(result);
       expect(Object.isFrozen(back)).toBe(true);
       expect(back.constructor).toBe(Object);
-      expect(back.hello).toBe(EXAMPLE.hello);
+      expect(back.example).toBe(EXAMPLE.example);
     });
     it('supports self-recursion', () => {
       const result = toJSON(RECURSIVE);
@@ -138,7 +146,7 @@ describe('frozen object', () => {
       const back = await fromJSON<Promise<typeof EXAMPLE>>(result);
       expect(Object.isFrozen(back)).toBe(true);
       expect(back.constructor).toBe(Object);
-      expect(back.hello).toBe(EXAMPLE.hello);
+      expect(back.example).toBe(EXAMPLE.example);
     });
     it('supports self-recursion', async () => {
       const result = await toJSONAsync(ASYNC_RECURSIVE);
@@ -167,9 +175,7 @@ describe('frozen object', () => {
   });
   describe('crossSerialize', () => {
     it('supports Objects', () => {
-      const example = { hello: 'world' } as { hello: string };
-      Object.freeze(example);
-      const result = crossSerialize(example);
+      const result = crossSerialize(EXAMPLE);
       expect(result).toMatchSnapshot();
     });
     it('supports self-recursion', () => {
@@ -368,7 +374,7 @@ describe('frozen object', () => {
       });
       expect(Object.isFrozen(back)).toBe(true);
       expect(back.constructor).toBe(Object);
-      expect(back.hello).toBe(EXAMPLE.hello);
+      expect(back.example).toBe(EXAMPLE.example);
     });
     it('supports self-recursion', () => {
       const result = toCrossJSON(RECURSIVE);
@@ -400,7 +406,7 @@ describe('frozen object', () => {
       });
       expect(Object.isFrozen(back)).toBe(true);
       expect(back.constructor).toBe(Object);
-      expect(back.hello).toBe(EXAMPLE.hello);
+      expect(back.example).toBe(EXAMPLE.example);
     });
     it('supports self-recursion', async () => {
       const result = await toCrossJSONAsync(ASYNC_RECURSIVE);

diff --git a/packages/seroval/test/null-constructor.test.ts b/packages/seroval/test/null-constructor.test.ts
@@ -17,9 +17,15 @@ import {
   toJSONAsync,
 } from '../src';
 
-const EXAMPLE = Object.assign(Object.create(null), { hello: 'world' }) as {
-  hello: string;
-};
+const EXAMPLE = Object.assign(Object.create(null), {
+  example: 'valid identifier',
+  '%example': 'invalid identifier',
+  '0x1': 'hexadecimal',
+  '0b1': 'binary',
+  '0o1': 'octal',
+  '1_000': 'numeric separator',
+  '1.7976931348623157e+308': 'exponentiation',
+});
 
 const RECURSIVE = Object.create(null) as Record<string, unknown>;
 RECURSIVE.a = RECURSIVE;
@@ -53,7 +59,7 @@ describe('null-constructor', () => {
       expect(result).toMatchSnapshot();
       const back = deserialize<typeof EXAMPLE>(result);
       expect(back.constructor).toBeUndefined();
-      expect(back.hello).toBe(EXAMPLE.hello);
+      expect(back.example).toBe(EXAMPLE.example);
     });
     it('supports self-recursion', () => {
       const result = serialize(RECURSIVE);
@@ -76,7 +82,7 @@ describe('null-constructor', () => {
       expect(result).toMatchSnapshot();
       const back = await deserialize<Promise<typeof EXAMPLE>>(result);
       expect(back.constructor).toBeUndefined();
-      expect(back.hello).toBe(EXAMPLE.hello);
+      expect(back.example).toBe(EXAMPLE.example);
     });
     it('supports self-recursion', async () => {
       const result = await serializeAsync(ASYNC_RECURSIVE);
@@ -106,7 +112,7 @@ describe('null-constructor', () => {
       expect(JSON.stringify(result)).toMatchSnapshot();
       const back = fromJSON<typeof EXAMPLE>(result);
       expect(back.constructor).toBeUndefined();
-      expect(back.hello).toBe(EXAMPLE.hello);
+      expect(back.example).toBe(EXAMPLE.example);
     });
     it('supports self-recursion', () => {
       const result = toJSON(RECURSIVE);
@@ -129,7 +135,7 @@ describe('null-constructor', () => {
       expect(JSON.stringify(result)).toMatchSnapshot();
       const back = await fromJSON<Promise<typeof EXAMPLE>>(result);
       expect(back.constructor).toBeUndefined();
-      expect(back.hello).toBe(EXAMPLE.hello);
+      expect(back.example).toBe(EXAMPLE.example);
     });
     it('supports self-recursion', async () => {
       const result = await toJSONAsync(ASYNC_RECURSIVE);
@@ -155,9 +161,7 @@ describe('null-constructor', () => {
   });
   describe('crossSerialize', () => {
     it('supports Object.create(null)', () => {
-      const example = { hello: 'world' } as { hello: string };
-      Object.freeze(example);
-      const result = crossSerialize(example);
+      const result = crossSerialize(EXAMPLE);
       expect(result).toMatchSnapshot();
     });
     it('supports self-recursion', () => {
@@ -356,7 +360,7 @@ describe('null-constructor', () => {
       });
 
       expect(back.constructor).toBeUndefined();
-      expect(back.hello).toBe(EXAMPLE.hello);
+      expect(back.example).toBe(EXAMPLE.example);
     });
     it('supports self-recursion', () => {
       const result = toCrossJSON(RECURSIVE);
@@ -388,7 +392,7 @@ describe('null-constructor', () => {
       });
 
       expect(back.constructor).toBeUndefined();
-      expect(back.hello).toBe(EXAMPLE.hello);
+      expect(back.example).toBe(EXAMPLE.example);
     });
     it('supports self-recursion', async () => {
       const result = await toCrossJSONAsync(ASYNC_RECURSIVE);
@@ -485,7 +489,7 @@ describe('null-constructor', () => {
       expect(result).toMatchSnapshot();
       const back = deserialize<typeof EXAMPLE>(result);
       expect(back.constructor).toBeUndefined();
-      expect(back.hello).toBe(EXAMPLE.hello);
+      expect(back.example).toBe(EXAMPLE.example);
     });
   });
   describe('compat#toJSON', () => {
@@ -497,7 +501,7 @@ describe('null-constructor', () => {
       expect(compileJSON(result)).toMatchSnapshot();
       const back = fromJSON<typeof EXAMPLE>(result);
       expect(back.constructor).toBeUndefined();
-      expect(back.hello).toBe(EXAMPLE.hello);
+      expect(back.example).toBe(EXAMPLE.example);
     });
   });
 });
diff --git a/packages/seroval/test/object.test.ts b/packages/seroval/test/object.test.ts
@@ -17,7 +17,15 @@ import {
   toJSONAsync,
 } from '../src';
 
-const EXAMPLE = { hello: 'world' };
+const EXAMPLE = {
+  example: 'valid identifier',
+  '%example': 'invalid identifier',
+  '0x1': 'hexadecimal',
+  '0b1': 'binary',
+  '0o1': 'octal',
+  '1_000': 'numeric separator',
+  '1.7976931348623157e+308': 'exponentiation',
+};
 
 const RECURSIVE = {} as Record<string, unknown>;
 RECURSIVE.a = RECURSIVE;
@@ -51,7 +59,7 @@ describe('objects', () => {
       expect(result).toMatchSnapshot();
       const back = deserialize<typeof EXAMPLE>(result);
       expect(back.constructor).toBe(Object);
-      expect(back.hello).toBe(EXAMPLE.hello);
+      expect(back.example).toBe(EXAMPLE.example);
     });
     it('supports self-recursion', () => {
       const result = serialize(RECURSIVE);
@@ -74,7 +82,7 @@ describe('objects', () => {
       expect(result).toMatchSnapshot();
       const back = await deserialize<Promise<typeof EXAMPLE>>(result);
       expect(back.constructor).toBe(Object);
-      expect(back.hello).toBe(EXAMPLE.hello);
+      expect(back.example).toBe(EXAMPLE.example);
     });
     it('supports self-recursion', async () => {
       const result = await serializeAsync(ASYNC_RECURSIVE);
@@ -104,7 +112,7 @@ describe('objects', () => {
       expect(JSON.stringify(result)).toMatchSnapshot();
       const back = fromJSON<typeof EXAMPLE>(result);
       expect(back.constructor).toBe(Object);
-      expect(back.hello).toBe(EXAMPLE.hello);
+      expect(back.example).toBe(EXAMPLE.example);
     });
     it('supports self-recursion', () => {
       const result = toJSON(RECURSIVE);
@@ -127,7 +135,7 @@ describe('objects', () => {
       expect(JSON.stringify(result)).toMatchSnapshot();
       const back = await fromJSON<Promise<typeof EXAMPLE>>(result);
       expect(back.constructor).toBe(Object);
-      expect(back.hello).toBe(EXAMPLE.hello);
+      expect(back.example).toBe(EXAMPLE.example);
     });
     it('supports self-recursion', async () => {
       const result = await toJSONAsync(ASYNC_RECURSIVE);
@@ -153,9 +161,7 @@ describe('objects', () => {
   });
   describe('crossSerialize', () => {
     it('supports Objects', () => {
-      const example = { hello: 'world' } as { hello: string };
-      Object.freeze(example);
-      const result = crossSerialize(example);
+      const result = crossSerialize(EXAMPLE);
       expect(result).toMatchSnapshot();
     });
     it('supports self-recursion', () => {
@@ -354,7 +360,7 @@ describe('objects', () => {
       });
 
       expect(back.constructor).toBe(Object);
-      expect(back.hello).toBe(EXAMPLE.hello);
+      expect(back.example).toBe(EXAMPLE.example);
     });
     it('supports self-recursion', () => {
       const result = toCrossJSON(RECURSIVE);
@@ -386,7 +392,7 @@ describe('objects', () => {
       });
 
       expect(back.constructor).toBe(Object);
-      expect(back.hello).toBe(EXAMPLE.hello);
+      expect(back.example).toBe(EXAMPLE.example);
     });
     it('supports self-recursion', async () => {
       const result = await toCrossJSONAsync(ASYNC_RECURSIVE);
@@ -483,7 +489,7 @@ describe('objects', () => {
       expect(result).toMatchSnapshot();
       const back = deserialize<typeof EXAMPLE>(result);
       expect(back.constructor).toBe(Object);
-      expect(back.hello).toBe(EXAMPLE.hello);
+      expect(back.example).toBe(EXAMPLE.example);
     });
   });
   describe('compat#toJSON', () => {
@@ -495,7 +501,7 @@ describe('objects', () => {
       expect(compileJSON(result)).toMatchSnapshot();
       const back = fromJSON<typeof EXAMPLE>(result);
       expect(back.constructor).toBe(Object);
-      expect(back.hello).toBe(EXAMPLE.hello);
+      expect(back.example).toBe(EXAMPLE.example);
     });
   });
 });