ETT-1189: Disable PHP error reporting in production; ETT-1140 show a styled 404 page #124
Conversation
We do not want php error output going to the user.
rewrite.php was removed in 64fd3d4 to no apparent ill effect. The redirect there to mirlyn-classic won't have worked for years, and the other rewrite there (with func=direct and a doc_number parameter) may have served for very obsolete forms of URLs. This may have been vestigial from when this code base was used for the Michigan catalog.
* Don't display PHP error messages on the 404 page * Use as ErrorDocument as well as as the default error handler in catalog
|
|
||
|
|
||
| RewriteRule ^robots.txt$ static/robots.txt [L] | ||
| RewriteRule ^F static/rewrite.php [L] |
aelkiss
changed the title
|
Playwright indicates that the API paths are returning a 200 when they should return a 404. I'll look at that, and maybe just go ahead and add a playwright test for the default error path in the catalog. |
|
@aelkiss the Bib API 404s in playwright seem to now be squeaking through index.php line 329. If I add |
| $module = (isset($_GET['module'])) ? $_GET['module'] : 'error'; | ||
|
|
||
| if ($module == 'api') { | ||
| exit(); |
There was a problem hiding this comment.
This seems to be responsible for the Bib API 200s where there should be 404s.
There was a problem hiding this comment.
Should put a 404 and maybe JSON payload?
There was a problem hiding this comment.
Probably. I'll check what it was doing before.
There was a problem hiding this comment.
I've restored the behavior it had before, which was a bare 404.
Commenting out the
php_valuestuff in.htaccesswas the main thing that gets the errors out of the web page;ErrorDocument 404plus the fixes in the PHP default error handler should handle the remaining 404 errors.To test this:
With an error, we'll get an unstyled default 500 page. This is better than a messy PHP error; I couldn't get
ErrorDocumentto work here, and I'm not sure why, but I also don't know how much it really matters. We can certainly keep an eye out to see what kinds of things trigger 500 errors in production.The fallthrough styled 404 also relies on
ErrorDocumentin.htaccess, so I don't have a good way to test that with playwright. We could test that e.g. "/foo/bar" gives a styled 404 with playwright, I suppose, if we want, since that's being handled by the application. Again, we can try it on dev-N with a variety of garbage URLs, all of which should give the 404 error page.@carylwyatt feel free to apply styling as desired in interface/themes/firebird/Error/Error404.tpl (and perhaps Record/error.tpl and Search/error.tpl?)