[elbe-devel] [PATCH 1/3] aptpkgutils: add our own version of fetch_binary, that does not crash

[Bastian Germann]

First of all:

Are you aware of the workaround that Olivier introduced for a related issue?

42b80640ad ("Avoid segfault on reading package's md5 attribute")

If you were not and the workaround is also applicable here, please use that.

Am 04.12.19 um 17:37 schrieb Torben Hohn:
> python-apt has a problem, with crashes, when md5 sums do not exist.
> This is the case for
> http://security.debian.org/debian-security buster/update
> 
> -----------------------------------------------------------------------------------------------
> Oct 20 07:39:21 in-target: Traceback (most recent call last):
> Oct 20 07:39:21 in-target:   File "/bin/elbe", line 55, in <module>
> Oct 20 07:39:21 in-target:     cmdmod.run_command(sys.argv[2:])
> Oct 20 07:39:21 in-target:   File "/usr/lib/python2.7/dist-packages/elbepack/commands/fetch_initvm_pkgs.py", line 108, in run_command
> Oct 20 07:39:21 in-target:     ElbeAcquireProgress(cb=None))
> Oct 20 07:39:21 in-target:   File "/usr/lib/python2.7/dist-packages/apt/package.py", line 867, in fetch_binary
> Oct 20 07:39:21 in-target:     if _file_is_same(destfile, self.size, self._records.md5_hash):
> Oct 20 07:39:21 in-target: SystemError: error return without exception set
> -----------------------------------------------------------------------------------------------
> 
> Prepare to fix this by copying Version.fetch_binary() from
> /usr/lib/python2.7/dist-packages/apt/package.py and changing it to sha256.

Fortunately that code is release under GPL-2.0-or-later which we can use
under GPL-3.0-or-later.

> 
> Theoretically, we also need a fixed version of fetch_sources, but the
> code the Sources is different, and the problem does not seem to

Leaving out "the Sources" results in an English sentence.

> happen there.
> 
> Signed-off-by: Torben Hohn <torben.hohn at linutronix.de>
> ---
>  elbepack/aptpkgutils.py | 45 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 45 insertions(+)
> 
> diff --git a/elbepack/aptpkgutils.py b/elbepack/aptpkgutils.py
> index 84dd0dce6..93098d13e 100644
> --- a/elbepack/aptpkgutils.py
> +++ b/elbepack/aptpkgutils.py
> @@ -5,6 +5,13 @@
>  #
>  # SPDX-License-Identifier: GPL-3.0-or-later
>  
> +import os
> +import logging
> +
> +import apt_pkg
> +import apt
> +from apt.package import FetchError
> +
>  MARKED_INSTALL = 0
>  MARKED_UPGRADE = 1
>  MARKED_DELETE = 2
> @@ -78,6 +85,44 @@ def pkgorigin(pkg):
>  
>      return origin
>  
> +def _file_is_same(path, size, sha256):
> +    # type: (str, int, str) -> bool
> +    """Return ``True`` if the file is the same."""
> +    if os.path.exists(path) and os.path.getsize(path) == size:
> +        with open(path) as fobj:
> +            return apt_pkg.sha256sum(fobj) == sha256
> +    return False
> +
> +def fetch_binary(version, destdir='', progress=None):
> +    # type: (str, AcquireProgress) -> str
> +    """Fetch the binary version of the package.
> +
> +    The parameter *destdir* specifies the directory where the package will
> +    be fetched to.
> +
> +    The parameter *progress* may refer to an apt_pkg.AcquireProgress()
> +    object. If not specified or None, apt.progress.text.AcquireProgress()
> +    is used.
> +
> +    taken from python-apt, and fixed up to use sha256.

The copyright and author information of that the original file should be
reproduced here. In my buster installation this is:

Copyright (c) 2005-2009 Canonical

Author: Michael Vogt <michael.vogt at ubuntu.com>

> +    """
> +    base = os.path.basename(version._records.filename)
> +    destfile = os.path.join(destdir, base)
> +    if _file_is_same(destfile, version.size, version._records.sha256_hash):
> +        logging.debug('Ignoring already existing file: %s', destfile)
> +        return os.path.abspath(destfile)
> +    acq = apt_pkg.Acquire(progress or apt.progress.text.AcquireProgress())
> +    acqfile = apt_pkg.AcquireFile(acq, version.uri, "SHA256:" + version._records.sha256_hash,  # type: ignore # TODO: Do not use MD5 # nopep8
> +                                  version.size, base, destfile=destfile)
> +    acq.run()
> +
> +    if acqfile.status != acqfile.STAT_DONE:
> +        raise FetchError("The item %r could not be fetched: %s",
> +                         acqfile.destfile,
> +                         acqfile.error_text)
> +
> +    return os.path.abspath(destfile)
> +
>  
>  class PackageBase(object):
>  
>