Commit Graph
5 Commits
Author SHA1 Message Date
Brad ArantandClaude Sonnet 5 94d4a30e88 Fix CGI form/cookie data handling and CGI URL encode/decode
- CGIFormData: fix broken setSize()+read() double-length bug that
  corrupted every parsed body with a leading run of garbage bytes
  (setSize(contentLength) pre-set length before read()/write()
  appended on top of it); switch content-type checks from exact
  equality to startsWith() since a real multipart Content-Type
  always carries a boundary= parameter; implement the previously
  empty multipart/form-data branch (ordinary fields only, file
  uploads deferred to a separate upload mechanism per TODO.txt);
  restore operator[] real lookup logic (was hardcoded to echo the
  key back); default-initialize formdata to NULL; fix mismatched
  include guard (leftover from an IMFFormData.h copy-paste); drop
  the unused IMFHeader.h include that pulled in a broken,
  unrelated StreamReader.h dependency.
- CGIFormattedData: parameterize the split delimiter (was hardcoded
  to &, which silently broke cookie parsing since HTTP_COOKIE uses
  ; not &) and trim whitespace on each split token.
- MString::toCGI()/fromCGI(): were unimplemented stubs (always
  returned an empty string); implemented proper CGI percent-encode
  / percent-decode + space plus-encoding, and fixed the one caller
  that discarded fromCGIs return value instead of using it.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-22 21:37:10 -07:00
Brad Arant 28feaff3d7 Rewrite JString for real JSON compliance
The previous implementation was a narrow path-accessor, not a JSON parser:
numbers/booleans/null were unhandled entirely (self-acknowledged by a
// TODO: Handle numbers... comment), string escape sequences were never
encoded or decoded, updating an existing non-string-valued key threw a
hard-coded Fix me exception, and skipping past an uninteresting key threw
Unrecognized data type for anything but string/array/object. Full
analysis given to the user before this rewrite; nothing here was guessed.

Added:
- JSONType enum + getType()/exists()/isNull() for type introspection.
- Typed read access: getString() (escape-decoded), getNumber(), getInt(),
  getBool() - each validates the value at the path is actually that type.
- Typed write access: setString() (escapes automatically), setNumber(),
  setInt(), setBool(), setNull() - and Proxy operator= overloads for
  double/int/long/bool so  produces a real JSON number, not
  a quoted string the way the old single MString-typed operator= did.
- jsonEscape()/jsonUnescape(): full escape codec including \uXXXX and
  UTF-16 surrogate pairs (astral characters, e.g. emoji) decoded to
  correct UTF-8.
- skipValue()/skipString()/skipContainer()/skipNumber() and their
  capture (span-returning) counterparts: escape-aware structural
  skipping, so a string value containing a literal { } [ ] , or : can
  no longer desync object/array nesting depth - this affected both
  locate()s internal key-skipping and find()s array/object extraction
  (the latter still relied on the base ZString::getContainer(), which
  has this exact bug and was not touched - see the ZString commit in
  this same session - so a JString-specific escape-aware capture was
  added instead of depending on it for this purpose).
- set() rewritten to use skipValue() to correctly locate and replace an
  EXISTING values full span regardless of its current type, instead of
  only handling the case where the existing value was already a quoted
  string.

Bugs found and fixed along the way, surfaced by writing real tests
against realistic JSON rather than just getting it to compile:
- The array-index-then-further-path case (e.g. b[2].c) always returned
  true immediately after reaching the index, so any path segment past
  the index was silently ignored - dead code after an unconditional
  return/throw meant the intended continuation logic could never run.
- The array selector syntax (label[key=value]) was only reachable as a
  top-level special case, never as part of a normal nested path (e.g.
  items[id=x].val) - unified both under the same bracket handling.
- Once reachable, the selector-match subkey check (find(key) on each
  array element) always searched from the document root, not the
  current array element, because find() unconditionally calls reset().
  Split into public find() (resets) and internal findRelative() (does
  not), and pointed both selector-matching call sites at the latter.
- path[0].getTokenExclude(...) in the array-selector path parser
  indexed into a list vector that split() never populated - undefined
  behavior. Removed the erroneous [0].

Verified via a dedicated test program (not included in this commit)
covering: string escape round-trip, numbers, booleans, null, exists(),
\uXXXX including a real surrogate pair decoded to correct 4-byte UTF-8,
nested dotted+bracketed navigation, a string value containing every
structural JSON character, replacing an existing key with a different
type, multi-key documents, pretty() not corrupting structural characters
inside a string, and the array selector syntax on both a matching first
and second element - 30/30 assertions pass. Full CoreUtils compile pass
unchanged apart from JString.cpp itself now succeeding (14/20 clean, same
7 pre-existing StreamReader.h-blocked failures as always, nothing new
broken).
2026-08-22 20:53:54 -07:00
Brad Arant 14697e2af2 Fix three real bugs found during a full ZString review
operator< computed end2 as valuex.getData() + valuex.getLength() - mixing
an absolute pointer (getData(), the buffer start) with a cursor-relative
length (getLength(), remaining-from-cursor). For any ZString with a
non-zero cursor offset this produces an end pointer short of the true
buffer end by exactly the amount already consumed, truncating the
comparison early. Fixed to use valuex.data + valuex.length (the true
end), matching how end1 was already computed for *this. operator<=,
>=, and > were separately broken (flagged by the codes own TODO
comment) - they compared via strncmp from data (ignoring cursor
entirely), inconsistent with operator< and with each other. Replaced
all three with derivations from the now-correct operator< (a <= b is
!(b < a), etc.) - guarantees trichotomy by construction instead of
three independently-written comparison loops.

substring(start, len): when the requested length overran the buffer,
computed newlen as endChar - end (the overrun amount) instead of
end - startChar (the actual remaining valid length) - backwards, and
stored into a char (max 127), silently wrapping for longer strings.
Fixed the formula and widened to size_t.

trimCRLF(): read *(data + length - 1) unconditionally, including when
length is already 0 - one byte before the buffer on an empty string.
Guarded both checks on length > 0.

Verified: full local compile (identical result to before - 13/20 clean,
same pre-existing StreamReader.h-blocked failures, nothing new broken);
a dedicated runtime test exercising all three fixes plus a trichotomy
check across multiple cursor offsets - all 13 assertions pass. Test
file not included in this commit.
2026-08-22 20:11:13 -07:00
Brad Arant 4082e82466 Fix Exception default file/line always reporting Exception.h itself
__FILE__/__LINE__ are preprocessor macros - as default argument values
in the header they expand once, at the header declaration site, not at
each call site. Every throw relying on the defaults (all but File.cpp,
which passed them explicitly) reported Exception.h itself instead of
the real error location.

Switched the defaults to std::source_location::current(), which the
C++20 standard specifically defines to evaluate at the call site when
used as a default argument - unlike macros. Kept the same 4-parameter
signature so every existing call site, including ones that pass
__FILE__/__LINE__ explicitly, is unaffected.

Verified two ways: full-repo compile pass (identical result to before,
13/20 clean, same pre-existing StreamReader.h-blocked failures, nothing
new broken); a real runtime test - a throw relying on defaults from a
known line correctly reported that exact file/line via the caught
exceptions .file/.line members. Test file not included in this commit.
2026-08-22 19:52:44 -07:00
Brad Arant 29c9a5b855 Fix MString::fromHex() silently swallowing the odd-length-input error
The Exception on the odd-length-input path was constructed but never
thrown (missing throw keyword) - the temporary was destroyed with no
effect, so execution fell through to an out-of-bounds nextChar() read
past the exhausted buffer instead of raising an error. Added the
missing throw.

Verified via full local compile: MString.cpp builds clean standalone;
a full-repo pass shows only the pre-existing, already-known
StreamReader.h-missing failures (7 files, unrelated to this change),
confirmed via each error log individually.
2026-08-22 19:52:35 -07:00