#1 2021-11-11 15:49:29

Eugene Ilyin
Member
From: milky_way/orion_arm/sun/earth
Registered: 2016-03-27
Posts: 132
Website

[Fixed] TDocVariantData.GetValueIndex, TDocVariantData.GetVarData

Hi, the recent 1.18.6318:

Check
program Test;
{$APPTYPE CONSOLE}
uses
  SynCommons;
var
  V: Variant;
  VD: TVarData;
begin
  with TDocVariantData(V) do
  begin
    InitJSON('{"":1}');
    Writeln(GetValueIndex('') = 0);
    Writeln(GetVarData('', VD, @StrComp));
    Writeln(AddValueFromText('','test') = 0);
  end;
  Readln;
end.
Expected
TRUE
TRUE
TRUE
Actual
Exception class $C0000005 with message 'access violation at 0x0052573a: read of address 0xfffffffc'. Process Test.exe (11556)
Issue

TDocVariantData.GetValueIndex problem place:

...
      result := FindNonVoidRawUTF8(pointer(VName),aName,aNameLen,VCount) else
      result := FindNonVoidRawUTF8I(pointer(VName),aName,aNameLen,VCount) else

[Actual]
  FindNonVoidRawUTF8:
    for result := 0 to count-1 do // all VName[]<>'' so n^<>0
      if (PStrLen(n^-_STRLEN)^=len) and CompareMemFixed(pointer(n^),name,len) then

  FindNonVoidRawUTF8I:
    if (PStrLen(n^-_STRLEN)^=len) and IdemPropNameUSameLen(pointer(n^),name,len) then

[Expected]
  FindNonVoidRawUTF8:
    for result := 0 to count-1 do
      if ((n^=0) and (len=0)) or
        ((PStrLen(n^-_STRLEN)^=len) and CompareMemFixed(pointer(n^),name,len)) then

  FindNonVoidRawUTF8I:
    if ((n^=0) and (len=0)) or
      ((n^<> 0) and (PStrLen(n^-_STRLEN)^=len) and IdemPropNameUSameLen(pointer(n^),name,len)) then

TDocVariantData.GetVarData problem place:

[Actual]
  if (integer(VType)<>DocVariantVType) or not(dvoIsObject in VOptions) or
     (VCount=0) or (aName='') then

[Expected]
  if (integer(VType)<>DocVariantVType) or not(dvoIsObject in VOptions) or
     (VCount=0) then

TDocVariantData.AddValueFromText problem place:

[Actual]
  function TDocVariantData.AddValueFromText(const aName,aValue: RawUTF8;
    Update, AllowVarDouble: boolean): integer;
  begin
    if aName='' then begin
      result := -1;
      exit;
    end;
  result := GetValueIndex(aName);

[Expected]
//  if aName='' then begin
//    result := -1;
//    exit;
//  end;
  result := GetValueIndex(aName);
Impact
  • Any request contained the empty name in JSON object will crush the thread request processing

  • Any TDocVariantData access: Exists(aName), NameIndex(aName), AddValue, AddValueFromText, SearchItemByProp, ReduceAsArray, Rename, Delete(aName), GetValueOrDefault, GetValueOrNull, GetValueOrEmpty, GetValueEnumerate, GetAsPVariant, RetrieveValueOrRaiseException, SetValueOrItem, AddOrUpdateValue, GetOrAddIndexByName, GetOrAddPVariantByName, GetPVariantByName, IntSet, DoFunction, Exists, ExistsOrLock, AddExistingPropOrLock, ...

  • Any JSON iterating, search, etc.

Context

Is empty name allowed for JSON object members?
Yes. Following the spec:

json
  element
elements
  element
  element ',' elements
element
  ws value ws
value
  object
object
  '{' members '}'
members
  member
  member ',' members
member
  ws string ws ':' element <-- String HERE for member name
string
  '"' characters '"' <-- Chars HERE for string value
characters
  "" <-- Empty content HERE for chars
  character characters

Coverage
No empty name tests provided in sanity checks.

P. S. Please check the other places where the Name prop assumed not to be empty (no sure that I found all the cases).

Last edited by Eugene Ilyin (2021-11-11 20:25:24)

Offline

#2 2021-11-11 17:08:34

ab
Administrator
From: France
Registered: 2010-06-21
Posts: 14,303
Website

Re: [Fixed] TDocVariantData.GetValueIndex, TDocVariantData.GetVarData

By design, we don't support void field names, even if the JSON specs allow it.
TDocVariantData.InternalAdd expects to be called with aName='' for arrays, so it would not allow to create void field names anyway.

So indeed there is a missing check for void name in TDocVariantData.GetValueIndex.
I have just fixed it by https://github.com/synopse/mORMot2/comm … 1df265067a and https://synopse.info/fossil/info/b914adc67d
It is the only place I have seen missing such a check - TDocVariantData.GetVarData has a proper check.

Offline

#3 2021-11-11 20:28:19

Eugene Ilyin
Member
From: milky_way/orion_arm/sun/earth
Registered: 2016-03-27
Posts: 132
Website

Re: [Fixed] TDocVariantData.GetValueIndex, TDocVariantData.GetVarData

Hm, ok partial support of the JSON spec for this rare case is good enough.
Anything better than unexpected exception for incoming JSONs smile

Thanks, fix confirmed, now it's

FALSE
FALSE
FALSE

Offline

#4 2021-11-11 20:46:16

ab
Administrator
From: France
Registered: 2010-06-21
Posts: 14,303
Website

Re: [Fixed] TDocVariantData.GetValueIndex, TDocVariantData.GetVarData

Sorry for the issue.

It was affecting mORMot 1 and 2 BTW.

Offline

Board footer

Powered by FluxBB