Rest delete on AuthUser delays to respond

damiand · 2021-02-24 15:45:01

When I send (as an Admin user) from a web application delete ajax requests to AuthUser the response is very slow, about 10 secs in a local development environment. The debug log shows no signs of execution delay:

20210224 13241602 # + mORMotSQLite3.TSQLRestServerDB(7ffff7f99590).URI DELETE root/AuthUser/11?session_signature=021c47f800d436046250611b in=0 B
20210224 13241602 # auth mORMot.TSQLRestRoutingREST(7ffff4c473d0) Admin/35407864 172.18.0.3
20210224 13241602 # cache SynSQLite3.TSQLDatabase(7ffff7fee220) bareserver.db3 cache flushed
20210224 13241602 # DB mORMotSQLite3.TSQLRestServerDB(7ffff7f99590) prepared 22us bareserver.db3 DELETE FROM AuthUser WHERE RowID=?;
20210224 13241602 # SQL mORMotSQLite3.TSQLRestServerDB(7ffff7f99590) 6.09ms DELETE FROM AuthUser WHERE RowID=:(11):;
20210224 13241602 # srvr mORMotSQLite3.TSQLRestServerDB(7ffff7f99590) Admin 172.18.0.3 DELETE root/AuthUser Write=200 out=0 B in 6.16ms
20210224 13241602 # - 00.006.186

But at the browser's side, when inspecting the network debugging info, at the Timings tab there is a 10.06 sec waiting time for the DELETE request to get its HTTP_OK (200) response code. Is this an on purpose security measure by mORMot or not?

ab · 2021-02-24 18:19:48

The request on server side takes only 6 milliseconds.
The delay is in-between.

Are you using the latest version of mORMot?
Do you use a proxy server?
On which OS is running the client and the server?
Did you try with another browser?
What does a plain GET request does to this server (172.18.0.xxx/root/timestamp)?
Did you try with a simple REST client?

On Windows, try to switch to the socket server.

damiand · 2021-02-25 12:05:10

My latest mORMot commit applied is 60aeaba63d770dcad2841aa0fe9e234d1982b51a from Feb 8 if I get it correctly (I'm not experienced with git). The whole installation is on OpenSUSE Linux Leap 15.2. The server is compiled with FPC 3.2.0. There is a reverse proxy indeed, because the client is an HTML page with jQuery, served by nginx hosted in a docker container, configured by ddev. The nginx is configured with TLS/SSL and as a reverse proxy to the mORMot server, located outside the container in the main operating system.

There is a similar behavior with Firefox and Chrome. A plain GET request to timestamp as you suggested takes 1ms to complete. Other CRU db actions to AuthUser using REST are performed rapidly. When I bypass the web server/docker container and load the html page from the filesystem, the delete is performed with no delay, provided that I have included also in the mORMot server compilation the AccessControlAllowOrigin := '*' setting. The latter is neither necessary in the previous (web hosted) setup, nor affects the delete performance. In conclusion, the delay resides in-between in the nginx reverse proxy, but I can't find out the reason yet.

ab · 2021-02-25 13:21:05

So I guess your nginx configuration may be not correct.
Please try locally, directly on the mORMot server.

BTW you should better not publish the ORM remote REST access to the clients, but encapsulate the ORM commands into proper services.
It is for instance very unsafe to publish the AuthUser/AuthGroup tables - any authenticated user may access the other users information - including hashed password....

damiand · 2021-02-25 14:05:25

ab wrote:

Please try locally, directly on the mORMot server.

I've already tried it - as I wrote in my previous post - and there was no delay, that's why I concluded that the problem must be in the middle.

ab wrote:

BTW you should better not publish the ORM remote REST access to the clients...

How can I prevent this publishing? I did nothing about it, that's the default way of operation. As far as I tested it - and as written in the documentation - only the Admin and Supervisor roles have access to these tables.

Thanks, ab.

ab · 2021-02-25 15:17:49

I usually publish the services using a TSQLRestServerFullMemory instance with nothing but the SOA interfaces published in it - i.e. a void model with no TSQLRecord in it.
Only this TSQLRestServerFullMemory is published over HTTP.
A JavaScript client will use the SOA interfaces (or method-based services if needed) directly, with high-level usecases.
This is called an application layer in KDD/DDD.

The ORM layer has a separated TSQLRestServerDB instance, with no HTTP publication.
The services hosted by the public TSQLRestServerFullMemory calls the ORM methods from the TSQLRestServerDB, safely on the server side.
Only for debugging/internal use, you may publish the main TSQLRestServerDB tables over REST.
This is called the domain + infrastructure in DDD or kingdom layer in KDD.

mpv · 2021-02-25 15:24:22

@damiand - try to replace in THttpSocket.GetBody
a block

if ContentLength<0 then begin // ContentLength=-1 if no Content-Length
...
end;

with

ContentLength := 0;

Does it help?

damiand · 2021-02-25 17:53:19

mpv wrote:

@damiand - try to replace in THttpSocket.GetBody
...
Does it help?

mpv I followed your advice and in line 6897 of SynCrtSock I made the following addition after the else:

  if ContentLength>0 then begin
...
  end else ContentLength := 0;
  if ContentLength<0 then begin // ContentLength=-1 if no Content-Length
...
  end;

And the problem solved! Do you think ab that this change should be applied also in the git repository? Many thanks.

ab · 2021-02-25 18:35:33

It solves your problem, but it would just ignore the whole process of a request with no Content-Length.

So it would break the use as client from some old servers existing in the Internet, which don't return any Content-Length but rely on the client to read all incoming socket until the socket is closed.
It is unorthodox, but it exists in the real world.
From the HTTP/1.1 RFC, the presence of a message-body is signaled by the inclusion of a Content-Length or Transfer-Encoding header (section 4.3).

@mpv What do you think?
My proposal may be to allow this loop only if IdemPChar(pointer(Command),'HTTP/1.0 200') then ... which means a parsing of HTTP/1.0 on client side only.
On server side, we would follow the RFC, so no Content-Length nor Transfer-Encoding Chunked would mean no body to retrieve.

Please try https://synopse.info/fossil/info/aba10df87c

damiand · 2021-02-25 19:58:49

ab wrote:

Please try https://synopse.info/fossil/info/aba10df87c

It works fine!

mpv · 2021-02-25 20:53:08

@ab - looks like we tries to fix a different things. I tries to fix a server socket (socket what receive http request message) and I still think my original fix is valid.
There is a good discussion in PR #334
Please, looks again at 3.3.3. Message Body Length - if there is no Transfer-Encoding: chunked AND no Content-Length header then paragraph 6 of RFC clearly say what

6. If this is a request message and none of the above are true, then the message body length is zero (no message body is present)

This is what I propose in my PR.

Paragraph 7 of RFC you mention in PR discussion is about "response message":

Otherwise, this is a response message without a declared message body length, so the message body length is determined by the number of octets received prior to the server closing the connection.

We never reach this condition for server HTTP socket.

BTW, I apply PR #334 to SyNode 7 month ago and it works perfectly without any problem on all my Linux productions. Clients are: browsers of all types (HTTP), MS Word/Excell, Nautilus (WebDAV), Java, Node, .NET, PHP, Python for API calls.

mpv · 2021-02-25 21:02:46

Currently we have too many "IFs" added by ceebf11b26 and dcb1a718f commits just instead of one line fix I initially propose. To be true I'm lost in it a little.

Last edited by mpv (2021-02-25 21:03:21)

ab · 2021-02-25 21:04:35

@mpv
This is exactly that my fix did:
- on server side (the command is something like 'GET /url/path HTTP/1.x'), always ignore the body if no Content-Length nor Chunked - this is what you did in PR #334;
- on client side (the command is 'HTTP/1.0 200'), only try to read the body in case of HTTP/1.0 response from an old server.

The RFC is about HTTP/1.1, and I have seen HTTP/1.0 servers which don't put neither COntent-Length nor Chunked transfer encoding for a GET!

So I guess my fix is fine. I checked the client side only thanks to the IdemPChar(pointer(command)) condition.

mpv · 2021-02-26 05:51:55

Now I understand what you did (I do not uses THttpClientSocket so do not dig deelpy into it code) . Now, after reading code several times, I think THttpSocket is too thick and propose a following:

1) create an individual GetBody method for THttpServerSocket and THttpClientSocket.
This allows not only remove unclear IdemPChar(command, HTTP/1.0 200) for server socket and replace it by ContentLength := 0 but also:
2) move a RegisterCompress (and fCompress, fCompressAcceptEncoding) into THttpClientSocket - for server socket we already have registered compressions in fServer instance and can use it
3) Move unneeded by server socket properties from THttpSocket into THttpClientSocket:
- Command - we need it only in GetBody and can use a local variable in server socket GetBody instead
- ServerInternalState - it is a client-side only
- XPoweredBy - it is a client-side only

As a result we decrease a server socket memory usage (important for many keep-alive connections for async server) and the code will become clearer. What do you think?

damiand · 2021-02-26 07:16:16

ab wrote:

I usually publish the services using a TSQLRestServerFullMemory instance with nothing but the SOA interfaces published in it - i.e. a void model with no TSQLRecord in it.

How do you handle user authentication/authorization at this level?

ab · 2021-02-26 08:26:31

In the public service, I do authorization as with any REST service.

Of course, you can use mORMot authentication - but disabling the REST ORM endpoints.
But for a public API, I usually define some authorization interface with regular login/logout endpoints, returning a JWT which is added to each request as 'Authentication: Bearer xxxxxx' header.

ab · 2021-02-26 08:35:24

@mpv
The memory decrease won't be huge. Only 4/8 bytes per string, because the strings are not filled. And the Command is always allocated and parsed anyway.
Overriding GetBody - or even easier adding the test in THttpClientSocket.Request - is a bit tricky because the server need ContentLength to be 0 from the parent GetBody, and the client needs -1... So a single expression seems simple enough, and on server side, ContentLength will always be >= 0 so it has no performance impact.
I will look into refactoring it on mORMot 2 for sure. But I have other ideas around, which may be beneficial for the performance, much better than such micro-optimizations. Thanks for the feedback.

mORMot Open Source

#1 2021-02-24 15:45:01

Rest delete on AuthUser delays to respond

#2 2021-02-24 18:19:48

Re: Rest delete on AuthUser delays to respond

#3 2021-02-25 12:05:10

Re: Rest delete on AuthUser delays to respond

#4 2021-02-25 13:21:05

Re: Rest delete on AuthUser delays to respond

#5 2021-02-25 14:05:25

Re: Rest delete on AuthUser delays to respond

#6 2021-02-25 15:17:49

Re: Rest delete on AuthUser delays to respond

#7 2021-02-25 15:24:22

Re: Rest delete on AuthUser delays to respond

#8 2021-02-25 17:53:19

Re: Rest delete on AuthUser delays to respond

#9 2021-02-25 18:35:33

Re: Rest delete on AuthUser delays to respond

#10 2021-02-25 19:58:49

Re: Rest delete on AuthUser delays to respond

#11 2021-02-25 20:53:08

Re: Rest delete on AuthUser delays to respond

#12 2021-02-25 21:02:46

Re: Rest delete on AuthUser delays to respond

#13 2021-02-25 21:04:35

Re: Rest delete on AuthUser delays to respond

#14 2021-02-26 05:51:55

Re: Rest delete on AuthUser delays to respond

#15 2021-02-26 07:16:16

Re: Rest delete on AuthUser delays to respond

#16 2021-02-26 08:26:31

Re: Rest delete on AuthUser delays to respond

#17 2021-02-26 08:35:24

Re: Rest delete on AuthUser delays to respond

Board footer