ENT-3417: Added allow_non_convergent to pattern_matching promise in edit_line#6025
ENT-3417: Added allow_non_convergent to pattern_matching promise in edit_line#6025victormlg wants to merge 2 commits intocfengine:masterfrom
Conversation
29c7642 to
0ebf812
Compare
larsewi
left a comment
There was a problem hiding this comment.
Please add an acceptance test :)
|
@nickanderson can you address the questions @victormlg posted in the PR description? |
0ebf812 to
dfa3472
Compare
cf-agent/files_editline.c
Outdated
| strlcpy(line_buff_cp, line_buff, sizeof(line_buff_cp)); | ||
| strlcpy(after, line_buff_cp + end_off, sizeof(after)); |
There was a problem hiding this comment.
Please check for truncation
|
|
||
| ###################################################### | ||
|
|
||
| bundle agent test |
There was a problem hiding this comment.
Since you are including default.cf.sub you can add the meta data with ticket number and description
Sorry, I had responded to @victormlg directly via slack. I don't love the name I am unsure about the non-descructive thing. Maybe it should be boxed in more specifically ... the use case is for the pattern similar to: sed -i 's/PORT=.*/PORT=22/' Basically, I don't know what is there now, but it should be PORT=22 |
Ticket: ENT-3417 Signed-off-by: Victor Moene <victor.moene@northern.tech>
2bb8035 to
56aac28
Compare
cf-agent/files_editline.c
Outdated
| strlcpy(line_buff_cp, line_buff, sizeof(line_buff_cp)); | ||
| strlcpy(after, line_buff_cp + end_off, sizeof(after)); |
Signed-off-by: Victor Moene <victor.moene@northern.tech>
56aac28 to
b48e42a
Compare
| { | ||
| int needed; | ||
| // Save portion of line after substitution: | ||
| needed = strlcpy(after, line_buff + end_off, sizeof(after)); |
Check failure
Code scanning / CodeQL
Suspicious 'sizeof' use High
| int needed; | ||
| // Save portion of line after substitution: | ||
| needed = strlcpy(after, line_buff + end_off, sizeof(after)); | ||
| if (needed >= sizeof(after)) |
Check failure
Code scanning / CodeQL
Suspicious 'sizeof' use High
| { | ||
| return false; | ||
| } | ||
| needed = snprintf(line_buff + start_off, sizeof(line_buff) - start_off, |
Check failure
Code scanning / CodeQL
Suspicious 'sizeof' use High
| } | ||
| needed = snprintf(line_buff + start_off, sizeof(line_buff) - start_off, | ||
| "%s%s", BufferData(replace), after); | ||
| if (needed < 0 || (size_t) needed >= (sizeof(line_buff) - start_off)) |
Check failure
Code scanning / CodeQL
Suspicious 'sizeof' use High
Added a new attribute
allow_non_convergentin replace_pattern promise to allow for idempotent non-convergent regex.I am a bit ensure on the implementation, but the idea is that we don't want to allow destructive non-convergent regex, such as:
This will replace and rematch the same line indefinitely:
PORT=22 #Some comment #Some comment #Some comment [...]So the idea was to let cfengine match and expand the pattern N times (as before), then we match and expand one more time (N+1), and if the total length of the line is greater, this means that the non-convergent regex is destructive and thus we need to error.