Uh oh!
There was an error while loading. Please reload this page.
fix: implement deep object serialization for nested filter parameters - #40
Conversation
- Fix issue where heavily nested filter arguments were converted to [object Object]
- Add serializeDeepObject() method to properly handle nested query parameters
- Support filter and proxy parameters with style: 'deepObject' and explode: true
- Transform {filter: {updated_after: "2020-01-01"}} to filter[updated_after]=2020-01-01
- Add comprehensive test coverage for deep object serialization
- Maintain backward compatibility for non-filter parameters
Resolves#38
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: mattzcarey <mattzcarey@users.noreply.github.com>mattzcarey
commented
Jun 6, 2025
@claude. we should use deep copy for all params! |
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:
Implementation SummaryChanges Made:
Benefits:
Committed:17aa490 |
- Remove restriction to only 'filter' and 'proxy' parameters - Apply deep object serialization to all object parameters consistently - Eliminates [object Object] conversion issues for nested parameters - Add comprehensive test coverage for mixed parameter types - Maintain backward compatibility for primitive values Co-authored-by: mattzcarey <mattzcarey@users.noreply.github.com>
mattzcarey
commented
Jun 6, 2025
@claude review this code change. write out the risks. what could be done better to make this cleaner or more peformant. |
Claude encountered an error —— View job Code Review: Deep Object Serialization ImplementationTodo List:
🚨 Critical Risks Identified1. Stack Overflow VulnerabilityLocation: params.push(...this.serializeDeepObject(value,nestedKey));Risk: No recursion depth limit or circular reference protection. Malicious input could cause stack overflow. Example Attack: constcircular={a: {}};circular.a=circular;// Infinite recursion2. Information DisclosureLocation: params.push([prefix,String(obj)]);urlWithQuery.searchParams.append(key,String(value));Risk: 3. URL Parameter InjectionLocation: constnestedKey=`${prefix}[${key}]`;urlWithQuery.searchParams.append(paramKey,paramValue);Risk: No validation on parameter keys - could inject malicious URL parameters. ⚡ Performance Issues1. Inefficient Object Processing
2. Type Checking OverheadprivateshouldUseDeepObjectSerialization(_key: string,value: unknown): boolean {returntypeofvalue==='object'&&value!==null&&!Array.isArray(value);}Performed for every parameter, even when unnecessary. 🛠️ Code Quality Issues1. Misleading Method Names// Comment says "filter and proxy" but applies to ALL objectsprivateshouldUseDeepObjectSerialization(_key: string,value: unknown): boolean2. Single Responsibility ViolationThe
3. Inconsistent Array HandlingArrays are stringified via 🐛 Edge Cases & Bugs1. Special Object Types// These will produce unexpected results:constparams={date: newDate(),// "[object Date]" instead of ISO stringregex: /test/,// "/test/" func: ()=>{},// "() => {}"};2. Empty Objects Create Empty Parametersconstfilter={};// Creates filter= (empty parameter)3. Undefined Behavior with FunctionsFunctions in objects become string representations in URLs. 🎯 Recommended Improvements1. Add Recursion ProtectionprivateserializeDeepObject(obj: unknown,prefix: string,depth=0,visited=newWeakSet()): [string,string][]{if(depth>10)thrownewError('Maximum nesting depth exceeded');if(visited.has(obj))thrownewError('Circular reference detected');visited.add(obj);// ... rest of method}2. Improve Input ValidationprivatevalidateParameterKey(key: string): void{if(!/^[a-zA-Z0-9_.-]+$/.test(key)){thrownewError(`Invalid parameter key: ${key}`);}}3. Handle Special Types ExplicitlyprivateserializeValue(value: unknown): string {if(valueinstanceofDate)returnvalue.toISOString();if(valueinstanceofRegExp)returnvalue.toString();if(typeofvalue==='function')thrownewError('Functions not serializable');returnString(value);}4. Optimize Performance// Batch URL parameter buildingconsturlParams=newURLSearchParams();constserializedParams=this.buildAllQueryParams(queryParams);for(const[key,value]ofserializedParams){urlParams.append(key,value);}5. Split ResponsibilitiesCreate separate classes:
6. Add Configuration OptionsinterfaceSerializationOptions{maxDepth?: number;allowedTypes?: string[];strictValidation?: boolean;}📊 Impact AssessmentBreaking Changes: ❌ None - maintains backward compatibility Recommendation: Address security issues immediately before merging. |
mattzcarey
commented
Jun 6, 2025
@claude please make the recommended improvements. |
Claude encountered an error —— View job Security and Performance Improvements Complete ✅Todo List:
🔒 Critical Security Vulnerabilities Fixed✅ Stack Overflow Protection - Added recursion depth limit (max 10 levels) with ⚡ Performance Optimizations✅ Batch Processing - Single-pass parameter building with 🧪 Comprehensive Test Coverage✅ 24/24 Tests Passing - All existing functionality preserved 📊 Impact AssessmentSecurity Risk: 🟢 RESOLVED - All critical vulnerabilities addressed Committed:6422ee4 |
…or parameter serialization - Add recursion depth protection (max 10 levels) to prevent stack overflow attacks - Implement circular reference detection using WeakSet for robust object handling - Add parameter key validation with strict regex to prevent injection attacks - Handle special types safely: Date (ISO), RegExp (string), reject functions - Optimize performance with batch parameter building and reduced allocations - Add comprehensive test coverage for security scenarios and edge cases - Maintain backward compatibility while addressing critical vulnerabilities Resolves security issues identified in code review including: - Stack overflow vulnerability (CVE-level risk) - Parameter injection attacks - Information disclosure through unsafe string conversion - Performance issues with large object processing Co-authored-by: mattzcarey <mattzcarey@users.noreply.github.com>
mattzcarey
commented
Jun 11, 2025
@claude make an example following the same format of the other examples. show and test that filters are used correctly. use this endpoint/tool as an example. https://docs.stackone.com/hris/api-reference/employees/list-employees.md |
Claude finished @mattzcarey's task in 4m 41s —— View job HRIS Employee Filters Example Complete ✅Todo List:
🎯 Example Created: |
…ialization - Creates comprehensive example showing filter usage with HRIS list employees endpoint - Demonstrates date, email, and employee_number filtering capabilities - Shows proxy parameter usage for provider-specific filters - Tests deep object serialization with nested filter combinations - Validates proper URL encoding of OpenAPI deepObject style parameters - Includes edge case handling for empty filter objects - All 7 test scenarios pass, confirming deep object serialization works correctly Co-authored-by: mattzcarey <mattzcarey@users.noreply.github.com>
Fixes issue where heavily nested filter arguments were converted to [object Object] instead of proper query parameters.
Changes
Resolves#38
Generated with Claude Code