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? |
Ticket: ENT-3417 Signed-off-by: Victor Moene <victor.moene@northern.tech>
Signed-off-by: Victor Moene <victor.moene@northern.tech>
0ebf812 to
dfa3472
Compare
| int needed = snprintf(line_buff_cp + start_off, sizeof(line_buff_cp) - start_off, | ||
| "%s%s", BufferData(replace), after); | ||
|
|
||
| if (needed >= sizeof(line_buff_cp) - start_off) { |
There was a problem hiding this comment.
Please make sure needed is not a negative integer before comparing it with an unsigned
| 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
| "%s%s", BufferData(replace), after); | ||
|
|
||
| if (needed >= sizeof(line_buff_cp) - start_off) { | ||
| RecordInterruption(ctx, pp, a, "Buffer overflow: replacement string is too large. '%s' in '%s'", |
There was a problem hiding this comment.
Maybe avoid printing buffer overflow. Because you are avoiding it. The remaining message is sufficient in my opinion
|
|
||
| ###################################################### | ||
|
|
||
| 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 |
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.