Update docs to note potential risks/memory usage of LDAP avatar imports

florent4014 commented 6 days ago

Describe the Bug

Using the linuxserver.io Bookstack docker image, and the docker compose from the documentation (with some LDAP env variables in order to log with LDAP accounts). A user with like a 2MB profile pic will cause HTTP ERROR 500 when logging in to Bookstack app, thus causing the unavailability of the service and requiring a reboot.

I was able to eliminate the problem by adding memory_limit = 512M to php-local.ini. Here is the log of the error :

2024/09/27 23:25:21 [error] 320#320: *108 FastCGI sent in stderr: "PHP message: PHP Fatal error:  
Allowed memory size of 134217728 bytes exhausted (tried to allocate 24576 bytes) in /app/www/vendor/intervention/image/src/Drivers/Gd/Decoders/BinaryImageDecoder.php 
on line 43; PHP message: PHP Fatal error:  Allowed memory size of 134217728 bytes exhausted 
(tried to allocate 32768 bytes) in /app/www/vendor/laravel/framework/src/Illuminate/Foundation/Exceptions/Handler.php 
on line 402" while reading response header from upstream, client: 192.168.1.56, server: _, 
request: "GET / HTTP/1.1", upstream: "fastcgi://127.0.0.1:9000", host: "bookstack.domain.lan"

Steps to Reproduce

Run a dockerized version of bookstack
Set up a LDAP account with >2MB profile pic
Try to connect to Bookstack
Enjoy ERROR 500

Expected Behaviour

Being able to log without causing critical behaviour

Screenshots or Additional Context

No response

Browser Details

BRAVE Version 1.70.119 Chromium: 129.0.6668.70 (64 bits)

Exact BookStack Version

BookStack v24.05.4

ssddanbrown commented 5 days ago

Yeah, this will happen during the image resize process with large source images. Memory exhaustion limits are a pain to handle, since the app/framework jumps to a shutdown state in out of memory scenarios rather than it being a normal catchable error like logical errors.

We could add specific handling (like we have done elsewhere in a few key areas) but I'm not sure if it's worthwhile to add complexity to logins for this rare edge-case of a scenario.

Another option is to skip certain potentially problematic images, but we don't really know the memory use beforehand.

florent4014 commented 5 days ago

I guess the less time consuming would be to put a warning in the documentation so that other users can be aware of this issue. Or maybe (i'm no dev) imagine a system, like you said, to ignore profile pics on resolution or size criteria. At least, once you spot the problem, it is easy to fix.

ssddanbrown commented 5 days ago

Yeah, that's a good idea. I'll update & tag this issue to focus on adding an comment for the setting in our docs to note/consider this scenario.

ssddanbrown commented 3 days ago

I've now updated the ldap docs to add a comment for that option to warn about possible liklihood of additional issues. I'll therefore close this off.

BookStackApp / BookStack