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

Add warning when using an empty regexp in lineinfile #42013

Merged
merged 9 commits into from
Jun 30, 2018
2 changes: 2 additions & 0 deletions changelogs/fragments/lineinfile-empty-regexp.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
minor_changes:
- lineinfile - add warning when using an empty regexp (https://github.com/ansible/ansible/issues/29443)
5 changes: 5 additions & 0 deletions docs/docsite/rst/porting_guides/porting_guide_2.7.rst
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,11 @@ Major changes in popular modules are detailed here
:ref:`DEFAULT_SYSLOG_FACILITY`. If you have :ref:`DEFAULT_SYSLOG_FACILITY` configured, the
location of remote logs on systems which use journald may change.

* The ``lineinfile`` module was changed to show a warning when using an empty string as a regexp.
Since an empty regexp matches every line in a file, it will replace the last line in a file rather
than inserting. If this is the desired behavior, use ``'^'`` which will match every line and
will not trigger the warning.


Modules removed
---------------
Expand Down
30 changes: 17 additions & 13 deletions lib/ansible/modules/files/lineinfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ def present(module, dest, regexp, line, insertafter, insertbefore, create,
if module._diff:
diff['before'] = to_native(b('').join(b_lines))

if regexp:
if regexp is not None:
bre_m = re.compile(to_bytes(regexp, errors='surrogate_or_strict'))

if insertafter not in (None, 'BOF', 'EOF'):
Expand All @@ -268,7 +268,7 @@ def present(module, dest, regexp, line, insertafter, insertbefore, create,
m = None
b_line = to_bytes(line, errors='surrogate_or_strict')
for lineno, b_cur_line in enumerate(b_lines):
if regexp:
if regexp is not None:
match_found = bre_m.search(b_cur_line)
else:
match_found = b_line == b_cur_line.rstrip(b('\r\n'))
Expand Down Expand Up @@ -410,14 +410,14 @@ def absent(module, dest, regexp, line, backup):
if module._diff:
diff['before'] = to_native(b('').join(b_lines))

if regexp:
if regexp is not None:
bre_c = re.compile(to_bytes(regexp, errors='surrogate_or_strict'))
found = []

b_line = to_bytes(line, errors='surrogate_or_strict')

def matcher(b_cur_line):
if regexp:
if regexp is not None:
match_found = bre_c.search(b_cur_line)
else:
match_found = b_line == b_cur_line.rstrip(b('\r\n'))
Expand Down Expand Up @@ -478,32 +478,36 @@ def main():
path = params['path']
firstmatch = params['firstmatch']
regexp = params['regexp']
line = params.get('line', None)
line = params['line']

if regexp == '':
module.warn(
"The regular expression is an empty string, which will match every line in the file. "
"This may have unintended consequences, such as replacing the last line in the file rather than appending. "
"If this is desired, use '^' to match every line in the file and avoid this warning.")

b_path = to_bytes(path, errors='surrogate_or_strict')
if os.path.isdir(b_path):
module.fail_json(rc=256, msg='Path %s is a directory !' % path)

if params['state'] == 'present':
if backrefs and not regexp:
module.fail_json(msg='regexp= is required with backrefs=true')
if backrefs and regexp is None:
module.fail_json(msg='regexp is required with backrefs=true')

if params.get('line', None) is None:
module.fail_json(msg='line= is required with state=present')
if line is None:
module.fail_json(msg='line is required with state=present')

# Deal with the insertafter default value manually, to avoid errors
# because of the mutually_exclusive mechanism.
ins_bef, ins_aft = params['insertbefore'], params['insertafter']
if ins_bef is None and ins_aft is None:
ins_aft = 'EOF'

line = params['line']

present(module, path, regexp, line,
ins_aft, ins_bef, create, backup, backrefs, firstmatch)
else:
if not regexp and line is None:
module.fail_json(msg='one of line= or regexp= is required with state=absent')
if regexp is None and line is None:
module.fail_json(msg='one of line or regexp is required with state=absent')

absent(module, path, regexp, line, backup)

Expand Down
12 changes: 10 additions & 2 deletions test/integration/targets/lineinfile/tasks/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -720,6 +720,8 @@

###################################################################
# Issue 29443
# When using an empty regexp, replace the last line (since it matches every line)
# but also provide a warning.

- name: Deploy the test file for lineinfile
copy:
Expand All @@ -746,8 +748,14 @@
path: "{{ output_dir }}/test.txt"
register: result

- name: Assert that the file contents match what is expected
- name: Assert that the file contents match what is expected and a warning was displayed
assert:
that:
- insert_empty_regexp is changed
- result.stat.checksum == '3baeade8eb2ecf4b01d70d541e9b8258b67c7f9f'
- warning_message in insert_empty_regexp.warnings
- result.stat.checksum == '23555a98ceaa88756b4c7c7bba49d9f86eed868f'
vars:
warning_message: >-
The regular expression is an empty string, which will match every line in the file.
This may have unintended consequences, such as replacing the last line in the file rather than appending.
If this is desired, use '^' to match every line in the file and avoid this warning.