Skip to content

Updates to configuration file parsing#383

Merged
cgzones merged 11 commits intologrotate:masterfrom
cgzones:config
Apr 30, 2021
Merged

Updates to configuration file parsing#383
cgzones merged 11 commits intologrotate:masterfrom
cgzones:config

Conversation

@cgzones
Copy link
Member

@cgzones cgzones commented Apr 20, 2021

No description provided.

cgzones added 11 commits April 20, 2021 19:02
The man page states
  Values are separated from directives by whitespace and/or an
  optional =.

But logrotate does accept no separator, like
  rotate7

Log those occurrences with a normal severity, as this usage is not
intended.
isolateWord() only fails on OOM and EOF.
Consider the following configuration:
    olddir /var/log/foo
    noolddir

Do not leak the memory of the initial olddir path.
Allow a tab in between the option keyword and its value, like
    dateformat\t.%Y%m%d

Currently the tab would be included in the format string, leading to
tabs in rotated files.

Also drop unnecessary continue block, since this is the default next
action.
Reiterating the loop is the default next action.
Fail on a parse error of a required option value of the directives
include, extension, addextension, rotate, start, minage, maxage,
shredcycles and su.
Failing is better than silently skipping a directive and running with an
undesired configuration.
Make it more visible.
No one sees debug logs if nothing fails.
Increase severity from debug to normal in case the configuration has a
potential dangerous file mode; make it more visible.
No one sees debug logs if nothing fails.
Copy link
Member

@kdudka kdudka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Thanks!

@cgzones cgzones merged commit 5f1640c into logrotate:master Apr 30, 2021
@cgzones cgzones deleted the config branch April 30, 2021 18:11
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.

2 participants