Skip to content

[18.0][FIX] auto_backup: open cached dump in binary mode#3659

Open
sebastienclaude wants to merge 1 commit into
OCA:18.0from
asc2si:18.0-fix-auto_backup-binary-copy
Open

[18.0][FIX] auto_backup: open cached dump in binary mode#3659
sebastienclaude wants to merge 1 commit into
OCA:18.0from
asc2si:18.0-fix-auto_backup-binary-copy

Conversation

@sebastienclaude

Copy link
Copy Markdown

Bug

When several db.backup records use method=local, only the first one performs the actual dump; the subsequent local records copy the file written by the first via shutil.copyfileobj(cached, destiny) in DbBackup.action_backup.

The source file cached is opened with open(backup), which defaults to text mode. Python then tries to UTF-8 decode the binary dump (ZIP archive or SQL stream containing non-UTF-8 bytes) and aborts with:

UnicodeDecodeError: 'utf-8' codec can't decode byte 0xd6 in position 12: invalid continuation byte

Symptom from the backup_log mail alert:

Traceback (most recent call last):
  File ".../auto_backup/models/db_backup.py", line 206, in backup_log
    yield
  File ".../auto_backup/models/db_backup.py", line 160, in action_backup
    shutil.copyfileobj(cached, destiny)
  File "/usr/lib/python3.12/shutil.py", line 203, in copyfileobj
    while buf := fsrc_read(length):
  File "<frozen codecs>", line 322, in decode
UnicodeDecodeError: 'utf-8' codec can't decode byte 0xd6 in position 12

Fix

The destination is already opened with "wb", so the fix is to open the cached source with "rb" too:

-                        with open(backup) as cached:
+                        with open(backup, "rb") as cached:
                             shutil.copyfileobj(cached, destiny)

Reproduction / validation

Reproduced on an Odoo 18 instance with two db.backup records, both method=local (one to a local disk, one to a CIFS-mounted share). Before the patch the second copy was a zero-byte file and the cron failed nightly. After the patch the second copy is byte-identical to the first dump (110 MB ZIP, same SHA), and no further UnicodeDecodeError is raised.

Other branches

The same defect is present on branches 16.0 and 17.0 of this module (identical code). Happy to open backport PRs if requested.

When several `db.backup` records use `method=local`, only the first one
performs the actual dump; the subsequent local records copy the file
written by the first via `shutil.copyfileobj(cached, destiny)`.

The source file `cached` was opened with `open(backup)`, which defaults
to text mode. Python then tries to UTF-8 decode the binary dump (ZIP or
SQL with non-UTF-8 bytes) and aborts with::

    UnicodeDecodeError: 'utf-8' codec can't decode byte 0xd6 in
    position 12: invalid continuation byte

The destination is already opened with `"wb"`, so the fix is to open
the cached source with `"rb"` too.

Reproduced and fixed on a production instance with two `db.backup`
records both in `method=local` (local disk + CIFS mount). After the
patch the second copy is byte-identical to the first dump.

The same defect is present on branches 16.0 and 17.0 of this module.

Signed-off-by: sebastienclaude <44704752+sebastienclaude@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants