#1 2019-09-23 17:01:18

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

[FIXED] TSQLRestServer.AuthenticationUnregisterAll bug

Hi ab,

The AuthenticationUnregisterAll code is forget to update fHandleAuthentication as it do in AuthenticationUnregister (one method earlier in code).

Suggested fix:

mORMot.pas: 40576

Before

procedure TSQLRestServer.AuthenticationUnregisterAll;
begin
...
  fSessions.Safe.Lock;
  ObjArrayClear(fSessionAuthentication);
  fSessions.Safe.UnLock;
end;

After

procedure TSQLRestServer.AuthenticationUnregisterAll;
begin
...
  fSessions.Safe.Lock;
  ObjArrayClear(fSessionAuthentication);
  fHandleAuthentication := false;
  fSessions.Safe.UnLock;
end;

Last edited by Eugene Ilyin (2019-09-24 19:59:36)

Offline

#2 2019-09-24 09:00:05

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

Re: [FIXED] TSQLRestServer.AuthenticationUnregisterAll bug

Nice catch!

Please check https://synopse.info/fossil/info/3240971f86

Thanks for the feedback!

Offline

#3 2019-09-24 19:59:13

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

Re: [FIXED] TSQLRestServer.AuthenticationUnregisterAll bug

Confirmed, thanks for the fix!

Last edited by Eugene Ilyin (2019-09-24 21:22:24)

Offline

#4 2019-09-26 06:31:44

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

Re: [FIXED] TSQLRestServer.AuthenticationUnregisterAll bug

Hi ab,
It's another issue with TSQLHttpServer constructor (looks like).

In the recent release 1.18.5382:

mORMotHTTPServer.pas: 702

constructor TSQLHttpServer.Create(const aPort: AnsiString;
var i,j: integer;
    ServersRoot: RawUTF8;
...
begin
...
      for i := 0 to high(aServers) do
      with aServers[i].Model do begin
        ServersRoot := ServersRoot+' '+Root;
...

Where is the ServersRoot stack var initialized?
Why the compiler is not concerned of non initialized var usage? hmm

Last edited by Eugene Ilyin (2019-09-26 06:54:48)

Offline

#5 2019-09-26 09:47:11

pvn0
Member
From: Slovenia
Registered: 2018-02-12
Posts: 211

Re: [FIXED] TSQLRestServer.AuthenticationUnregisterAll bug

Eugene Ilyin wrote:

Where is the ServersRoot stack var initialized?
Why the compiler is not concerned of non initialized var usage?

The  ServersRoot variable is initialized by the compiler, in fact , only memory for the pointer is allocated and it is set to nil. Code is valid.

Edit: At least for FPC&AnsiString, the pointer is guaranteed to be Nil, regardless if variable is local or global.

Last edited by pvn0 (2019-09-26 10:01:37)

Offline

#6 2019-09-26 18:10:59

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

Re: [FIXED] TSQLRestServer.AuthenticationUnregisterAll bug

It is a perfectly safe and documented use in Delphi and FPC.
Local variables of managed types are always initialized on the stack: string, AnsiString, WideString, variant, interface. It is set to nil/'' by some hidden generated code.
And there is an hidden try..finally block generated by the compiler to release the local variable - e.g. decrement its reference counter of the string or interface.

Only non-managed types (like integer, class, double) are filled with what is currently on the stack, which may be 0, but is very likely some garbage values.

If you write:

begin
  ServersRoot := '';
  ...

this code will do nothing, since ServersRoot is already ''.
It will just slow down the execution (there is a _LStrClear call made).

Offline

#7 2019-09-26 21:46:38

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

Re: [FIXED] TSQLRestServer.AuthenticationUnregisterAll bug

ab, pvn0, ah, I see.

So funny, that I missed so basic Delphi & FPC fundamental specialty.
I was sure that anything in stack is a garbage initially.

But you right:

function Test: RawUTF8;
var
  S: RawUTF8;
begin
  Result := S;
end;

Gives us:

Test.dpr.86: begin
0000000000B00E10 55               push rbp
0000000000B00E11 4883EC30         sub rsp,$30
0000000000B00E15 488BEC           mov rbp,rsp
0000000000B00E18 48C7452800000000 mov qword ptr [rbp+$28],$0000000000000000
0000000000B00E20 48894D40         mov [rbp+$40],rcx
0000000000B00E24 90               nop
Test.dpr.87: Result := S;
0000000000B00E25 488B4D40         mov rcx,[rbp+$40]
0000000000B00E29 488B5528         mov rdx,[rbp+$28]
0000000000B00E2D E84E2291FF       call @LStrAsg
0000000000B00E32 90               nop
Test.dpr.88: end;
0000000000B00E33 488D4D28         lea rcx,[rbp+$28]
0000000000B00E37 E8541B91FF       call @LStrClr
0000000000B00E3C 488B4540         mov rax,[rbp+$40]
0000000000B00E40 488D6530         lea rsp,[rbp+$30]
0000000000B00E44 5D               pop rbp
0000000000B00E45 C3               ret

Now I see this Initialization (mov qword ptr [rbp+$28],$0000000000000000) and Finalization (call @LStrClr) auto code.

Thanks for clarification!
Will remove all nil/empty local stack managed-types initialization in my code in future.

Offline

Board footer

Powered by FluxBB