From 5ce47646add41077872b9cd9e0e8a24874995aae Mon Sep 17 00:00:00 2001 From: psi / Ryo Hirafuji Date: Wed, 22 Jul 2020 04:00:21 +0900 Subject: [PATCH] cron - Allow non-ascii (UTF-8) chars in cron file paths and jobs (#70426) * Encode/Decode files in UTF-8 * Use helper function in ansible * Add an integration test * Use emoji in test data. * add changelog * Also support non-ascii chars in filepath and add tests about this. * Also use non-ascii chars in replaced text and ensure not to break cron syntax. * rename self.existing to self.n_existing * rename crontab.existing to crontab.n_existing --- .../70426-allow-non-ascii-chars-in-cron.yml | 2 + lib/ansible/modules/cron.py | 28 +++++---- test/integration/targets/cron/tasks/main.yml | 62 +++++++++++++++++++ 3 files changed, 79 insertions(+), 13 deletions(-) create mode 100644 changelogs/fragments/70426-allow-non-ascii-chars-in-cron.yml diff --git a/changelogs/fragments/70426-allow-non-ascii-chars-in-cron.yml b/changelogs/fragments/70426-allow-non-ascii-chars-in-cron.yml new file mode 100644 index 00000000000..65275d82dc0 --- /dev/null +++ b/changelogs/fragments/70426-allow-non-ascii-chars-in-cron.yml @@ -0,0 +1,2 @@ +bugfixes: + - cron - encode and decode crontab files in UTF-8 explicitly to allow non-ascii chars in cron filepath and job (https://github.com/ansible/ansible/issues/69492) diff --git a/lib/ansible/modules/cron.py b/lib/ansible/modules/cron.py index 781a75facfb..de7ade540bd 100644 --- a/lib/ansible/modules/cron.py +++ b/lib/ansible/modules/cron.py @@ -210,6 +210,7 @@ import sys import tempfile from ansible.module_utils.basic import AnsibleModule +from ansible.module_utils.common.text.converters import to_bytes, to_native from ansible.module_utils.six.moves import shlex_quote @@ -231,14 +232,16 @@ class CronTab(object): self.root = (os.getuid() == 0) self.lines = None self.ansible = "#Ansible: " - self.existing = '' + self.n_existing = '' self.cron_cmd = self.module.get_bin_path('crontab', required=True) if cron_file: if os.path.isabs(cron_file): self.cron_file = cron_file + self.b_cron_file = to_bytes(cron_file, errors='surrogate_or_strict') else: self.cron_file = os.path.join('/etc/cron.d', cron_file) + self.b_cron_file = os.path.join(b'/etc/cron.d', to_bytes(cron_file, errors='surrogate_or_strict')) else: self.cron_file = None @@ -250,9 +253,8 @@ class CronTab(object): if self.cron_file: # read the cronfile try: - f = open(self.cron_file, 'r') - self.existing = f.read() - self.lines = self.existing.splitlines() + f = open(self.b_cron_file, 'rb') + self.n_existing = to_native(f.read(), errors='surrogate_or_strict') f.close() except IOError: # cron file does not exist @@ -266,7 +268,7 @@ class CronTab(object): if rc != 0 and rc != 1: # 1 can mean that there are no jobs. raise CronTabError("Unable to read crontab") - self.existing = out + self.n_existing = out lines = out.splitlines() count = 0 @@ -277,7 +279,7 @@ class CronTab(object): self.lines.append(l) else: pattern = re.escape(l) + '[\r\n]?' - self.existing = re.sub(pattern, '', self.existing, 1) + self.n_existing = re.sub(pattern, '', self.n_existing, 1) count += 1 def is_empty(self): @@ -291,15 +293,15 @@ class CronTab(object): Write the crontab to the system. Saves all information. """ if backup_file: - fileh = open(backup_file, 'w') + fileh = open(backup_file, 'wb') elif self.cron_file: - fileh = open(self.cron_file, 'w') + fileh = open(self.b_cron_file, 'wb') else: filed, path = tempfile.mkstemp(prefix='crontab') os.chmod(path, int('0644', 8)) - fileh = os.fdopen(filed, 'w') + fileh = os.fdopen(filed, 'wb') - fileh.write(self.render()) + fileh.write(to_bytes(self.render())) fileh.close() # return if making a backup @@ -628,7 +630,7 @@ def main(): if module._diff: diff = dict() - diff['before'] = crontab.existing + diff['before'] = crontab.n_existing if crontab.cron_file: diff['before_header'] = crontab.cron_file else: @@ -724,8 +726,8 @@ def main(): changed = True # no changes to env/job, but existing crontab needs a terminating newline - if not changed and crontab.existing != '': - if not (crontab.existing.endswith('\r') or crontab.existing.endswith('\n')): + if not changed and crontab.n_existing != '': + if not (crontab.n_existing.endswith('\r') or crontab.n_existing.endswith('\n')): changed = True res_args = dict( diff --git a/test/integration/targets/cron/tasks/main.yml b/test/integration/targets/cron/tasks/main.yml index 155fc2d5809..950d52dc4fc 100644 --- a/test/integration/targets/cron/tasks/main.yml +++ b/test/integration/targets/cron/tasks/main.yml @@ -122,3 +122,65 @@ - assert: that: not cron_file_stats.stat.exists + +- name: Allow non-ascii chars in job (#69492) + block: + - name: Cron file creation + cron: + cron_file: cron_filename + name: "cron job that contain non-ascii chars in job (これは日本語です; This is Japanese)" + job: 'echo "うどんは好きだがお化け👻は苦手である。"' + user: root + + - name: "Ensure cron_file contains job string" + replace: + path: /etc/cron.d/cron_filename + regexp: "うどんは好きだがお化け👻は苦手である。" + replace: "それは機密情報🔓です。" + register: find_chars + failed_when: (find_chars is not changed) or (find_chars is failed) + + - name: Cron file deletion + cron: + cron_file: cron_filename + name: "cron job that contain non-ascii chars in job (これは日本語です; This is Japanese)" + state: absent + + - name: Check file succesfull deletion + stat: + path: /etc/cron.d/cron_filename + register: cron_file_stats + + - assert: + that: not cron_file_stats.stat.exists + +- name: Allow non-ascii chars in cron_file (#69492) + block: + - name: Cron file creation with non-ascii filename (これは日本語です; This is Japanese) + cron: + cron_file: 'なせば大抵なんとかなる👊' + name: "integration test cron" + job: 'echo "Hello, ansible!"' + user: root + + - name: Check file exists + stat: + path: "/etc/cron.d/なせば大抵なんとかなる👊" + register: cron_file_stats + + - assert: + that: cron_file_stats.stat.exists + + - name: Cron file deletion + cron: + cron_file: 'なせば大抵なんとかなる👊' + name: "integration test cron" + state: absent + + - name: Check file succesfull deletion + stat: + path: "/etc/cron.d/なせば大抵なんとかなる👊" + register: cron_file_stats + + - assert: + that: not cron_file_stats.stat.exists