mORMot Open Source

avista

Member

Registered: 2014-01-06

Posts: 63

JSON Parser Bug - Enumerated Types

I found a problem where, if the 'twoEnumSetsAsTextInRecord' option is enabled, deserializing fails.

The problem is in TTextWriter.AddTypedJSON(), where the lowercase prefix of the enumerated type name is being stripped:

procedure TTextWriter.AddTypedJSON(aTypeInfo: pointer; const aValue;
  EnumSetsAsText,FullSetsAsStar: boolean);
var max, i: Integer;
    PS: PShortString;
begin
  case PTypeKind(aTypeInfo)^ of
    tkClass:
      WriteObject(TObject(aValue),[woFullExpand]);
    tkEnumeration:
      if EnumSetsAsText then begin
        Add('"');
        AddTrimLeftLowerCase(GetEnumName(aTypeInfo,byte(aValue))); <<<<<<<<<<<<<<<<
        Add('"');
      end else
        AddU(byte(aValue));

And when it's being read, it's assuming the lowercase prefix is NOT present:

function TJSONCustomParserCustomSimple.CustomReader(P: PUTF8Char;
  var aValue; out EndOfObject: AnsiChar): PUTF8Char;
[...]
    ktEnumeration: begin
      if wasString then
        i32 := GetEnumNameValue(fCustomTypeInfo,PropValue,StrLen(PropValue)) else <<<<<<<<<<<<<<<
        i32 := GetCardinal(PropValue);
      if i32<0 then
[...]

It should be:

        i32 := GetEnumNameValue(fCustomTypeInfo,PropValue,StrLen(PropValue), TRUE) else

Logging is making the opposite assumption:

procedure TSynLog.LogInternal(Level: TSynLogInfo; const aName: RawUTF8;
   aTypeInfo: pointer; var aValue; Instance: TObject=nil);
begin
  if LogHeaderLock(Level,false) then
  try
    if Instance<>nil then
      fWriter.AddInstancePointer(Instance,' ',fFamily.WithUnitName);
    fWriter.AddFieldName(aName);
    fWriter.AddTypedJSON(aTypeInfo,aValue,true,true); <<<<<<<<<<<<<<<<<<
  finally
    LogTrailerUnLock(Level);
  end;
end;

HOWEVER, IMO the lowercase prefix shouldn't be trimmed in the first place, unless there is a corresponding option when de-serializing to specify whether or not the prefix is expected.

Therefore, because this looks like it has been broken for a long time (and unless I'm missing some other use cases), I propose that the prefix NOT be trimmed, as this is assuming everybody wants their enumerated types stripped of the prefix:

procedure TTextWriter.AddTypedJSON(aTypeInfo: pointer; const aValue;
  EnumSetsAsText,FullSetsAsStar: boolean);
var max, i: Integer;
    PS: PShortString;
begin
  case PTypeKind(aTypeInfo)^ of
    tkClass:
      WriteObject(TObject(aValue),[woFullExpand]);
    tkEnumeration:
      if EnumSetsAsText then begin
        Add('"');
        AddShort(GetEnumName(aTypeInfo,byte(aValue))^); <<<<<<<<<<<<<<<<
        Add('"');
      end else
        AddU(byte(aValue));

In my case, I want the enumerated type names *unmodified*, with the prefix left intact.

(Of course, the logging would need to change too)

procedure TSynLog.LogInternal(Level: TSynLogInfo; const aName: RawUTF8;
   aTypeInfo: pointer; var aValue; Instance: TObject=nil);
begin
  if LogHeaderLock(Level,false) then
  try
    if Instance<>nil then
      fWriter.AddInstancePointer(Instance,' ',fFamily.WithUnitName);
    fWriter.AddFieldName(aName);
    fWriter.AddTypedJSON(aTypeInfo,aValue,false,true); <<<<<<<<<<<<<<
  finally
    LogTrailerUnLock(Level);
  end;
end;

Thanks

Last edited by avista (2016-01-25 20:23:14)

ab

Administrator

From: France

Registered: 2010-06-21

Posts: 14,659

Website

Re: JSON Parser Bug - Enumerated Types

Good points.
There was some inconsistencies here.
I guess you are right: it does make more sense to serialize enumerates and sets with their FULL name.

See http://synopse.info/fossil/info/866f95be54
Note that I've also modified TJSONSerializer.WriteObject() so that it would also NOT trim the lower case type names any more...
Existing JSON content, with trimmed lowercase enumeration identifiers, would still be recognized by the framework.

Why did you disable EnumSetsAsText=false for logging?

avista

Member

Registered: 2014-01-06

Posts: 63

Re: JSON Parser Bug - Enumerated Types

Why did you disable EnumSetsAsText=false for logging?

That was unintentional.  I just meant to disable the prefix trimming.

Thank you for the updates!

ab

Administrator

From: France

Registered: 2010-06-21

Posts: 14,659

Website

Re: JSON Parser Bug - Enumerated Types

AFAIK EnumSetsAsText=false would write the enums as integers, not as text.

mpv

Member

From: Ukraine

Registered: 2012-03-24

Posts: 1,571

Website

Re: JSON Parser Bug - Enumerated Types

This changes is break my JavaScript/PHP/.NET clients. I serialize some structures to JSON and clients await trimmed enum. So this changes break my REST protocol. Many third-party developers use my services, so this is really BRAKING changes for me. Regression is very big, I can't rewrite third-party modules.
May be we add option or at last {$IFDEF} for this?

ab

Administrator

From: France

Registered: 2010-06-21

Posts: 14,659

Website

Re: JSON Parser Bug - Enumerated Types

I've introduced TTextWriter.SetDefaultEnumTrim to force enumeration text JSON serialization to trim lowercase identifiers, as before.
The breaking change could be now by-passed for the whole process.
I prefer using such a singleton/global setting, since it sounds better than compiler conditionals.
And you could set an item in the TTextWriter.CustomOptions for a given TTextWriter instance, if needed.
See http://synopse.info/fossil/info/b19a32bb3b

Sorry for the inconvenience.
Any feedback is welcome.

mpv

Member

From: Ukraine

Registered: 2012-03-24

Posts: 1,571

Website

Re: JSON Parser Bug - Enumerated Types

Thank you! I'll test it ASAP

mpv

Member

From: Ukraine

Registered: 2012-03-24

Posts: 1,571

Website

Re: JSON Parser Bug - Enumerated Types

I test TTextWriter.SetDefaultEnumTrim(true);
Everything is OK. Thanks again.