Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] files that do not exists downloads index.php #420

Closed
1 task done
golles opened this issue Oct 13, 2023 · 8 comments · Fixed by #432
Closed
1 task done

[BUG] files that do not exists downloads index.php #420

golles opened this issue Oct 13, 2023 · 8 comments · Fixed by #432
Labels
work-in-progress Stale exempt

Comments

@golles
Copy link

golles commented Oct 13, 2023

Is there an existing issue for this?

  • I have searched the existing issues

Current Behavior

I'm not an expert in Nginx, so I hope I've provided enough information.

When I open the following URLs in a browser, this is what happens:
https://domain.tld/foo -> downloads foo (content is index.php)
https://domain.tld/foo.html -> downloads foo (content is index.php)
https://domain.tld/foo.php -> serves index.php

My index.php looks like this:

<?php
header("HTTP/1.0 404 Not Found");

I do not have an index.html file.

Expected Behavior

My assumption is that all the above examples would serve index.php.

I think the problem is with this line,
https://github.com/linuxserver/docker-swag/blob/master/root/defaults/nginx/site-confs/default.conf.sample#L51
When I change it to

try_files $uri /index.php$is_args$args;

Things happen as I expect.

Steps To Reproduce

I'm using the default config as provided here: https://github.com/linuxserver/docker-swag/blob/master/root/defaults/nginx/site-confs/default.conf.sample

See my steps in the current behavior above.

Environment

- OS: Ubuntu 22.04.3 LTS
- How docker service was installed: docker-compose

CPU architecture

x86-64

Docker creation

swag:
    container_name: swag
    restart: unless-stopped
    image: linuxserver/swag:latest
    cap_add:
      - NET_ADMIN
    volumes:
      - ${DATA_DIR}/swag:/config
    ports:
      - 80:80
      - 443:443
    environment:
      - EMAIL=${PROXY_CERT_EMAIL}
      - URL=${PROXY_CERT_URL}
      - SUBDOMAINS=${PROXY_CERT_SUBDOMAINS}
      - ONLY_SUBDOMAINS=${PROXY_CERT_ONLY_SUBDOMAINS}
      - VALIDATION=${PROXY_CERT_VALIDATION}
      - DNSPLUGIN=${PROXY_CERT_DNSPLUGIN}
      - PROPAGATION=${PROXY_CERT_PROPAGATION}
      - STAGING=${PROXY_CERT_STAGING}
      - DOCKER_MODS=linuxserver/mods:swag-dashboard|linuxserver/mods:swag-maxmind
      - MAXMINDDB_LICENSE_KEY=${PROXY_MAXMIND_LICENSE_KEY}
      - PUID=${PUID}
      - PGID=${PGID}
      - TZ=${TZ}

Container logs

N/A
@nemchik
Copy link
Member

nemchik commented Oct 14, 2023

If you remove

header("HTTP/1.0 404 Not Found");

from your index.php and put something like

echo "Hello World";

with the original nginx configuration shipped, does that work as expected?

The reason for the =404 in try_files is so that 404 pages can redirect to index.php to be handled. This is common with many php scripts, including for example WordPress, which has its own 404 pages.

@golles
Copy link
Author

golles commented Oct 15, 2023

If I replace it with the hello world example, the browser downloads foo (https://domain.tld/foo) with

<?php
echo "Hello World";

@nemchik
Copy link
Member

nemchik commented Oct 15, 2023

I have confirmed the behavior and done some searching as to the reason behind it but not found a concrete answer yet. I'll keep searching later. I am unsure about making the exact change you mentioned (removing =404 from try_files) as I believe it was put there intentionally as it is needed by some php scripts (we have other images using the same logic and they don't work without it).

If you can dig up any documentation on why nginx/php behave this way with this configuration feel free to cite it here and I'll see what we can do to adjust.

@golles
Copy link
Author

golles commented Oct 15, 2023

Like I mentioned, I'm not an expert in nginx, but I'm happy to do a bit of investigation as well

@LinuxServer-CI
Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity. This might be due to missing feedback from OP. It will be closed if no further activity occurs. Thank you for your contributions.

@nemchik
Copy link
Member

nemchik commented Nov 25, 2023

Leaving a note for myself to continue investigating:

try_files $fastcgi_script_name =404;

inside the php location and removing =404 from the try_files in the main location seems to work. I need to find an image that depends on the =404 and remove it, observe the break, then add the new line and confirm if it restores the expected behavior without downloading the php code.

@nemchik
Copy link
Member

nemchik commented Nov 28, 2023

Leaving a note for myself to continue investigating:

try_files $fastcgi_script_name =404;

inside the php location and removing =404 from the try_files in the main location seems to work. I need to find an image that depends on the =404 and remove it, observe the break, then add the new line and confirm if it restores the expected behavior without downloading the php code.

After a bit of testing this seems to be a working fix. @golles if you'd like to test the changes you can see a fully updated copy of the default.conf here https://github.com/linuxserver/docker-swag/blob/fbe212b67c8dd255e326daa30fd4517fed0523e4/root/defaults/nginx/site-confs/default.conf.sample and specifically the lines changed are easy to see in this PR #432

@golles
Copy link
Author

golles commented Nov 30, 2023

This looks good, awesome!

None php files give an empty body, with the PHP extension I get:

<html>

<head>
	<title>404 Not Found</title>
</head>

<body>
	<center>
		<h1>404 Not Found</h1>
	</center>
	<hr>
	<center>nginx</center>
</body>

</html>

But I guess that is expected as the request is handled by PHP in such a case.

@LinuxServer-CI LinuxServer-CI moved this from Issues to Done in Issue & PR Tracker Nov 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
work-in-progress Stale exempt
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants