-
Notifications
You must be signed in to change notification settings - Fork 429
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
Added FileHandler for logging #1165
base: master
Are you sure you want to change the base?
Conversation
@Unrud Any reason why this shouldn't be merged? |
@@ -218,6 +218,11 @@ def _convert_to_bool(value): | |||
"value": "warning", | |||
"help": "threshold for the logger", | |||
"type": logging_level}), | |||
("logfile", { | |||
"value": "/tmp/radicale.log", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not part of the Radicale team, so this is a quick review by a random developer.
The string "/tmp/radicale.log" is considered True in Python. As such this will by default enable logging to "/tmp/radicale.log". As "/tmp" is a world-writable directory, if any other users exist on the system they can cause Radicale to overwrite any file writable by Radicale. In other words, I'm pretty sure this is a security hole.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Logging to /tmp
in an example is ugly.
@@ -110,6 +110,8 @@ | |||
# Value: debug | info | warning | error | critical | |||
#level = warning | |||
|
|||
logfile = /var/log/radicale.log |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this can trigger SELinux issues, better would be /var/log/radicale/radicale.log + a proper logwatch configuration for rotation, in case no autorotation is implemented
@ekaats : please rebase to latest Radicale 3 and also check all comments. |
Solves #1158
Added a FileHandler for logging. Two things I am not sure about: