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

Disable Apache FallbackResource for faildumps #321

Merged
merged 2 commits into from
Apr 2, 2025

Conversation

andrewnicols
Copy link
Contributor

@andrewnicols andrewnicols commented Mar 31, 2025

Per docs in https://httpd.apache.org/docs/trunk/mod/mod_dir.html#fallbackresource the FallbackResource can be disabled using the {{disabled}} argument where it is set in a parent path.

Fixed #320

@andrewnicols andrewnicols force-pushed the fallbackResourceDisabled branch from 817bd68 to 76962d0 Compare April 1, 2025 06:23
@andrewnicols
Copy link
Contributor Author

Pinging @marinaglancy,

This should solve the issue. The docker image places the FallbackResource configuration in a file named fallback.conf which was being loaded after the apache-faildump.conf file and therefore overriding it, despite being less specific.

I've renamed the file to moodle-faildump.conf which should solve the issue. Can you test again please?

@marinaglancy
Copy link
Member

marinaglancy commented Apr 1, 2025

yes it works.
Btw, when I was testing http://localhost:6002/asdfasff/a.php serves r.php, http://localhost:6002/asdfasff serves r.php but http://localhost:6002/asdfasff.php says "File not found". Is it expected behavior?

@andrewnicols
Copy link
Contributor Author

andrewnicols commented Apr 1, 2025 via email

@andrewnicols
Copy link
Contributor Author

Okay, replying now to you redited comment instead of the mail I recived

So it's working in that the fallback is not applied to the _ Location. I'll look into the .php side of things.

@andrewnicols
Copy link
Contributor Author

le sigh: https://bz.apache.org/bugzilla/show_bug.cgi?id=52403#c7

If you use SetHandler or AddHandler with FallbackResource non-existent PHP file do not use the FallbackResource. With AddType instead of SetHandler or AddHandler the FallbackResource works as expected.

Response:

This would be expected. The alternate handler is responsible for the content. The FallbackResource applies only to the core file system (default) handler.

Totally undocumented behaviour.

There seem to be a couple of options to address this:

  1. Switch from SetHandler to AddType
  2. Wrap the SetHandler call in an conditional
  3. Switch to mod_rewrite (which is not recommended for this usage)
  4. Switch to ErrorHandler (which is not recommended for this usage)

Set the handler to use for files whose extension ends in `.php`.
This is a workaround for docker-library/php#1576
The `FallbackResource` is a default Handler action which is only used if a Handler has not already been applied.
The docker/php image sets the handler unconditionally for any file whose name matches `.php`.
This configuration block unsets the default handler, and instead only applies it if the file exists.
@andrewnicols
Copy link
Contributor Author

I've raised docker-library/php#1576 against the upstream Docker image. I've also raised a documentation issue against Apache: https://bz.apache.org/bugzilla/show_bug.cgi?id=69640

@marinaglancy
Copy link
Member

Sorry for editing the comment. I thought the fallback was not working but I was checking the "wrong" url. Good job finding the cause of it.

@andrewnicols
Copy link
Contributor Author

If you're happy with the solution I've proposed here, please review + merge :0)

@marinaglancy
Copy link
Member

I do not have formal permissions to review or merge in this repo.

I confirm that both patches fix respective issues.

@marinaglancy
Copy link
Member

@andrewnicols just one question. Sorry, I do not have a quick way to test it. Will there be the same problem with the fallback on moodle-php-apache when it is executed without moodle-docker (i.e. in GHA or gitlab ci)?

@paulholden
Copy link
Member

Thanks both!

@paulholden paulholden merged commit f93dccd into moodlehq:main Apr 2, 2025
111 of 133 checks passed
@andrewnicols
Copy link
Contributor Author

andrewnicols commented Apr 2, 2025 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Problems accessing faildump on 5.0dev - looks like routing conflict?
3 participants