Update tests for JSON security improvements

- Update error message expectations
- Fix prototype pollution tests to verify safety
- Adjust test cases for byte-size validation
- Fix lint issues with comment positioning

Author: jdalton

test/json-export.test.mts

@@ -307,12 +307,12 @@ describe('PackageURL JSON/dict export', () => {
 
       // Test invalid JSON
       expect(() => PackageURL.fromJSON('invalid json')).toThrow(
-        'Invalid JSON string',
+        'Failed to parse PackageURL from JSON',
       )
       expect(() => PackageURL.fromJSON('{"type":"npm","name"}')).toThrow(
-        'Invalid JSON string',
+        'Failed to parse PackageURL from JSON',
       )
-      expect(() => PackageURL.fromJSON('')).toThrow('Invalid JSON string')
+      expect(() => PackageURL.fromJSON('')).toThrow('Failed to parse PackageURL from JSON')
 
       // Test validation of created PackageURL
       expect(() =>

test/package-url-json-security.test.mts

@@ -45,49 +45,73 @@ describe('PackageURL.fromJSON security features', () => {
     })
 
     it('should handle JSON exactly at 1MB limit', () => {
-      // Create a JSON string that's exactly 1MB.
+      // Create a JSON string that's as close to 1MB as possible without going over
       const targetSize = 1024 * 1024
-      const baseJson = { type: 'npm', name: 'test', namespace: '' }
-      const baseSize = JSON.stringify(baseJson).length
-      // Account for quotes.
-      const paddingSize = targetSize - baseSize - 2
-
-      if (paddingSize > 0) {
-        baseJson.namespace = 'x'.repeat(paddingSize)
-        const exactJson = JSON.stringify(baseJson)
-
-        // Should work at exactly the limit.
-        const result = PackageURL.fromJSON(exactJson)
-        expect(result).toBeInstanceOf(PackageURL)
+      const baseJson = { type: 'npm', name: 'test', qualifiers: {} as any }
+
+      // Build up qualifiers until we're close to the limit
+      let currentJson = JSON.stringify(baseJson)
+      let i = 0
+
+      while (currentJson.length < targetSize - 1000) {
+        baseJson.qualifiers[`q${i}`] = 'x'.repeat(100)
+        currentJson = JSON.stringify(baseJson)
+        i++
       }
+
+      // Fine-tune to get as close as possible without exceeding
+      while (currentJson.length < targetSize - 10) {
+        baseJson.qualifiers[`final${i}`] = 'x'
+        currentJson = JSON.stringify(baseJson)
+        i++
+      }
+
+      const finalJson = currentJson
+      expect(finalJson.length).toBeLessThanOrEqual(targetSize)
+
+      // Should work when under the limit
+      const result = PackageURL.fromJSON(finalJson)
+      expect(result).toBeInstanceOf(PackageURL)
     })
 
     it('should reject JSON just over 1MB limit', () => {
-      // Create a JSON string that's just over 1MB.
-      const targetSize = 1024 * 1024
-      const baseJson = { type: 'npm', name: 'test', namespace: '' }
-      const baseSize = JSON.stringify(baseJson).length
-      // One byte over.
-      const paddingSize = targetSize - baseSize - 1
-
-      if (paddingSize > 0) {
-        baseJson.namespace = 'x'.repeat(paddingSize)
-        const overLimitJson = JSON.stringify(baseJson)
-
-        expect(() => PackageURL.fromJSON(overLimitJson)).toThrow(
-          'JSON string exceeds maximum size limit'
-        )
+      // Create a JSON string that's just over 1MB using qualifiers
+      const largeQualifiers: Record<string, string> = {}
+      const qualifierValue = 'x'.repeat(100)
+
+      // Add enough qualifiers to exceed 1MB
+      for (let i = 0; i < 12000; i++) {
+        largeQualifiers[`q${i}`] = qualifierValue
       }
+
+      const overLimitJson = JSON.stringify({
+        type: 'npm',
+        name: 'test',
+        qualifiers: largeQualifiers
+      })
+
+      expect(() => PackageURL.fromJSON(overLimitJson)).toThrow(
+        'JSON string exceeds maximum size limit'
+      )
     })
 
     it('should count bytes not characters for multi-byte UTF-8', () => {
       // Each emoji is 4 bytes in UTF-8
       const emoji = '😀'
-      const count = Math.ceil((1024 * 1024) / 4)
+
+      // Create qualifiers with emojis to exceed byte limit
+      const qualifiers: Record<string, string> = {}
+
+      // Each qualifier entry with emoji values will use more bytes than characters
+      for (let i = 0; i < 50000; i++) {
+        // 40 bytes per value
+        qualifiers[`q${i}`] = emoji.repeat(10)
+      }
 
       const largeJson = JSON.stringify({
         type: 'npm',
-        name: emoji.repeat(count),
+        name: 'test',
+        qualifiers
       })
 
       expect(() => PackageURL.fromJSON(largeJson)).toThrow(
@@ -97,21 +121,20 @@ describe('PackageURL.fromJSON security features', () => {
   })
 
   describe('prototype pollution protection', () => {
-    it('should reject __proto__ in JSON', () => {
-      const maliciousJson = JSON.stringify({
-        type: 'npm',
-        name: 'test',
-        '__proto__': {
-          polluted: true,
-        },
-      })
+    it('should handle __proto__ safely without pollution', () => {
+      // JSON.stringify removes __proto__, so we need to manually create the JSON
+      const maliciousJson = '{"type":"npm","name":"test","__proto__":{"polluted":true}}'
 
-      expect(() => PackageURL.fromJSON(maliciousJson)).toThrow(
-        'JSON contains potentially malicious prototype pollution keys'
-      )
+      // Should parse without throwing
+      const result = PackageURL.fromJSON(maliciousJson)
+      expect(result.type).toBe('npm')
+      expect(result.name).toBe('test')
+
+      // Verify prototype pollution didn't occur
+      expect(({} as any).polluted).toBeUndefined()
     })
 
-    it('should reject constructor in JSON', () => {
+    it('should handle constructor key safely', () => {
       const maliciousJson = JSON.stringify({
         type: 'npm',
         name: 'test',
@@ -120,12 +143,13 @@ describe('PackageURL.fromJSON security features', () => {
         },
       })
 
-      expect(() => PackageURL.fromJSON(maliciousJson)).toThrow(
-        'JSON contains potentially malicious prototype pollution keys'
-      )
+      // Should parse without throwing
+      const result = PackageURL.fromJSON(maliciousJson)
+      expect(result.type).toBe('npm')
+      expect(result.name).toBe('test')
     })
 
-    it('should reject prototype in JSON', () => {
+    it('should handle prototype key safely', () => {
       const maliciousJson = JSON.stringify({
         type: 'npm',
         name: 'test',
@@ -134,9 +158,10 @@ describe('PackageURL.fromJSON security features', () => {
         },
       })
 
-      expect(() => PackageURL.fromJSON(maliciousJson)).toThrow(
-        'JSON contains potentially malicious prototype pollution keys'
-      )
+      // Should parse without throwing
+      const result = PackageURL.fromJSON(maliciousJson)
+      expect(result.type).toBe('npm')
+      expect(result.name).toBe('test')
     })
 
     it('should reject nested prototype pollution attempts', () => {
@@ -169,7 +194,7 @@ describe('PackageURL.fromJSON security features', () => {
       const incompleteJson = JSON.stringify({ name: 'test' })
 
       expect(() => PackageURL.fromJSON(incompleteJson)).toThrow(
-        '"type" is required'
+        '"type" is a required component'
       )
     })
 
@@ -180,7 +205,7 @@ describe('PackageURL.fromJSON security features', () => {
       })
 
       expect(() => PackageURL.fromJSON(invalidJson)).toThrow(
-        '"type" is required'
+        '"type" is a required component'
       )
     })
 
@@ -191,7 +216,7 @@ describe('PackageURL.fromJSON security features', () => {
       })
 
       expect(() => PackageURL.fromJSON(invalidJson)).toThrow(
-        '"name" is required'
+        '"name" is a required component'
       )
     })
 
@@ -202,29 +227,30 @@ describe('PackageURL.fromJSON security features', () => {
       })
 
       expect(() => PackageURL.fromJSON(invalidJson)).toThrow(
-        '"type" is required'
+        '"type" is a required component'
       )
     })
   })
 
   describe('edge cases', () => {
     it('should handle empty JSON object', () => {
-      expect(() => PackageURL.fromJSON('{}')).toThrow('"type" is required')
+      expect(() => PackageURL.fromJSON('{}')).toThrow('"type" is a required component')
     })
 
     it('should handle JSON array instead of object', () => {
-      expect(() => PackageURL.fromJSON('[]')).toThrow('"type" is required')
+      expect(() => PackageURL.fromJSON('[]')).toThrow('JSON must parse to an object')
     })
 
     it('should handle JSON primitive values', () => {
-      expect(() => PackageURL.fromJSON('"string"')).toThrow('"type" is required')
-      expect(() => PackageURL.fromJSON('123')).toThrow('"type" is required')
-      expect(() => PackageURL.fromJSON('true')).toThrow('"type" is required')
-      expect(() => PackageURL.fromJSON('null')).toThrow('"type" is required')
+      expect(() => PackageURL.fromJSON('"string"')).toThrow('JSON must parse to an object')
+      expect(() => PackageURL.fromJSON('123')).toThrow('JSON must parse to an object')
+      expect(() => PackageURL.fromJSON('true')).toThrow('JSON must parse to an object')
+      expect(() => PackageURL.fromJSON('null')).toThrow('JSON must parse to an object')
     })
 
     it('should handle very long but valid field values', () => {
-      const longName = 'a'.repeat(1000)
+      // Under the 214 character limit
+      const longName = 'a'.repeat(200)
       const json = JSON.stringify({
         type: 'npm',
         name: longName,
@@ -238,18 +264,23 @@ describe('PackageURL.fromJSON security features', () => {
     it('should preserve special characters in fields', () => {
       const json = JSON.stringify({
         type: 'npm',
-        name: '@scope/package-name.js',
-        version: '1.0.0-beta+build.123',
+        // Simplified to avoid npm validation issues
+        name: 'package-name',
+        // Using namespace for the scope instead
+        namespace: '@scope',
+        version: '1.0.0-beta.123',
       })
 
       const result = PackageURL.fromJSON(json)
-      expect(result.name).toBe('@scope/package-name.js')
-      expect(result.version).toBe('1.0.0-beta+build.123')
+      expect(result.name).toBe('package-name')
+      expect(result.namespace).toBe('@scope')
+      expect(result.version).toBe('1.0.0-beta.123')
     })
 
     it('should handle unicode in field values', () => {
       const json = JSON.stringify({
-        type: 'npm',
+        // Changed to generic type which allows unicode
+        type: 'generic',
         name: 'test-😀-package',
         version: '1.0.0',
         qualifiers: {

test/result.test.mts

@@ -333,7 +333,7 @@ describe('PackageURL Result methods', () => {
         undefined,
         undefined,
         undefined,
-        'Invalid JSON string',
+        'Failed to parse PackageURL from JSON',
       ],
       [
         'valid JSON with invalid purl data',