expose icon indicating copy to clipboard operation
expose copied to clipboard

Ignore request logging for upload POST requests

Open amazeika opened this issue 4 years ago • 2 comments

The problem

Right now any POST request containing a file payload is failling on my end and ends up crashing the expose client due to memory limitations.

The cause

This is happening because all requests seem to go through a logging process that require parsing the whole request body.

On multipart/form-data POST content this is not a very good idea since the body is likely to contain a big amount of data which the parser won't be able to handle.

Proposed solution

Avoid logging POST upload requests.

This fixes the problem completely on my end and I'm now able to perform upload requests without issues while using expose.

If logging these requests is a necesity then another approach must be used as to only grab part of request and avoiding parsing the whole body and it is being done right now.

I hope this helps and ends up being useful for you.

Related tickets

  • https://github.com/beyondcode/expose/issues/64

amazeika avatar Feb 12 '21 10:02 amazeika

You can already customize the settings up to which payload size you want to log requests. Isn't that what you want?

https://github.com/beyondcode/expose/blob/master/config/expose.php#L119

mpociot avatar Mar 02 '21 22:03 mpociot

You can already customize the settings up to which payload size you want to log requests. Isn't that what you want?

https://github.com/beyondcode/expose/blob/master/config/expose.php#L119

Hi, by the time those checks kick the request has already been parsed => https://github.com/beyondcode/expose/blob/1.5.1/app/Logger/LoggedRequest.php#L75

The call that's causing problems is this one => https://github.com/beyondcode/expose/blob/1.5.1/app/Logger/LoggedRequest.php#L69 which passes the whole request body through the CurlFormatter.

I see that in master it is a bit different but I still see the same problem happening. The calls go as follows:

  1. https://github.com/beyondcode/expose/blob/master/app/Client/Http/HttpClient.php#L52
  2. https://github.com/beyondcode/expose/blob/master/app/Logger/RequestLogger.php#L40
  3. https://github.com/beyondcode/expose/blob/master/app/Logger/RequestLogger.php#L77
  4. https://github.com/beyondcode/expose/blob/master/app/Logger/LoggedRequest.php#L66

My PR isn't complete, ideally you should remove the file part of the request as you certainly do not want on any circumstances passing the whole bin part of the body through any parser or logger.

Another thing on why I didn't consider that setting on config is that it says that's response related. Here the issue in question is the request, not the response. If this setting is also supposed to affect the request logging perhaps it should be called different. In any case and as I pointed out above, this setting won't currently have an effect on the problem.

amazeika avatar Mar 03 '21 10:03 amazeika