mORMot Open Source

WladiD

Member

From: Germany

Registered: 2010-10-27

Posts: 35

Little Bug, is it?

Hello Arnaud,

may be I found a little bug:

function TSQLRestServer.Retrieve(aID: integer; Value: TSQLRecord;
  ForUpdate: boolean): boolean;
var TableIndex: integer; // used by EngineRetrieve() for SQL statement caching
    Resp: RawUTF8;
    Static: TSQLRestServerStatic;
begin // this version handles locking and use fast EngineRetrieve() method
  // check parameters
  result := false;
  if Value=nil  then
    exit; // avoid GPF
  Value.fID := 0;
  if (self=nil) or (aID=0) then
    exit;
  TableIndex := Model.GetTableIndex(Value.RecordClass);
  if TableIndex<0 then
    exit;
  // try to lock before retrieval (if ForUpdate)
  if ForUpdate and not Model.Lock(TableIndex,aID) then
    exit;
  // get JSON object '{...}' in Resp
  Static := fStaticData[TableIndex]; // <-- Here, fStaticData may be nil or smaller TableIndex
  if Static<>nil then
    Resp := Static.EngineRetrieve(TableIndex,aID) else
    Resp := EngineRetrieve(TableIndex,aID);
  // fill Value from JSON if was correctly retrieved
  if Resp<>'' then begin
    Value.FillFrom(Resp);
    result := true;
  end;
end;

It's rising by me with the following code:

TSQLRecordImages = class(TSQLRecordMany);

TSQLRecordItem = class(TSQLRecord)
private
    FImages:TSQLRecordImages;
published
    property Images:TSQLRecordImages read FImages write FImages;
end;


function TSQLRecordItem.RetrievePictureURLs:TStringList;
var
    OneRecord:TSQLRecord;
begin    
    if not ((ID > 0) and (Images.FillMany(TSQLAccess.RestServer, ID) > 0)) then
        Exit(nil);
    Result:=TStringList.Create;
    while Images.FillOne do
    begin
        OneRecord:=TSQLAccess.RestServer.Retrieve(Images.Dest); // <--From this point to the upper posted code it happens
        try
            if Assigned(OneRecord) and (OneRecord is TSQLRecordImage) then
                Result.Add(UTF8ToString(TSQLRecordImage(OneRecord).URL));
        finally
            OneRecord.Free;
        end;
    end;
end;

I hope, I don't bother you.?

ab

Administrator

From: France

Registered: 2010-06-21

Posts: 14,240

Website

Re: Little Bug, is it?

Indeed!

Need to check the fStaticData array existence first!
I'll correct this tomorrow, and perform a full check in the source code that this error is not trigerred anywhere else in the code.

Nice catch!
And such detailed feedback will never bother me!

Thanks a lot

ab

Administrator

From: France

Registered: 2010-06-21

Posts: 14,240

Website

Re: Little Bug, is it?

OK I performed the check against the whole source code tree.

Sounds like all fStaticData[] access are now safe.
The issue you found was the only one I could identify.

Thanks for the feedback.

WladiD

Member

From: Germany

Registered: 2010-10-27

Posts: 35

Re: Little Bug, is it?

Perfect "team" work.

Thank you.