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

Fix error reporting of templates #3660

Merged
merged 17 commits into from
May 4, 2021

Conversation

MrAnno
Copy link
Collaborator

@MrAnno MrAnno commented Apr 28, 2021

log_template_compile() has a return value, which can indicate compilation error.

Unfortunately, the return value was not checked in several places, producing repeated run-time errors and unexpected behavior.

For example:

options {
	create-dirs(yes);
};

log {
	source { stdin(); };
	destination {
		file("/tmp/${DATE");
	};

	flags(flow-control);
};

The above config snippet created a file named ${DATE inside the following directory structure:

$ tree .

error in template: /
└── tmp
    └── ${DATE

This is because the log_template_format() method called on an invalid template will contain the error itself (error in template: ...).

This PR adds proper error handling where templates are used, or where possible, just replaces the template creation logic with the template_content grammar rule, which takes care of error reporting:

Error parsing affile, Error compiling template, error=Invalid macro, '}' is missing, error_pos='11' in /home/anno/dev/syslog-ng/install/etc/syslog-ng.conf:39:8-39:21:
34      };
35
36      log {
37              source { stdin(); };
38              destination {
39---->                 file("/tmp/${DATE");
39---->                      ^^^^^^^^^^^^^

The PR contains some additional unit tests for non-trivial templates, and also makes empty templates (template("")) trivial and adds log_template_is_literal_string(), so we can get rid of the strchr($) trickery in the file destination.

@kira-syslogng
Copy link
Contributor

Build SUCCESS

lib/template/tests/test_template.c Outdated Show resolved Hide resolved
modules/affile/affile-dest.c Outdated Show resolved Hide resolved
@Kokan Kokan added this to the syslog-ng-3.32 milestone Apr 29, 2021
@MrAnno
Copy link
Collaborator Author

MrAnno commented Apr 29, 2021

@kira-syslogng do stresstest

@kira-syslogng
Copy link
Contributor

Build SUCCESS

@kira-syslogng
Copy link
Contributor

Kira-stress-test: Build SUCCESS

Kokan
Kokan previously approved these changes Apr 30, 2021
Copy link
Collaborator

@Kokan Kokan left a comment

Choose a reason for hiding this comment

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

Thanks.

MrAnno added a commit to MrAnno/syslog-ng that referenced this pull request Apr 30, 2021
Signed-off-by: László Várady <[email protected]>
@kira-syslogng
Copy link
Contributor

Build SUCCESS

Kokan
Kokan previously approved these changes May 3, 2021
@gaborznagy gaborznagy self-requested a review May 3, 2021 12:04
Copy link
Collaborator

@gaborznagy gaborznagy left a comment

Choose a reason for hiding this comment

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

Besides a minor thing, approve. 👍

lib/template/tests/test_template.c Outdated Show resolved Hide resolved
MrAnno added 8 commits May 4, 2021 16:50
When using the standard log_template_format() call, empty templates
are formatted into empty strings. This can be considered trivial, and
log_template_get_trivial_value() should produce the same output.

Signed-off-by: László Várady <[email protected]>
These methods can be used to determine whether the template contains
only literal values.

Signed-off-by: László Várady <[email protected]>
Literal strings are trivial, but instead of hard-coding this fact,
compile_literal_string() calls _calculate_triviality(), so the rules of
triviality are listed in one place.

Signed-off-by: László Várady <[email protected]>
file("/tmp/${DATE");

Previously, the above config snippet created the following file
structure:

$ tree .
error in template: /
└── tmp
    └── ${DATE

Signed-off-by: László Várady <[email protected]>
Signed-off-by: László Várady <[email protected]>
@kira-syslogng
Copy link
Contributor

Build SUCCESS

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.

4 participants