Hi Markus,
thanks for pursuing this! Some minor nitpicks:
On Mon, Jan 29, 2018 at 12:11:03AM +0100, Markus Koschany wrote:
[..snip..]
> + if utils.is_security_update(package, pkgversion):
> + if ui.yes_no('Do you want to report a regression because of
> a security update? ',
> + 'Yes, please inform the LTS and security
> teams.',
> + 'No or I am not sure.', True):
> + regex = re.compile('(\+|~)deb(\d+)u(\d+)')
> + secversion = regex.search(pkgversion)
> + distnumber = secversion.group(2)
> + support = 'none'
Using
support = None
is a bit more pythonic.
> + email_address = []
> + try:
> + r =
> requests.get('https://security-tracker.debian.org/tracker/distributions.json',
> + timeout=self.options.timeout)
This will not catch 404 or similar http status codes. If you don't care
about the type of error you can just do …
r.raise_for_status()
… ff you dont handle the error this …
> + data = r.json()
… will fail like:
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/usr/lib/python2.7/dist-packages/requests/models.py", line 892, in json
return complexjson.loads(self.text, **kwargs)
File "/usr/lib/python2.7/dist-packages/simplejson/__init__.py", line 518, in
loads
return _default_decoder.decode(s)
File "/usr/lib/python2.7/dist-packages/simplejson/decoder.py", line 370, in
decode
obj, end = self.raw_decode(s)
File "/usr/lib/python2.7/dist-packages/simplejson/decoder.py", line 400, in
raw_decode
return self.scan_once(s, idx=_w(s, idx).end())
simplejson.errors.JSONDecodeError: Expecting value: line 1 column 1 (char 0)
> + for key, value in data.items():
> + if distnumber == value['major-version']:
> + if value['support']:
> + support = value['support']
> + if value['contact']:
> + email_address = value['contact']
> +
> + if support != 'none':
(With the abouve "support = None"):
if support is not None:
> + listcc += [email_address]
> +
… The raise_for_status exception is caught here alreadyx since
requests.exceptions.HTTPError is a subclass of
requests.exceptions.RequestException):
> + except requests.exceptions.RequestException:
> + ewrite('Unable to connect to
> security-tracker.debian.org.\n'
> + 'Please try again later or contact the LTS or
> security team via email directly.\n')
> +
It would also improve readability if this whole code block moved into a
get_security_contact()
function.
Cheers,
-- Guido
> if severity and rtype:
> severity = debbugs.convert_severity(severity, rtype)
>
> diff -Nru reportbug-7.1.8/debian/changelog
> reportbug-7.1.8+nmu1/debian/changelog
> --- reportbug-7.1.8/debian/changelog 2017-12-29 05:25:43.000000000 +0100
> +++ reportbug-7.1.8+nmu1/debian/changelog 2018-01-23 20:43:14.000000000
> +0100
> @@ -1,3 +1,10 @@
> +reportbug (7.1.8+nmu1) UNRELEASED; urgency=medium
> +
> + * Non-maintainer upload.
> + *
> +
> + -- Markus Koschany <a...@debian.org> Tue, 23 Jan 2018 20:43:14 +0100
> +
> reportbug (7.1.8) unstable; urgency=medium
>
> * reportbug/debbugs.py
> diff -Nru reportbug-7.1.8/reportbug/utils.py
> reportbug-7.1.8+nmu1/reportbug/utils.py
> --- reportbug-7.1.8/reportbug/utils.py 2017-12-29 05:25:43.000000000
> +0100
> +++ reportbug-7.1.8+nmu1/reportbug/utils.py 2018-01-23 20:43:14.000000000
> +0100
> @@ -39,6 +39,8 @@
> import socket
> import subprocess
> import pipes
> +import apt
> +import gzip
>
> from .urlutils import open_url
> from string import ascii_letters, digits
> @@ -1333,3 +1335,80 @@
> break
>
> return lsminfo
> +
> +
> +def is_security_update(pkgname, pkgversion):
> + """Determine whether a given package is a security update.
> +
> + Detection of security update versions works most reliably if the
> + package version under investigation is the currently installed
> + version. If this is not the case, the probability of false
> + negatives increases.
> +
> + Parameters
> + ----------
> + pkgname : str
> + package name
> + pkgversion : str
> + package version
> +
> + Returns
> + -------
> + bool
> + True if there is evidence that this version is a security
> + update, otherwise False
> + """
> + # Check 1:
> + # If it does not follow the debXuY version number pattern, it is
> + # definitely no security update.
> + #
> + # This check is not sufficient to detect security updates reliably,
> + # since other stable updates also use the same version pattern.
> + regex = re.compile('(\+|~)deb(\d+)u(\d+)')
> + secversion = regex.search(pkgversion)
> + if not secversion:
> + return False
> +
> + # Check 2:
> + # If the package comes from the Debian-Security package source, it
> + # is definitely a security update.
> + #
> + # This check does not identify all security updates, since some of
> + # them are distributed through the normal channels as part of a
> + # stable release update.
> + try:
> + p = apt.Cache()[pkgname]
> + if 'Debian-Security' in [o.label for o in
> + p.versions[pkgversion].origins]:
> + return True
> + except:
> + pass
> +
> + # Check 3:
> + # Inspect the package changelog if it mentions any vulnerability,
> + # identified by a CVE number, in the section of the latest version.
> +
> + cl = None
> + for cl in ['/usr/share/doc/{}/changelog.Debian.gz'.format(pkgname),
> + '/usr/share/doc/{}/changelog.gz'.format(pkgname)]:
> + if os.path.exists(cl):
> + break
> +
> + try:
> + with gzip.open(cl, 'rt') as f:
> + ln = f.readline()
> + if pkgversion not in ln:
> + raise KeyError
> +
> + for ln in f.readlines():
> + # stop reading at the end of the first section
> + if ln.rstrip() != '' and (ln.startswith(' -- ') or not
> ln.startswith(' ')):
> + break
> +
> + if 'CVE-20' in ln.upper():
> + return True
> + except:
> + pass
> +
> + # guess 'no security update, but normal stable update' by default
> + return False
