mirror of
https://github.com/dani-garcia/vaultwarden.git
synced 2026-04-25 17:25:57 +03:00
[GH-ISSUE #2767] vaultwarden ignores 404.html page #1374
Labels
No labels
SSO
Third party
better for forum
bug
bug
documentation
duplicate
enhancement
future Vault
future Vault
future Vault
good first issue
help wanted
low priority
notes
pull-request
question
troubleshooting
wontfix
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
starred/vaultwarden#1374
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @tessus on GitHub (Sep 24, 2022).
Original GitHub issue: https://github.com/dani-garcia/vaultwarden/issues/2767
Subject of the issue
When I enter a non existing path
https://vaultwarden.server.com/blabla, the following is logged:and I see the following:
instead of the 404.html page:
Deployment environment
Your environment (Generated via diagnostics page)
Config (Generated via diagnostics page)
Show Running Config
Environment settings which are overridden:
Steps to reproduce
https://vaultwarden.server.com/blablaExpected behaviour
Actual behaviour
Troubleshooting data
My httpd proxy config:
I use
ROCKET_PORT=8888instead of the default port.@tessus commented on GitHub (Sep 24, 2022):
I also tried to add
ErrorDocument 404 /404.htmlto the VirtualHost stanza, but it is royally ignored.@stefan0xC commented on GitHub (Sep 25, 2022):
I think you also need to enable
ProxyErrorOverrideto tell apache to load the 404 page.@tessus commented on GitHub (Sep 25, 2022):
Yep, you are correct, @stefan0xC
However, I would have hoped that the web service would redirect it. I haven't had the chance to look into the architecture between the binary and the web UI in more detail.
I only came across this project 2 days ago and I am not familiar with Rust.
@BlackDex commented on GitHub (Sep 25, 2022):
That option will break the functionality of the API, since it does return error messages when sending 4xx or 5xx. So that's not the right option.
@stefan0xC commented on GitHub (Sep 25, 2022):
You are right. Sorry, I have not checked for side effects. My pull request should fix the issue without breaking the API but I have not tested it beyond creating a send and deleting it. Is there a way to automatically test the API for regressions?
@BlackDex commented on GitHub (Sep 25, 2022):
@stefan0xC currently there isn't.
To test the 404 if that isn't breaking something in your current codebase you could test that with Send and upload a file, since it doesn't have the v2 file upload feature yet, that should generate a 404 and fall back to the old upload function.
Other then clicking through the interface and using other clients there isn't any way for testing. I was thinking about some options, but wasn't able to start on anything yet
@stefan0xC commented on GitHub (Sep 25, 2022):
Okay, I've added another 404 catcher for
/apicalls to return the same message as Rockets default json. Not sure if this is the best solution (we will probably have to wait on SergioBenitez/Rocket#453) or if the other routes also need a json result for 404 errors but checking the path as suggested in SergioBenitez/Rocket#694 seemed a bit hackish to me.Rockets default handler is created by this macro
github.com/SergioBenitez/Rocket@6778089c12/core/lib/src/catcher/catcher.rs (L331)@tessus commented on GitHub (Sep 25, 2022):
@BlackDex
Hmm, in this case something is wrong. The API should not return http error codes. The API itself should return 200 with a status code in the response. At least this is how API development usually works.
Unless I misundestood your statement.
e.g. so how does my Apache (reverse proxy) return a proper error message when the vaultwarden service is not running? That is for status code 503? The service can't return anything when it is not running.
You can also specify specific error codes:
Update:
This is the config that seems to work for me. Please note that without the
ProxyErrorOverride, I receive a 404 when trying to access/admin(unless I remove theLocationstanza).virtual host apache config
@BlackDex commented on GitHub (Sep 25, 2022):
@tessus I'm not sure why you think that is bad. 4xx errors are errors which can be returned by any web application. Normally a 5xx error is returned by a proxy server if it can't reach the endpoint, or the actual application when it has an internal server error, so that is all fine.
Only returning 200 and provide an error message is not really a standard way, those error codes are there for a reason.
@tessus commented on GitHub (Sep 25, 2022):
I think you misunderstood. I am talking about the API.
Let's say an API endpoint returns a list of items as the response. The search term was specified in the request payload. If the search term does not yield any results, the API must never, ever return the status code 404. The API must return status code 200 (because the API call succeeded), with the reponse that nothing was found and a status code (where you can e.g. also use 404).
@BlackDex commented on GitHub (Sep 25, 2022):
I'm not talking about 404 specific, there is also 401 for example.
And if you call an API endpoint that doesn't exist, you should receive a 404. So no, it's definitely not only 200.
Also, a developer can decide to return a 404 for a non existing resources with a json body containing the error message, thats all up to the developer. And I'm not saying either is good or bad.