linuxserver / docker-lychee

GNU General Public License v3.0
45 stars 16 forks source link

Fix upload processing bug #17

Closed mostlyjason closed 5 years ago

mostlyjason commented 5 years ago

This is to fix a bug that results in uploads failing. The issue is that lychee needs the temporary directory to be writable but it's not resulting in a 500 error. You find more details about the bug here https://github.com/LycheeOrg/Lychee/issues/180

linuxserver.io

Thanks, team linuxserver.io

thelamer commented 5 years ago

I made some modifications, @mostlyjason please pull and confirm functionallity here: https://hub.docker.com/r/lspipepr/lychee/tags/

aptalca commented 5 years ago

@mostlyjason

This is already performed by the baseimage: https://github.com/linuxserver/docker-baseimage-alpine-nginx/blob/master/root/etc/cont-init.d/20-config#L38

The issue you are observing is a bug with nginx. The band-aid fix is to set different folders for those temps. It affects a small subset of the users and no one on our team has been able to reproduce the issue, so we can't troubleshoot properly.

See here for the band-aid fix: https://github.com/linuxserver/docker-letsencrypt/issues/67#issuecomment-382715473

mostlyjason commented 5 years ago

@aptalca the chown is insufficient to fix the problem. I tested it and it requires chmod to fix the error. See the original commit before @thelamer changed it. https://github.com/linuxserver/docker-lychee/pull/17/commits/56f3d903e8f3d007edc94e9c06b3a28812c3db6e This is better than setting differnet temps because it maintains the conventional and expected temp file location. If the chmod doesn't hurt anyone and helps a subset of users why not merge it?

aptalca commented 5 years ago

I'm not convinced that chmod solves the issue, because 1) the issue is intermittent, and 2) it doesn't affect everyone. If it truly was due to incorrect permissions, everyone would be affected.

I have not been able to reproduce it, so I can't test or troubleshoot.

If you'd like to test further, you can map a 90-config file into /etc/cont-init.d and put inside the chmod command so it runs during each container start and will survive container updates/recreation

mostlyjason commented 5 years ago

@aptalca your intuition was correct. The error has reappeared even though I have 777 permissions on the directory. Must be the bug you referenced.

2019/02/09 21:40:02 [crit] 286#286: *527 open() "/var/tmp/nginx/client_body/0000000005" failed (13: Permission denied), client: 172.24.0.10, server: _, request: "POST /php/index.php HTTP/1.1", host: "mysite", referrer: "https://mysite/"
ubuntu:~/server-config/lychee$ d exec -it lychee_lychee_1 /bin/bash
root@7abb5046379c:/$ ls -la /var/tmp/nginx/client_body/
total 8
drwxrwxrwx 2 abc abc 4096 Feb  4 18:53 .
drwxrwxrwx 7 abc abc 4096 Feb  4 18:49 ..
root@7abb5046379c:/$ ls -la /var/tmp/nginx/
total 28
drwxrwxrwx 7 abc  abc  4096 Feb  4 18:49 .
drwxrwxrwt 4 root root 4096 Feb  4 18:49 ..
drwxrwxrwx 2 abc  abc  4096 Feb  4 18:53 client_body
drwxrwxrwx 2 abc  abc  4096 Feb  4 18:49 fastcgi
drwxrwxrwx 2 abc  abc  4096 Feb  4 18:49 proxy
drwxrwxrwx 2 abc  abc  4096 Feb  4 18:49 scgi
drwxrwxrwx 2 abc  abc  4096 Feb  4 18:49 uwsgi