Skip to content

fix(core): csv delimiter, api security, serializers, and visibility#81

Open
pixincreate wants to merge 10 commits into
masterfrom
fix/csv-output-delimiter
Open

fix(core): csv delimiter, api security, serializers, and visibility#81
pixincreate wants to merge 10 commits into
masterfrom
fix/csv-output-delimiter

Conversation

@pixincreate

@pixincreate pixincreate commented Mar 15, 2026

Copy link
Copy Markdown
Owner

Type of Change

  • Bugfix
  • Enhancement
  • Refactoring
  • New feature
  • Dependency updates
  • Documentation
  • CI/CD

Description

P0 Bug Fixes

  1. CSV output ignores custom delimiter — serialize_csv() and escape_csv() now use the configured delimiter from ConvertOptions instead of hardcoded comma
  2. HTTP API DoS vulnerability — Added 10MB request body limit via DefaultBodyLimit::max()
  3. JSON control character escaping — Added \b, \f, and \uXXXX escapes per RFC 8259
  4. TOML key escaping — Keys with spaces, dots, brackets, quotes now properly quoted
  5. format_datetime error masking — Changed from unwrap_or_else with hardcoded dates to proper Result propagation

P1 Fixes

  1. XML empty element serialization — Empty elements now serialize as Null instead of empty Object
  2. XML round-trip instability — Single-key objects use key as root name to prevent double-wrapping

P2 Improvements

  1. TOML parser performance — Removed .get().cloned() anti-pattern in 4 functions, eliminating unnecessary Value clones
  2. escape_xml optimization — Single-pass implementation (5 allocations → 1)
  3. TOML nested table support — Proper [table] sections for nested objects instead of inline tables only

Code Quality

  1. Restored API encapsulation — Cursor is pub(crate), JsonParser internal methods are #[cfg(test)]
  2. Added builder methods — with_max_depth(), with_max_size() on JSON/TOML Config types
  3. Modularized zparse-api — Split into types.rs, handlers.rs, router.rs modules

Context

This PR addresses all P0 bugs and medium-priority issues identified in comprehensive code review. The changes improve correctness (CSV delimiter actually works), security (API body limits), and maintainability (proper encapsulation).

Related to code review findings:

  • CSV delimiter feature was broken on output (always used comma)
  • Internal APIs were exposed as pub solely for testing
  • Missing test coverage for CLI and HTTP API entry points

How did you test it?

  • Unit tests: 156 passing
  • CLI integration tests: 12 added using assert_cmd
  • API integration tests: 12 added using axum::test
  • Doctests: 1 passing
  • Total: 181 tests passing
cargo test --workspace --exclude zparse-wasm  # All pass
cargo clippy --workspace --exclude zparse-wasm -- -D warnings  # Clean
cargo fmt --all  # Clean
cargo build --package zparse-wasm --target wasm32-unknown-unknown  # Builds

Manual verification:

  • CSV custom delimiter round-trip (tab, pipe, semicolon)
  • HTTP API error responses return proper status codes
  • XML round-trip stability confirmed

Checklist

  • I formatted the code cargo +nightly fmt --all
  • I addressed lints thrown by cargo clippy
  • I reviewed the submitted code
  • I added unit tests for my changes where possible

@pixincreate
fix: address all P0 bugs and medium-priority issues from code review
58ff1e5 
- Fix CSV output ignoring custom delimiter (serialize_csv, escape_csv)
- Add 10MB request body limit to HTTP API (DoS prevention)
- Fix JSON escape_string missing control chars per RFC 8259
- Fix serialize_toml not escaping keys with special chars
- Fix format_datetime masking errors with hardcoded fallback dates
- Fix API csv_delimiter validation to return error on invalid input
- Fix Object::remove to use shift_remove (preserve insertion order)
- Fix Array::remove to return Option and bounds check
@pixincreate
fix: remaining code review issues (CORR-6/7, P1, P2 fixes)
af9d04e 
- API: Unified error responses with proper HTTP status codes (200/400/422)
- API: Consistent ApiResult<T> type for both parse and convert endpoints
- XML: Empty elements now serialize as Null instead of empty Object
- XML: Fix round-trip instability by using single-key Object as root name
- TOML: Refactor .get().cloned() anti-pattern in 4 functions (no more Value cloning)
- XML: Single-pass escape_xml implementation (1 allocation vs 5)
@pixincreate
feat: add comprehensive tests and TOML nested table support
129e25c 
- Add nested [table] section support to serialize_toml (proper TOML output)
- Create modular zparse-api structure (types, handlers, router modules)
- Add 12 API integration tests using axum::test
- Add 13 CLI integration tests using assert_cmd
- Fix test expectations to match actual CLI behavior
- All tests passing: 156 unit + 12 API + 13 CLI + 1 doctest
@pixincreate
refactor: restore internal API encapsulation
6d0b061 
- Make JsonParser::config(), depth(), bytes_parsed() #[cfg(test)]
- Add with_max_depth() and with_max_size() builder methods to Config
- Make Cursor pub(crate) with all methods internal
- Move Cursor tests from tests/ to inline #[cfg(test)] module
- Remove external lexer_cursor_tests.rs
- Make Cursor::consume #[cfg(test)] (only used in tests)
- All 177 tests pass, clippy clean, build clean
@pixincreate pixincreate self-assigned this Mar 15, 2026
@pixincreate pixincreate added bug Something isn't working enhancement New feature or request Refactor Code refactoring labels Mar 15, 2026
pixincreate added a commit that referenced this pull request Mar 15, 2026
@pixincreate
fix: address PR #81 review comments
03b5bf9 
- Make CORS configurable via ZPARSE_CORS_ORIGIN env var
  - * = allow any origin (dev mode)
  - specific value = allow that origin
  - unset = restrictive (no CORS headers)
- Simplify main.rs to use modular structure (lib/handlers/router)
- Remove datetime fallbacks in serialize_json and value_to_children
  - Now returns null or empty instead of hardcoded 1979 date
- All 181 tests pass, clippy clean, fmt clean
@pixincreate
fix: address PR #81 review comments
df32ebd 
- Make CORS configurable via ZPARSE_CORS_ORIGIN env var
  - * = allow any origin (dev mode)
  - specific value = allow that origin
  - unset = restrictive (no CORS headers)
- Simplify main.rs to use modular structure (lib/handlers/router)
- Remove datetime fallbacks in serialize_json and value_to_children
  - Now returns null or empty instead of hardcoded 1979 date
- All 181 tests pass, clippy clean, fmt clean
@pixincreate pixincreate force-pushed the fix/csv-output-delimiter branch 2 times, most recently from 9259637 to aba3d3e Compare March 15, 2026 11:39
@pixincreate
refactor: use native zparse parsers in API, remove double JSON conver…
8b7330f 
…sion

- Parse endpoint now uses native zparse parsers directly:
  - JsonParser for JSON/JSONC
  - CsvParser for CSV
  - from_toml_str for TOML
  - from_yaml_str for YAML
  - zparse::convert for XML (XmlDocument handling)
- Remove serde_json::from_str double conversion
- Only use serde_json for final HTTP response serialization
- All 181 tests pass
@pixincreate
refactor(api): remove serde_json, add native JSON response types
bfac8c3 
- Remove serde_json from production dependencies
- Add native JsonResponse and JsonError types in new json.rs module
- Export json module from lib.rs
@pixincreate
refactor(api): update handlers to use native JSON serialization
48f2071 
- Use AxumJson for request parsing
- Use custom JsonResponse/JsonError for responses
- Remove ApiResponse type alias from router
@pixincreate
fix(api-tests): add serde_json to dev-dependencies
2df1c7b 
- Move serde_json to dev-dependencies for test usage
@pixincreate pixincreate force-pushed the fix/csv-output-delimiter branch from aba3d3e to c4e58a2 Compare March 15, 2026 11:42
@pixincreate
refactor: export serializers from zparse library
217f4df 
- Add to_json_string, to_toml_string, to_yaml_string, to_csv_string to convert.rs
- Export serializers in lib.rs for public use
- Update API to use zparse serializers instead of custom implementation
@pixincreate pixincreate force-pushed the fix/csv-output-delimiter branch from 2dbda38 to 217f4df Compare March 15, 2026 12:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@pixincreate pixincreate

Labels

bug Something isn't working enhancement New feature or request Refactor Code refactoring

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@pixincreate