╭─── Claude Code v2.1.132 ─────────────────────────────────────────────────────╮
│                                                    │ Tips for getting        │
│                 Welcome back Gabr!                 │ started                 │
│                                                    │ Run /init to create a … │
│                       ▐▛███▜▌                      │ ─────────────────────── │
│                      ▝▜█████▛▘                     │ What's new              │
│                        ▘▘ ▝▝                       │ Added `CLAUDE_CODE_SES… │
│    Sonnet 4.6 · Claude Max · gabr42@gmail.com's    │ Added `CLAUDE_CODE_DIS… │
│    Organization                                    │ Added a "Pasting…" foo… │
│           c:\0\DelphiMVCFramework\sources          │ /release-notes for more │
╰──────────────────────────────────────────────────────────────────────────────╯

✻ Conversation compacted (ctrl+o for history)

  ⎿  Read ..\CLAUDE.md (142 lines)
  ⎿  Referenced file MVCFramework.Serializer.Streaming.pas
  ⎿  Read MVCFramework.Serializer.JsonDataObjects.OptionalCustomTypes.pas (182 lines)
  ⎿  Referenced file MVCFramework.Serializer.JsonDataObjects.CustomTypes.pas
  ⎿  Read MVCFramework.Serializer.Abstract.pas (371 lines)

● Bug Audit: MVCFramework.Serializer*.pas — Full Report

  ---
  Category 1: Resource / Memory Lifecycle

  ---
  [L1-A] USE-AFTER-FREE — crash-level
  MVCFramework.Serializer.JsonDataObjects.pas ~line 1820

  GetMem(lBuff, lInnerType.TypeSize);
  JSONObjectToRecord(lJItem, lInnerTypeAsRecord, lBuff);
  TValue.MakeWithoutCopy(lBuff, lInnerType.Handle, lArr[I]);  // TValue takes 
  ownership of lBuff
  FreeMem(lBuff, lInnerType.TypeSize);                        // frees the 
  memory TValue now holds

  Failure mode: TValue.MakeWithoutCopy with a heap pointer transfers ownership —
   the TValue holds and will manage that pointer. The immediately following
  FreeMem releases the same block. Any subsequent read of lArr[I] or its managed
   fields reads freed memory. Crash severity; may be masked by heap luck in
  debug builds but will corrupt in release.

  Fix: Remove the FreeMem call. Do not free lBuff after MakeWithoutCopy; the
  TValue owns it. If you need deterministic control, use TValue.Make (copies)
  and then free lBuff.

  ---
  [L1-B] BUFFER LEAK on non-EInvalidCast exceptions
  MVCFramework.Serializer.JsonDataObjects.pas ~lines 2525–2635
  (JSONObjectToRecord / JSONObjectToRecordStatic)

  GetMem(Buffer, lTypeSize);
  try
    // ... loop over fields ...
  except
    on E: EInvalidCast do
    begin
      FreeMem(Buffer, lTypeSize);
      raise;
    end;
  end;
  // no finally; non-EInvalidCast exceptions (EConvertError, EMVCException, 
  etc.) skip FreeMem

  Failure mode: Any exception other than EInvalidCast thrown inside the loop
  escapes without freeing Buffer. Since this is called for every nested record
  during deserialization, repeated errors (e.g., type mismatches that throw
  EConvertError) accumulate leaks per request.

  Fix: Replace the except handler with a try/finally FreeMem(Buffer, lTypeSize) 
  end wrapper, and re-raise inside the except block only for the error you want
  to log/wrap.

  ---
  [L1-C] ENCODING OBJECT LEAK
  MVCFramework.Serializer.Commons.pas ~lines 641–656 (DeSerializeStringStream)

  Encoding := TEncoding.GetEncoding(AEncoding);  // allocates a new TEncoding
  SS := TStringStream.Create(ASerializedString, Encoding);
  try
    // ...
  finally
    SS.Free;  // frees the stream, NOT Encoding
  end;

  Failure mode: TEncoding.GetEncoding always returns a newly allocated object
  (for non-standard encodings) that the caller owns. It is never freed. Called
  on every non-default deserialization of a string stream — once per JSON field
  that goes through this path. Accumulates silently.

  Fix:
  Encoding := TEncoding.GetEncoding(AEncoding);
  try
    SS := TStringStream.Create(ASerializedString, Encoding, False);
    // ... use SS ...
    SS.Free;
  finally
    Encoding.Free;
  end;
  Or use TEncoding.GetEncoding only for the standard encodings that return
  singleton instances, and free only non-standard ones.

  ---
  [L1-D] THREADVAR OBJECTS NEVER FREED
  MVCFramework.Serializer.Streaming.pas ~lines 235–236

  threadvar
    GStreamWriter: TStreamWriter;
    GJsonWriter  : TJsonTextWriter;

  Failure mode: These per-thread singletons are allocated on first use and never
   released for any thread other than the finalizer thread. For a server that
  uses thread pools (Indy, etc.) with bounded or recycled threads, this is a
  bounded leak per unique thread — but for systems with dynamic thread creation
  or long-lived servers, it accumulates. ClassDestroy runs only on the
  main/finalizer thread and cannot reach other threads' threadvar slots.

  Fix: Provide a thread cleanup procedure (e.g., FreeStreamingThreadLocals) and
  either call it from the server's OnIdle/thread teardown hook, or switch from
  threadvar to an IInterface-wrapped holder that frees itself when the reference
   count drops to zero.

  ---
  [L1-E] PARENT JSON OBJECT LEAKED / CHILD DOUBLE-FREED
  MVCFramework.Serializer.JsonDataObjects.pas ~lines 1126–1210
  (DeserializeCollection, ARootNode branch)

  JSONObject := TJDOJsonObject.ParseWithExceptions(lBody) as TJDOJsonObject;
  JSONArray  := JSONObject.A[ARootNode];   // child reference; owned by 
  JSONObject
  try
    // ... iterate JSONArray ...
  finally
    JSONArray.Free;   // frees the child object — JSONObject still holds a 
  reference to it
  end;
  // JSONObject is never freed → memory leak + JSONObject still points to freed 
  child

  Failure mode: JSONObject.A[...] returns a reference to a child node; that
  child is owned (and will be freed) by the parent JSONObject. Calling
  JSONArray.Free manually frees the child while the parent still references it
  (double-free on JSONObject.Free which never happens anyway). JSONObject itself
   is never freed — full subtree leak on every deserialization call that uses a
  root node.

  Fix:
  JSONObject := TJDOJsonObject.ParseWithExceptions(lBody) as TJDOJsonObject;
  try
    JSONArray := JSONObject.A[ARootNode];   // child; DO NOT free separately
    // ... iterate JSONArray ...
  finally
    JSONObject.Free;   // frees parent and all children
  end;

  ---
  Category 2: Copy-Paste Bugs

  ---
  [L2-A] CLEAR CALLED ON TEMPORARY TValue COPY — silent no-op, widespread
  MVCFramework.Serializer.Commons.pas ~lines 1451–1682
  (MapDataSetFieldToNullableRTTIField) and ~lines 1684–1869
  (MapDataSetFieldToNullableRTTIProperty)

  // For null dataset fields, intended to clear the nullable:
  if AField.IsNull then
    aRTTIField.GetValue(AObject).AsType<NullableInt32>().Clear;
    // GetValue() → returns a TValue by value (a copy of the record)
    // AsType<>()  → returns another copy of the NullableInt32
    // .Clear      → mutates that temporary copy, which is then discarded

  This pattern repeats for every nullable type (~23 variants): NullableInt32,
  NullableInt64, NullableUInt32, NullableCurrency, NullableBoolean,
  NullableString, NullableDateTime, etc.

  Failure mode: When a dataset field is NULL, the corresponding nullable on the
  object is never cleared. Whatever value was in the nullable before
  deserialization remains. Deserialized objects silently carry stale data. No
  crash, no exception — just wrong results on null columns.

  Fix: Use GetReferenceToRawData to obtain a pointer to the actual record, then
  call Clear through it:

  if AField.IsNull then
  begin
    lValue := aRTTIField.GetValue(AObject);
    PNullableInt32(lValue.GetReferenceToRawData)^.Clear;
    // SetValue not needed — GetReferenceToRawData points into the live object's
   memory
  end;
  Or use SetValue after building a cleared copy:
  var lNullable: NullableInt32;
  lNullable.Clear;
  aRTTIField.SetValue(AObject, TValue.From<NullableInt32>(lNullable));

  ---
  [L2-B] LOOP VARIABLE TYPO — always inspects element [0]
  MVCFramework.Serializer.JsonDataObjects.pas ~line 1295 (inside
  JsonArrayToArray)

  for I := 0 to Pred(AJsonArray.Count) do
  begin
    case AJsonArray.types[0] of   // BUG: should be types[I]
      jdtString:  ...
      jdtInteger: ...
      ...
    end;
  end;

  Failure mode: For any JSON array where elements are not all the same type as
  element [0], wrong branch is taken — e.g., a numeric element is treated as
  string type, or vice versa. Array deserialization silently produces wrong
  values or raises conversion errors. Only arrays that are homogeneous (all
  elements matching the type of index 0) work correctly.

  Fix: Change AJsonArray.types[0] → AJsonArray.types[I].

  ---
  Category 3: Control Flow

  ---
  [L3-A] MISSING Exit — empty-array result overwritten
  MVCFramework.Serializer.JsonDataObjects.pas ~lines 3221–3268
  (SerializeArrayOfRecord)

  if ATValueContainingAnArray.GetArrayLength = 0 then
  begin
    Result := '[]';
    // no Exit here!
  end;
  lJSONArr := TJsonArray.Create;
  try
    for I := 0 to GetArrayLength - 1 do   // runs 0 iterations
      // ...
    Result := lJSONArr.ToJSON();  // overwrites '[]' with whatever the library 
  emits for an empty array
  finally
    lJSONArr.Free;
  end;

  Failure mode: For an empty array, Result is set to '[]' and then immediately
  overwritten by lJSONArr.ToJSON(). Functionally this is probably harmless (an
  empty TJsonArray serializes to []), but it creates an unnecessary TJsonArray
  allocation and immediately discards the guard code. Any future change to the
  guard value (e.g., 'null') would be silently negated.

  Fix: Add Exit after Result := '[]', or restructure to skip the serialization
  block entirely for count = 0.

  ---
  [L3-B] DEAD CODE — nil check after successful is test
  MVCFramework.Serializer.JsonDataObjects.pas ~line 2440 (inside
  JsonObjectToObject)

  if AObject is TJsonObject then
  begin
    if not Assigned(AObject) then   // always False: 'is' already confirmed 
  non-nil
      raise EMVCException.CreateFmt('...', [AObject.ClassName]);  // would AV on
   nil

  Failure mode: The inner nil check never triggers. If AObject were nil, AObject
   is TJsonObject would return False and skip this block entirely. The inner
  raise line also calls AObject.ClassName which would itself AV on a nil
  pointer. Dead code hiding an intent error — likely was meant to be placed
  before the is test.

  Fix: Move the nil guard before the is test:
  if not Assigned(AObject) then
    raise EMVCException.Create('...');
  if AObject is TJsonObject then
    // ...

  ---
  [L3-C] OUT-OF-BOUNDS AV on malformed URL-encoded input
  MVCFramework.Serializer.URLEncoded.pas ~lines 170–175 (DeserializeObject)

  lKeyValue := lPieces[I].Split(['=']);
  SL.AddPair(lKeyValue[0], URLDecode(lKeyValue[1]));  // AV if piece has no '='

  Failure mode: If a query string contains a fragment with no = (e.g.,
  ?flag&key=val), Split(['=']) returns a single-element array. Accessing
  lKeyValue[1] raises ERangeError/AV. Any client sending a bare flag parameter
  will crash the deserialization.

  Fix:
  lKeyValue := lPieces[I].Split(['=']);
  if Length(lKeyValue) >= 2 then
    SL.AddPair(lKeyValue[0], URLDecode(lKeyValue[1]))
  else if Length(lKeyValue) = 1 then
    SL.AddPair(lKeyValue[0], '');

  ---
  Category 4: Threading

  ---
  [L4-A] RACE CONDITION on lazy singleton initialization
  MVCFramework.Serializer.Streaming.pas ~lines 165–171 (HasCustomTypeSerializer)

  function HasCustomTypeSerializer(...): Boolean;
  begin
    if GTypeSerializerProbe = nil then
      GTypeSerializerProbe := TMVCJsonDataObjectsSerializer.Create;  // no lock
    Result := GTypeSerializerProbe.TypeSerializerExists(ATypeInfo);
  end;

  Failure mode: Two threads can both read GTypeSerializerProbe = nil
  simultaneously and both create an instance. One instance is assigned; the
  other is abandoned (leaked). Worse, if one thread is mid-construction while
  another reads a partially initialized pointer, behavior is undefined. This is
  a classic TOCTOU race on a global variable.

  Fix: Use a TCriticalSection or TMonitor.Enter/Exit guard around the nil check
  and assignment. Or initialize GTypeSerializerProbe in a unit initialization
  block on the main thread before any concurrent access is possible.

  ---
  [L4-B] THREADVAR CLEANUP UNREACHABLE from non-finalizer threads
  (cross-reference L1-D)
  MVCFramework.Serializer.Streaming.pas ~lines 380–386 (ClassDestroy)

  ClassDestroy is the unit finalizer. It runs on the main thread only.
  GStreamWriter and GJsonWriter are threadvar — their per-thread copies are
  inaccessible from any other thread. Worker threads that exit never free their
  copies. See L1-D for the full analysis; noting here as a threading concern
  distinct from the lifecycle leak.

  ---
  Category 5: Arithmetic / Type Errors

  ---
  [L5-A] SILENT TRUNCATION of UInt64/UInt32 nullables
  MVCFramework.Serializer.JsonDataObjects.pas ~lines 3465–3659
  (TryNullableToJSON)

  // NullableUInt64:
  AJSONObject.I[AName] := AValue.AsType<NullableUInt64>().Value;  // .I is Int32
   property

  // NullableUInt32:
  AJSONObject.I[AName] := AValue.AsType<NullableUInt32>().Value;  // .I is Int32
   property

  Failure mode: .I[name] on a TJDOJsonObject is the Int32 property (range
  −2147483648..2147483647). For UInt64 values above MaxInt (2,147,483,647) or
  UInt32 values in the high half, the assignment silently truncates/wraps.
  Serialized JSON contains the wrong number. No exception raised. Values between
   MaxInt+1 and MaxUInt32 (for UInt32) or MaxInt64+1 and MaxUInt64 (for UInt64)
  are always corrupted.

  Fix: Use the appropriate property:
  // For UInt64:
  AJSONObject.L[AName] := Int64(AValue.AsType<NullableUInt64>().Value);
  // For UInt32 values that may exceed MaxInt:
  AJSONObject.L[AName] := AValue.AsType<NullableUInt32>().Value;
  For true unsigned semantics, serialize as string for values exceeding
  MaxInt64, or use the library's U[name] property if available.

  ---
  Category 6: String / Buffer Handling

  ---
  [L6-A] WRITES STRING HEADER BYTES, NOT CHARACTERS
  MVCFramework.Serializer.Commons.pas ~line 1242 (inside
  MapDataSetFieldToRTTIField, string-field branch)

  lStrValue := AField.AsString;
  lInternalStream.Write(lStrValue, Length(lStrValue));

  Failure mode: In Delphi, a string variable is a pointer to the heap string
  data preceded by a reference count, length, and code-page header.
  Write(lStrValue, N) passes the address of the string pointer variable (a 4- or
   8-byte pointer), not the address of the string's characters. The stream
  receives SizeOf(Pointer) bytes of the pointer value, not the actual UTF-16
  character data. The RTTI field subsequently receives corrupted bytes.

  Fix: Pass the actual character buffer:
  if Length(lStrValue) > 0 then
    lInternalStream.Write(lStrValue[1], Length(lStrValue) * SizeOf(Char));
  Or use TEncoding.Unicode.GetBytes(lStrValue) for explicit byte handling.

  ---
  Category 7: Collections

  ---
  [L7-A] OBJECT LEAK when Assert fails
  MVCFramework.Serializer.JsonDataObjects.pas ~lines 1420–1434 (JsonArrayToList)

  Obj := TMVCSerializerHelper.CreateObject(AClazz.QualifiedClassName);  // 
  allocated
  Assert(AJsonArray.Items[I].Typ = jdtObject, 'Expected JSON object in array at 
  index ' + IntToStr(I));
  // Assert raises EAssertionFailed if type mismatch; Obj is never added to list
   and never freed

  Failure mode: If the JSON array contains a non-object element where an object
  is expected, Assert raises EAssertionFailed. Obj was just created and is not
  yet in any owning collection. It leaks. In release builds with assertions
  disabled, the code continues with a non-object item — different corruption.

  Fix:
  Obj := TMVCSerializerHelper.CreateObject(AClazz.QualifiedClassName);
  try
    if AJsonArray.Items[I].Typ <> jdtObject then
      raise EMVCDeserializationException.CreateFmt('Expected JSON object at 
  index %d', [I]);
    JsonObjectToObject(AJsonArray.Items[I].ObjectValue, Obj, AType,
  AIgnoredAttributes);
    AList.Add(Obj);
    Obj := nil;   // list took ownership
  finally
    Obj.Free;     // no-op if Obj = nil (already owned by list)
  end;

  ---
  Category 8: Exception Safety

  ---
  [L8-A] RECORD FIELD OFFSET SILENTLY ZEROED — data corruption
  MVCFramework.Serializer.JsonDataObjects.pas ~lines 2663–2688
  (JSONObjectToNestedRecordFieldStatic)

  // Commented-out line from what should be the working version:
  // lChildFieldOffset := RecordFieldRTTIType.Offset + TypeOffset;

  lValue := lField.GetValue(Buffer);              // reads from Buffer + 0 
  (wrong offset)
  // ... deserialize into lValue ...
  lField.SetValue(Buffer, lValue);                // writes to Buffer + 0 (wrong
   offset)

  Compare with the sibling JSONObjectToNestedRecordField (non-static, ~lines
  2637–2661) which correctly computes and applies the offset.

  Failure mode: Nested records inside a parent record are all deserialized into
  offset 0 of the buffer, overwriting each other and the first field of the
  record. Every field after the first nested record in a struct receives
  corrupted data. This is silent data corruption — no exception, wrong values
  written to the correct object memory.

  Fix: Restore the commented-out offset arithmetic:
  lChildFieldOffset := RecordFieldRTTIType.Offset + TypeOffset;
  lValue := lField.GetValue(PByte(Buffer) + lChildFieldOffset);
  // ... deserialize ...
  lField.SetValue(PByte(Buffer) + lChildFieldOffset, lValue);

  ---
  [L8-B] (See L1-B — GetMem buffer leak on non-EInvalidCast exceptions. Listed 
  under lifecycle but is fundamentally an exception-safety failure.)

  ---
  Category 9: API Contracts

  ---
  [L9-A] WRONG PAGINATION SIGNAL — next_page_uri emitted incorrectly
  MVCFramework.Serializer.Commons.pas ~lines 519–546 (GetPaginationData)

  if CurrPageSize = DefaultPageSize then
    // emit "next_page_uri"

  Failure mode: The condition should be "did we get a full page worth of
  results?" as the heuristic for "there might be more". Using CurrPageSize = 
  DefaultPageSize conflates "the page size setting equals the default" with
  "this page was full". If a caller requests a custom page size (different from
  the default) and receives a full page, no next_page_uri is emitted. If the
  default equals the actual results count by coincidence on the last page, a
  bogus next_page_uri is emitted.

  Fix: The condition should compare the number of records actually returned
  against the requested page size: ActualRecordCount = RequestedPageSize (or
  equivalently, ActualRecordCount >= PageSize).

  ---
  [L9-B] UNSAFE INTERFACE CAST without type verification
  MVCFramework.Serializer.JsonDataObjects.CustomTypes.pas ~line 485
  (TMVCObjectDictionarySerializer.SerializeAttribute)

  lObjDict := TMVCObjectDictionary(AElementValue.AsInterface);

  Failure mode: AsInterface returns an IInterface; casting directly to
  TMVCObjectDictionary (a class) bypasses the interface dispatch table and
  treats the raw interface pointer as an object reference. This is an incorrect
  cast pattern — interface pointers and object references are not the same
  pointer. On any object that implements multiple interfaces, the interface
  pointer from AsInterface will be an offset into the object's vtable region,
  not the object's Self pointer. Result: corrupted object reference, AV or
  silent wrong dispatch.

  Fix:
  if not Supports(AElementValue.AsObject, IMVCObjectDictionary, lObjDictIntf)
  then
    raise EMVCSerializationException.Create('Expected TMVCObjectDictionary');
  // use lObjDictIntf, or:
  lObjDict := AElementValue.AsObject as TMVCObjectDictionary;

  ---
  Category 10: Design Smells that Cause Actual Bugs

  ---
  [L10-A] SHADOW CLASS HACK for TStreamWriter internals
  MVCFramework.Serializer.Streaming.pas ~lines 188–197

  type
    TStreamWriterLayout = class  // shadow to access private fields
    public
      FStream     : TStream;
      FEncoding   : TEncoding;
      FNewLine    : string;
      FAutoFlush  : Boolean;
      FBufSize    : Integer;
      FBufferIndex: Integer;
      FBuffer     : TCharArray;
    end;

  Used to rebind FStream of a live TStreamWriter to a new stream, bypassing the
  public API.

  Failure mode: This layout is not guaranteed by any Embarcadero ABI contract.
  If the RTL adds, removes, or reorders fields between Delphi versions (which
  has happened between major versions), FStream points to a different field and
  the assignment corrupts the writer's internal state. Given the framework's
  declared support range (D10 Seattle through D13 Florence), field layout
  changes across that span are a real risk. Corruption would be silent — wrong
  stream written, data directed to wrong target.

  Design fix: Replace with a subclass that overrides the relevant write method,
  or keep a wrapper that creates a new TStreamWriter per call rather than
  rebinding it.

  ---
  [L10-B] GetPaginationData — wrong guard variable (see L9-A)

  ---
  Top 10 — Fix These First

  Ranked by: crash/corruption potential × call frequency × ease of triggering

  #: 1
  ID: L1-A
  File: JsonDataObjects.pas ~1820
  Severity: CRASH
  Finding: Use-after-free: FreeMem(lBuff) immediately after
    TValue.MakeWithoutCopy(lBuff,...)
  ────────────────────────────────────────
  #: 2
  ID: L8-A
  File: JsonDataObjects.pas ~2663
  Severity: CORRUPTION
  Finding: Offset arithmetic commented out in
  JSONObjectToNestedRecordFieldStatic
    — all nested record fields deserialized to offset 0
  ────────────────────────────────────────
  #: 3
  ID: L2-A
  File: Commons.pas ~1451–1869
  Severity: WRONG RESULT
  Finding: TValue.GetValue().AsType<Nullable>().Clear — clears a temporary copy;

    null columns never cleared; affects every nullable type (~23 variants)
  ────────────────────────────────────────
  #: 4
  ID: L6-A
  File: Commons.pas ~1242
  Severity: CORRUPTION
  Finding: stream.Write(lStrValue, Length) writes the string pointer bytes, not
    the characters
  ────────────────────────────────────────
  #: 5
  ID: L1-E
  File: JsonDataObjects.pas ~1126
  Severity: CRASH+LEAK
  Finding: JSONArray.Free double-frees a child owned by JSONObject; parent never

    freed
  ────────────────────────────────────────
  #: 6
  ID: L3-C
  File: URLEncoded.pas ~170
  Severity: CRASH
  Finding: AV on any URL-encoded key without = sign; trivially triggered by
    malformed client input
  ────────────────────────────────────────
  #: 7
  ID: L2-B
  File: JsonDataObjects.pas ~1295
  Severity: WRONG RESULT
  Finding: AJsonArray.types[0] in loop — always checks type of first element;
    heterogeneous arrays deserialized with wrong branch
  ────────────────────────────────────────
  #: 8
  ID: L9-B
  File: CustomTypes.pas ~485
  Severity: CRASH
  Finding: TMVCObjectDictionary(AElementValue.AsInterface) — invalid cast from
    interface pointer to class pointer
  ────────────────────────────────────────
  #: 9
  ID: L5-A
  File: JsonDataObjects.pas ~3465
  Severity: WRONG RESULT
  Finding: NullableUInt64/NullableUInt32 serialized via .I[] (Int32) — silent
    truncation for values > MaxInt
  ────────────────────────────────────────
  #: 10
  ID: L4-A
  File: Streaming.pas ~165
  Severity: RACE
  Finding: GTypeSerializerProbe lazy init without lock — double-create and
    potential partially-initialized read under concurrent requests

  ---
  Items not in the top 10 but worth scheduling: L1-B (buffer leak on
  deserialization errors), L1-C (encoding leak per call), L1-D (threadvar leak
  per worker thread), L3-A (benign but dead guard code), L3-B (dead nil check),
  L7-A (object leak on type mismatch), L9-A (wrong pagination signal), L10-A
  (shadow class field layout dependency).*

✻ Worked for 7m 20s

> /export serialization_bug_analysis.txt                                        
  ⎿  Conversation exported to:
     c:\0\DelphiMVCFramework\sources\serialization_bug_analysis.txt

> Do the detailed analysis of the top 10 problems to see if they are real       
  problems.                                                                     

● I'll read the exact code at each reported location in parallel to verify every
   finding.

  Read 3 files (ctrl+o to expand)

● Now let me read the remaining critical sections:

  Read 1 file (ctrl+o to expand)

● Now let me read the remaining three findings:

  Read 3 files (ctrl+o to expand)

● I need to check who calls JSONObjectToNestedRecordFieldStatic and how
  MakeWithoutCopy behaves:

  Searched for 1 pattern, read 1 file (ctrl+o to expand)

● Let me check the broader context of BuildATValueArrayFromJSONArrayOfJSONObject
   and how lArr is used:

  Read 1 file (ctrl+o to expand)

● Now I have all the information needed. Let me compile the definitive verdict.

  ---
  Top-10 Detailed Verification Report

  ---
  #1 — L1-A: Use-after-free in BuildATValueArrayFromJSONArrayOfJSONObject

  File: JsonDataObjects.pas lines 1822–1829

  Actual code:
  lBuff := AValue.GetReferenceToRawArrayElement(I);   // ptr into AValue's array
   data (discarded below)
  lJItem := AJSONObject.A[APropertyName].Items[I].ObjectValue;
  JSONObjectToRecord(lJItem, lInnerTypeAsRecord, lBuff);  // out param: 
  overwrites lBuff with new GetMem'd ptr
  TValue.MakeWithoutCopy(lBuff, lInnerType.Handle, lArr[I]); // TValue stores 
  REFERENCE to lBuff (no copy)
  FreeMem(lBuff, lInnerType.TypeSize);                       // frees what 
  lArr[I] now references

  JSONObjectToRecord (line 2525) is declared out Buffer: PByte — it ignores the
  input value, calls GetMem, and assigns a fresh allocation. After the call,
  lBuff is that new allocation.

  TValue.MakeWithoutCopy with ACopy = False stores lBuff as a bare pointer in
  the TValue (FData.FValueData.FValue = lBuff); it does not copy the data. The
  call to FreeMem(lBuff) on the next line frees the memory the TValue is now
  referencing.

  This TValue is then consumed at line 2134 / 2157:
  TValue.FromArray(ARTTIField.FieldType.Handle, lArr) — which reads from
  lArr[I]'s internal pointer, which is now freed memory.

  Verdict: CONFIRMED — real use-after-free crash. FreeMem must be removed; the 
  buffer's lifetime is owned by the TValue (or replaced with TValue.Make + 
  FinalizeRecord + FreeMem).

  ---
  #2 — L8-A: Offset commented out in JSONObjectToNestedRecordFieldStatic

  File: JsonDataObjects.pas lines 2678–2686

  Actual code:
  // lChildFieldOffset := RecordFieldRTTIType.Offset + TypeOffset;  // commented
   out
  for lField in lChildType.GetFields do
  begin
    lValue := lField.GetValue(Buffer);           // no offset applied
    ...
    lField.SetValue(Buffer { + lChildFieldOffset }, lValue);
  end;

  Call site (line 2106–2107):
  lRef := PByte(AValue.GetReferenceToRawData);
  JSONObjectToNestedRecordFieldStatic(AJSONObject, ARTTIField, 0, lRef);

  AValue at this point is the TValue of the nested record field —
  GetReferenceToRawData returns a pointer already positioned at the start of the
   nested record's data within the TValue. Adding RecordFieldRTTIType.Offset
  (the offset of the nested record within its parent) would be doubly wrong
  here. The fields of the nested record are at Buffer + innerField.Offset, which
   is exactly what lField.GetValue(Buffer) computes internally.

  The non-static version's call site (JSONObjectToRecord, line 2555) passes the
  parent record buffer where the extra offset IS needed. These are two different
   call contracts.

  Verdict: FALSE POSITIVE. The commented-out offset would introduce data 
  corruption for this call site. The current code is correct.

  ---
  #3 — L2-A: TValue.GetValue().AsType<>().Clear on temporary copy

  File: Commons.pas lines 1472–1475 (and repeated for all non-String nullable
  types)

  Actual code:
  // NullableString — correctly handled:
  aRTTIField.SetValue(AObject, TValue.From<NullableString>(nil));   // line 1462

  // NullableInt32 and all subsequent nullable types — broken:
  aRTTIField.GetValue(AObject).AsType<NullableInt32>().Clear;       // line 1474

  TRttiField.GetValue returns a TValue by value — a copy of the record.
  AsType<NullableInt32>() extracts another copy. .Clear mutates that ephemeral
  copy and discards it. The field in AObject is never modified. Confirmed for
  NullableInt32, NullableUInt32, NullableInt64, NullableUInt64, NullableInt16,
  and every other nullable variant that follows the same pattern (the full list
  spans lines 1470–1682).

  Verdict: CONFIRMED — real wrong-result bug. Null dataset columns are silently 
  left with their previous value for all nullable integer types.

  ---
  #4 — L6-A: TStream.Write(lStrValue, Length(lStrValue)) writes string pointer 
  bytes

  File: Commons.pas lines 1239–1244

  Actual code:
  lStrValue := AField.AsString;
  if not lStrValue.IsEmpty then
  begin
    lInternalStream.Write(lStrValue, Length(lStrValue));  // line 1242
    lInternalStream.Position := 0;
  end;

  TStream.Write(const Buffer; Count: Longint) takes Buffer by const reference —
  the address of the variable lStrValue is passed, which is a 4- or 8-byte
  pointer to the string heap data. Length(lStrValue) is the character count
  (WideChars). So the write emits Length bytes starting from the address of the
  local pointer variable itself: on Win64 that means the 8-byte pointer value +
  up to 8 bytes of adjacent stack data, not the string's characters.

  This is the tkClass branch where a string field is being mapped to a TStream
  property (e.g., a TMemoryStream). The resulting stream content is completely
  wrong.

  Verdict: CONFIRMED — real data corruption. The correct write is 
  lInternalStream.Write(lStrValue[1], Length(lStrValue) * SizeOf(Char)) for raw 
  UTF-16, or via a TEncoding.GetBytes call for a specific encoding.

  ---
  #5 — L1-E: JSONObject leak in DeserializeCollection (custom-serializer + 
  root-node path)

  File: JsonDataObjects.pas lines 1141–1171

  Actual code:
  // ARootNode is not empty:
  JsonBase := TJDOJsonObject.Parse(ASerializedList);     // allocates JSONObject
  JSONObject := TJDOJsonObject(JsonBase);
  JSONArray := JSONObject.A[ARootNode] as TJDOJsonArray;  // child reference; 
  owned by JSONObject
  // ...
  try
    GetTypeSerializers.Items[AList.ClassInfo].DeserializeRoot(JSONArray, AList,
  []);
    Exit;
  finally
    JSONArray.Free;  // frees a child of JSONObject; JSONObject is never freed
  end;

  When ARootNode is empty, JSONArray is a standalone object and JSONArray.Free
  is correct. When ARootNode is not empty, JSONObject holds the root and
  JSONArray is a child. JSONObject is never freed. Every call to this path leaks
   the entire parsed JSON tree.

  The JSONArray.Free call also frees a node still referenced by JSONObject, but
  since JSONObject.Free is never called, there is no actual double-free — it's
  purely a leak.

  Verdict: CONFIRMED — real memory leak on every deserialization call that uses 
  a custom type serializer with a non-empty root node.

  ---
  #6 — L3-C: Out-of-bounds on URL-encoded bare parameter (no =)

  File: URLEncoded.pas lines 176–177

  Actual code:
  lKeyValue := lPieces[I].Split(['=']);
  SL.AddPair(lKeyValue[0], URLDecode(lKeyValue[1]));  // AV if no '=' in piece

  For a fragment without = (e.g., flag in flag&key=value), Split(['=']) returns
  a single-element array. lKeyValue[1] raises ERangeError.

  However, the entire for loop is wrapped in:
  except
    on E: Exception do
      raise EMVCException.Create(HTTP_STATUS.BadRequest, E.Message);
  end;

  ERangeError is a Exception descendant and is caught. The server does not crash
   — it returns a 400 Bad Request with an opaque "List index out of bounds (1)"
  message instead of gracefully ignoring the bare flag.

  Verdict: CONFIRMED as a bug (wrong behavior for valid inputs), but the 
  severity was overstated — it does not crash. Severity: wrong-result / bad 
  error message, not crash.

  ---
  #7 — L2-B: AJsonArray.types[0] inside a loop over all elements

  File: JsonDataObjects.pas line 1313

  Actual code:
  for I := 0 to Pred(AJsonArray.Count) do
  begin
    case AJsonArray.types[0] of    // always [0], not [I]
      jdtString: ...
      jdtInt, jdtLong: ...
      jdtFloat: ...
      jdtBool: ...
    end;
  end;

  Note that BuildATValueArrayFromJSONArrayOfSimpleType (nested proc at line
  1847) uses the same pattern intentionally: lArrayItemType := LJArr.types[0]
  and uses it for all iterations. That function's intent is "assume homogeneous
  type from the first element." The outer JsonArrayToArray function does the
  same.

  For JSON arrays where all elements share the same type (the overwhelmingly
  common case in REST APIs), this is harmless. For heterogeneous arrays (e.g.,
  [1, "text", true]), all elements after index 0 are routed through the first
  element's branch — elements are silently dropped into the wrong typed array or
   produce wrong conversions.

  Verdict: CONFIRMED for heterogeneous arrays, but the impact is low in practice
   since typed API payloads are almost always homogeneous. The // TODO - I dont 
  like this comment at line 1310 confirms the author was aware.

  ---
  #8 — L9-B: TMVCObjectDictionary(AElementValue.AsInterface) cast

  File: CustomTypes.pas line 486

  Actual code:
  lObjDict := TMVCObjectDictionary(AElementValue.AsInterface);

  TValue.AsInterface returns an IInterface. A hard-cast TClass(anInterface)
  reinterprets the interface pointer as an object pointer without adjustment. In
   Delphi's COM-compatible ABI, interface pointers and object pointers are NOT
  necessarily the same address.

  However, TMVCObjectDictionary inherits from TInterfacedObject, which places
  IUnknown as the first (and by inheritance layout rules, lowest-addressed)
  interface. For TInterfacedObject descendants, the IUnknown vtable pointer
  lives at the same address as the object's own VMT pointer — making the
  interface pointer and the object pointer coincide. This is an implementation
  detail of Delphi's Win32/Win64 ABI that has been stable.

  In practice this cast works for TMVCObjectDictionary, but it is semantically
  wrong and would fail for any class where AsInterface returns a non-primary
  interface (offset ≠ 0), or on non-Windows platforms with a different ABI.

  Verdict: LATENT BUG — works by accident on current targets. Severity: low now,
   non-trivial risk in any ABI change or multi-interface scenario. The safe 
  replacement is AElementValue.AsObject as TMVCObjectDictionary.

  ---
  #9 — L5-A: NullableUInt32 and NullableUInt64 serialized via .I[] (Int32 
  property)

  File: JsonDataObjects.pas lines 3617 and 3632

  Actual code:
  // NullableUInt32 (line 3617):
  AJSONObject.I[AName] := AValue.AsType<NullableUInt32>().Value;   // .I is 
  Int32 (signed 32-bit)

  // NullableUInt64 (line 3632):
  AJSONObject.I[AName] := AValue.AsType<NullableUInt64>().Value;   // .I is 
  Int32 (signed 32-bit)

  For comparison, NullableInt64 correctly uses .L[AName] (Int64, line 3508).

  .I[] is the Int32 property. Assignment from UInt32 where value > MaxInt32
  (2,147,483,647) silently truncates or wraps. Assignment from UInt64 has the
  same problem for values exceeding MaxInt32. For UInt64, even .L[] (Int64)
  would lose the top bit for values in the true-unsigned range above MaxInt64.

  Verdict: CONFIRMED — real silent data corruption for any NullableUInt32 value 
  ≥ 2^31 and any NullableUInt64 value ≥ 2^31. Fix: use .L[AName] for 
  NullableUInt32; use .L[AName] or .S[AName] (string serialization) for 
  NullableUInt64 values that may exceed MaxInt64.

  ---
  #10 — L4-A: Race condition on GTypeSerializerProbe lazy initialization

  File: Streaming.pas lines 165–171

  Actual code:
  function HasCustomTypeSerializer(const ATypeInfo: PTypeInfo): Boolean;
  begin
    if ATypeInfo = nil then Exit(False);
    if GTypeSerializerProbe = nil then
      GTypeSerializerProbe := TMVCJsonDataObjectsSerializer.Create;  // no lock
    Result := GTypeSerializerProbe.GetTypeSerializers.ContainsKey(ATypeInfo);
  end;

  The comment on line 163 confirms this is intentionally lazy-initialized. In a
  multi-threaded request handler (Indy thread-per-request), two threads can
  simultaneously read GTypeSerializerProbe = nil, both call .Create, and both
  write to the global. The first writer "wins"; the second's object is leaked.
  Additionally, a third thread reading GTypeSerializerProbe during construction
  would observe a partially initialized object — the Serializer's constructor
  registers type serializers into a TDictionary, and a read of an
  incompletely-constructed dictionary is undefined.

  Verdict: CONFIRMED — real race condition. The fix is to initialize 
  GTypeSerializerProbe in the unit's initialization block (single-threaded) 
  rather than lazily, or to guard with 
  TMonitor.Enter(TObject(GTypeSerializerProbe))-style locking around the nil 
  check and assignment.

  ---
  Revised Top-10 Ranking

  ┌────────┬───────────────────────────────────┬─────────────┬──────────────┐
  │  Rank  │                Bug                │   Verdict   │   Severity   │
  ├────────┼───────────────────────────────────┼─────────────┼──────────────┤
  │ 1      │ L1-A Use-after-free: FreeMem      │ ✅          │ Crash / heap │
  │        │ after MakeWithoutCopy             │ CONFIRMED   │  corruption  │
  ├────────┼───────────────────────────────────┼─────────────┼──────────────┤
  │ 2      │ L2-A GetValue().AsType<>().Clear  │ ✅          │ Silent wrong │
  │        │ on copy — null fields not cleared │ CONFIRMED   │  result      │
  ├────────┼───────────────────────────────────┼─────────────┼──────────────┤
  │ 3      │ L6-A Write(lStrValue, Length) —   │ ✅          │ Data         │
  │        │ writes string pointer bytes       │ CONFIRMED   │ corruption   │
  ├────────┼───────────────────────────────────┼─────────────┼──────────────┤
  │        │ L5-A                              │ ✅          │ Silent wrong │
  │ 4      │ NullableUInt32/NullableUInt64 via │ CONFIRMED   │  result      │
  │        │  .I[] — truncation                │             │              │
  ├────────┼───────────────────────────────────┼─────────────┼──────────────┤
  │        │ L1-E JSONObject leaked in         │ ✅          │ Memory leak  │
  │ 5      │ custom-serializer + root-node     │ CONFIRMED   │ per request  │
  │        │ path                              │             │              │
  ├────────┼───────────────────────────────────┼─────────────┼──────────────┤
  │ 6      │ L4-A Race on GTypeSerializerProbe │ ✅          │ Race /       │
  │        │  init                             │ CONFIRMED   │ partial init │
  ├────────┼───────────────────────────────────┼─────────────┼──────────────┤
  │        │                                   │ ✅          │ Wrong result │
  │ 7      │ L2-B types[0] in loop —           │ CONFIRMED   │  for mixed   │
  │        │ heterogeneous array wrong routing │ (limited    │ arrays       │
  │        │                                   │ impact)     │              │
  ├────────┼───────────────────────────────────┼─────────────┼──────────────┤
  │        │                                   │ ✅          │              │
  │ 8      │ L3-C URL bare-param lKeyValue[1]  │ CONFIRMED   │ Bad error    │
  │        │ out of bounds                     │ (caught →   │ handling     │
  │        │                                   │ 400)        │              │
  ├────────┼───────────────────────────────────┼─────────────┼──────────────┤
  │        │                                   │ ⚠️ LATENT   │              │
  │ 9      │ L9-B Interface→class cast without │ (works by   │ Latent       │
  │        │  adjustment                       │ ABI         │              │
  │        │                                   │ accident)   │              │
  ├────────┼───────────────────────────────────┼─────────────┼──────────────┤
  │ ~~10~~ │ ~~L8-A~~ Offset commented out in  │ ❌ FALSE    │ —            │
  │        │ Static variant                    │ POSITIVE    │              │
  └────────┴───────────────────────────────────┴─────────────┴──────────────┘

  L8-A is retracted. The commented-out offset was intentionally not applied: the
   Static variant's call site passes AValue.GetReferenceToRawData, which already
   points to the nested record's start, so adding RecordFieldRTTIType.Offset
  again would corrupt memory. The non-static variant has a different call
  convention (parent buffer + offset) which is why it needs the computation.

✻ Cooked for 9m 4s

※ recap: Audited 11 serializer files for real bugs. Verified the top 10 findings: 7 confirmed, 1 false positive (L8-A offset), 2 partial. Ready to fix any confirmed bug on request. (disable recaps in /config)