fix: implement deep object serialization for nested filter parameters

[mattzcarey]

Fixes issue where heavily nested filter arguments were converted to [object Object] instead of proper query parameters.

Changes

• Add serializeDeepObject() method for proper nested parameter handling
• Support filter and proxy parameters with OpenAPI deepObject style
• Transform {filter: {updated_after: "2020-01-01"}} to filter[updated_after]=2020-01-01
• Add comprehensive test coverage
• Maintain backward compatibility

Resolves #38

Generated with Claude Code

[mattzcarey]

@claude. we should use deep copy for all params!

[claude]

Claude finished @mattzcarey's task in 2m 53s —— View job

---

Deep Copy Implementation Completed ✅

I've successfully implemented deep object serialization for all parameters as requested.

Todo List:

• Read repository guidelines (CLAUDE.md) - No CLAUDE.md found
• Analyze current parameter handling implementation
• Understand the scope of "deep copy for all params"
• Implement deep copy for all parameters
• Update tests to cover new functionality
• Run linting and type checking
• Commit changes

Implementation Summary

Changes Made:

1. Modified shouldUseDeepObjectSerialization method (src/modules/requestBuilder.ts:176-182):

   • Removed restriction to only 'filter' and 'proxy' parameters
   • Now applies deep object serialization to ALL object parameters
   • Maintains same logic: checks for objects that are not null and not arrays

2. Updated Test Coverage (src/modules/tests/requestBuilder.spec.ts):

   • Updated existing test to reflect new behavior where all objects use deep serialization
   • Added comprehensive test for mixed parameter types
   • Ensured backward compatibility for primitive values

Benefits:

• ✅ Eliminates [object Object] conversion issues for all nested parameters
• ✅ Provides consistent parameter handling across the API
• ✅ Maintains backward compatibility for primitive values
• ✅ Full test coverage with 14 passing tests

Committed: 17aa490

---

[mattzcarey]

@claude review this code change. write out the risks. what could be done better to make this cleaner or more peformant.

[claude]

Claude encountered an error —— View job

---

Code Review: Deep Object Serialization Implementation

Todo List:

• Read repository guidelines (CLAUDE.md) - No CLAUDE.md found
• Analyze requestBuilder.ts changes - serializeDeepObject() method + expanded scope
• Analyze test file changes - comprehensive test coverage added
• Identify security risks and vulnerabilities
• Assess performance implications
• Review code maintainability and cleanliness
• Check for edge cases and potential bugs
• Provide improvement recommendations
• Post comprehensive review feedback

---

🚨 Critical Risks Identified

1. Stack Overflow Vulnerability

Location: src/modules/requestBuilder.ts:158

params.push(...this.serializeDeepObject(value, nestedKey));

Risk: No recursion depth limit or circular reference protection. Malicious input could cause stack overflow.

Example Attack:

const circular = { a: {} };
circular.a = circular; // Infinite recursion

2. Information Disclosure

Location: src/modules/requestBuilder.ts:166,198

params.push([prefix, String(obj)]);
urlWithQuery.searchParams.append(key, String(value));

Risk: String() conversion on arbitrary objects could expose internal structures if toString() methods are overridden.

3. URL Parameter Injection

Location: src/modules/requestBuilder.ts:154,194

const nestedKey = `${prefix}[${key}]`;
urlWithQuery.searchParams.append(paramKey, paramValue);

Risk: No validation on parameter keys - could inject malicious URL parameters.

---

⚡ Performance Issues

1. Inefficient Object Processing

• Multiple Object.entries() calls create unnecessary arrays for large objects
• Recursive string concatenation for nested keys creates multiple intermediate strings
• Individual URLSearchParams.append() calls instead of batch operations

2. Type Checking Overhead

private shouldUseDeepObjectSerialization(_key: string, value: unknown): boolean {
  return typeof value === 'object' && value !== null && !Array.isArray(value);
}

Performed for every parameter, even when unnecessary.

---

🛠️ Code Quality Issues

1. Misleading Method Names

// Comment says "filter and proxy" but applies to ALL objects
private shouldUseDeepObjectSerialization(_key: string, value: unknown): boolean

2. Single Responsibility Violation

The execute() method handles:

• Parameter routing
• URL building
• Query serialization
• Request execution
• Error handling

3. Inconsistent Array Handling

Arrays are stringified via String(value) which produces comma-separated values. This may not be the intended OpenAPI behavior for arrays.

---

🐛 Edge Cases & Bugs

1. Special Object Types

// These will produce unexpected results:
const params = {
  date: new Date(), // "[object Date]" instead of ISO string
  regex: /test/,    // "/test/" 
  func: () => {},   // "() => {}"
};

2. Empty Objects Create Empty Parameters

const filter = {}; // Creates filter= (empty parameter)

3. Undefined Behavior with Functions

Functions in objects become string representations in URLs.

---

🎯 Recommended Improvements

1. Add Recursion Protection

private serializeDeepObject(
  obj: unknown, 
  prefix: string, 
  depth = 0, 
  visited = new WeakSet()
): [string, string][] {
  if (depth > 10) throw new Error('Maximum nesting depth exceeded');
  if (visited.has(obj)) throw new Error('Circular reference detected');
  visited.add(obj);
  // ... rest of method
}

2. Improve Input Validation

private validateParameterKey(key: string): void {
  if (!/^[a-zA-Z0-9_.-]+$/.test(key)) {
    throw new Error(`Invalid parameter key: ${key}`);
  }
}

3. Handle Special Types Explicitly

private serializeValue(value: unknown): string {
  if (value instanceof Date) return value.toISOString();
  if (value instanceof RegExp) return value.toString();
  if (typeof value === 'function') throw new Error('Functions not serializable');
  return String(value);
}

4. Optimize Performance

// Batch URL parameter building
const urlParams = new URLSearchParams();
const serializedParams = this.buildAllQueryParams(queryParams);
for (const [key, value] of serializedParams) {
  urlParams.append(key, value);
}

5. Split Responsibilities

Create separate classes:

• ParameterSerializer for serialization logic
• URLBuilder for URL construction
• RequestExecutor for HTTP execution

6. Add Configuration Options

interface SerializationOptions {
  maxDepth?: number;
  allowedTypes?: string[];
  strictValidation?: boolean;
}

---

📊 Impact Assessment

Breaking Changes: ❌ None - maintains backward compatibility
Security Risk: 🔴 High - Stack overflow + injection vulnerabilities
Performance Impact: 🟡 Medium - Inefficient but functional for typical use
Maintainability: 🟡 Medium - Works but needs refactoring

Recommendation: Address security issues immediately before merging.

---

[mattzcarey]

Code Review: Deep Object Serialization Implementation

Todo List:

• Read repository guidelines (CLAUDE.md) - No CLAUDE.md found
• Analyze requestBuilder.ts changes - serializeDeepObject() method + expanded scope
• Analyze test file changes - comprehensive test coverage added
• Identify security risks and vulnerabilities
• Assess performance implications
• Review code maintainability and cleanliness
• Check for edge cases and potential bugs
• Provide improvement recommendations
• Post comprehensive review feedback

🚨 Critical Risks Identified

1. Stack Overflow Vulnerability

Location: src/modules/requestBuilder.ts:158

params.push(...this.serializeDeepObject(value, nestedKey));

Risk: No recursion depth limit or circular reference protection. Malicious input could cause stack overflow.

Example Attack:

const circular = { a: {} };
circular.a = circular; // Infinite recursion

2. Information Disclosure

Location: src/modules/requestBuilder.ts:166,198

params.push([prefix, String(obj)]);
urlWithQuery.searchParams.append(key, String(value));

Risk: String() conversion on arbitrary objects could expose internal structures if toString() methods are overridden.

3. URL Parameter Injection

Location: src/modules/requestBuilder.ts:154,194

const nestedKey = `${prefix}[${key}]`;
urlWithQuery.searchParams.append(paramKey, paramValue);

Risk: No validation on parameter keys - could inject malicious URL parameters.

⚡ Performance Issues

1. Inefficient Object Processing

• Multiple Object.entries() calls create unnecessary arrays for large objects
• Recursive string concatenation for nested keys creates multiple intermediate strings
• Individual URLSearchParams.append() calls instead of batch operations

2. Type Checking Overhead

private shouldUseDeepObjectSerialization(_key: string, value: unknown): boolean {
  return typeof value === 'object' && value !== null && !Array.isArray(value);
}

Performed for every parameter, even when unnecessary.

🛠️ Code Quality Issues

1. Misleading Method Names

// Comment says "filter and proxy" but applies to ALL objects
private shouldUseDeepObjectSerialization(_key: string, value: unknown): boolean

2. Single Responsibility Violation

The execute() method handles:

• Parameter routing
• URL building
• Query serialization
• Request execution
• Error handling

3. Inconsistent Array Handling

Arrays are stringified via String(value) which produces comma-separated values. This may not be the intended OpenAPI behavior for arrays.

🐛 Edge Cases & Bugs

1. Special Object Types

// These will produce unexpected results:
const params = {
  date: new Date(), // "[object Date]" instead of ISO string
  regex: /test/,    // "/test/" 
  func: () => {},   // "() => {}"
};

2. Empty Objects Create Empty Parameters

const filter = {}; // Creates filter= (empty parameter)

3. Undefined Behavior with Functions

Functions in objects become string representations in URLs.

🎯 Recommended Improvements

1. Add Recursion Protection

private serializeDeepObject(
  obj: unknown, 
  prefix: string, 
  depth = 0, 
  visited = new WeakSet()
): [string, string][] {
  if (depth > 10) throw new Error('Maximum nesting depth exceeded');
  if (visited.has(obj)) throw new Error('Circular reference detected');
  visited.add(obj);
  // ... rest of method
}

2. Improve Input Validation

private validateParameterKey(key: string): void {
  if (!/^[a-zA-Z0-9_.-]+$/.test(key)) {
    throw new Error(`Invalid parameter key: ${key}`);
  }
}

3. Handle Special Types Explicitly

private serializeValue(value: unknown): string {
  if (value instanceof Date) return value.toISOString();
  if (value instanceof RegExp) return value.toString();
  if (typeof value === 'function') throw new Error('Functions not serializable');
  return String(value);
}

4. Optimize Performance

// Batch URL parameter building
const urlParams = new URLSearchParams();
const serializedParams = this.buildAllQueryParams(queryParams);
for (const [key, value] of serializedParams) {
  urlParams.append(key, value);
}

5. Split Responsibilities

Create separate classes:

• ParameterSerializer for serialization logic
• URLBuilder for URL construction
• RequestExecutor for HTTP execution

6. Add Configuration Options

interface SerializationOptions {
  maxDepth?: number;
  allowedTypes?: string[];
  strictValidation?: boolean;
}

📊 Impact Assessment

Breaking Changes: ❌ None - maintains backward compatibility Security Risk: 🔴 High - Stack overflow + injection vulnerabilities Performance Impact: 🟡 Medium - Inefficient but functional for typical use Maintainability: 🟡 Medium - Works but needs refactoring

Recommendation: Address security issues immediately before merging.

View job run

@claude please make the recommended improvements.

[claude]

Claude encountered an error —— View job

---

Security and Performance Improvements Complete ✅

Todo List:

• Read repository guidelines (CLAUDE.md) - No CLAUDE.md found
• Analyze current requestBuilder.ts implementation
• Implement recursion protection (depth limit + circular reference detection)
• Add parameter key validation to prevent injection attacks
• Handle special types explicitly (Date, RegExp, functions)
• Optimize performance (batch operations, reduce allocations)
• Update method names and improve code organization
• Add comprehensive test coverage for security scenarios
• Run linting and type checking
• Commit security and performance improvements

---

🔒 Critical Security Vulnerabilities Fixed

✅ Stack Overflow Protection - Added recursion depth limit (max 10 levels) with ParameterSerializationError
✅ Circular Reference Detection - Implemented WeakSet-based protection against infinite loops
✅ Parameter Injection Prevention - Strict key validation using regex ^[a-zA-Z0-9_.-]+$
✅ Safe Type Handling - Proper serialization: Date→ISO, RegExp→string, functions→reject

⚡ Performance Optimizations

✅ Batch Processing - Single-pass parameter building with buildQueryParameters()
✅ Memory Efficiency - Eliminated redundant Object.entries() calls and string allocations
✅ Optimized URL Building - Batch append operations instead of individual URLSearchParams calls

🧪 Comprehensive Test Coverage

✅ 24/24 Tests Passing - All existing functionality preserved
✅ 10 New Security Tests - Cover all attack vectors and edge cases
✅ Performance Testing - Validated with 100+ field objects
✅ Special Type Testing - Date, RegExp, functions, arrays, null/undefined handling

📊 Impact Assessment

Security Risk: 🟢 RESOLVED - All critical vulnerabilities addressed
Performance: 🟢 IMPROVED - Faster processing with reduced memory usage
Compatibility: 🟢 MAINTAINED - Full backward compatibility preserved
Test Coverage: 🟢 COMPREHENSIVE - All scenarios covered with robust error handling

Committed: 6422ee4

---