check_stamp() function of apt.systemd.daily should not assume interval is a number

Bug #1840995 reported by Ivan Kurnosov on 2019-08-21
10
This bug affects 1 person
Affects Status Importance Assigned to Milestone
apt (Ubuntu)
Medium
Unassigned
Disco
Undecided
Unassigned

Bug Description

[Impact]
Warning messages when using suffixes in intervals such as d for day

/usr/lib/apt/apt.systemd.daily: 87: [: Illegal number: 20h

[Test case]
Create 99local in apt.conf.d with
APT::Periodic::Update-Package-Lists "1d";
and run /usr/lib/apt/apt.systemd.daily - make sure no warning appears.

[Regression potential]
The fix replaces -eq 0 checks with = 0 checks which might have different behavior in case -eq also accepts some values as equal to 0 that are not literally 0 and that now no longer match. But then you'd have to do stuff like set the interval to "+0", and it seems unrealistic people do that.

[Original bug report]
In the second half of the function there is

    # Calculate the interval in seconds depending on the unit specified
    if [ "${interval%s}" != "$interval" ] ; then
        interval="${interval%s}"
    elif [ "${interval%m}" != "$interval" ] ; then
        interval="${interval%m}"
        interval=$((interval*60))
    elif [ "${interval%h}" != "$interval" ] ; then
        interval="${interval%h}"
        interval=$((interval*60*60))
    else
        interval="${interval%d}"
        interval=$((interval*60*60*24))
    fi

so, a variable might hold something like "1d", "100m", etc.

Yet in the first there is a condition

    if [ "$interval" -eq 0 ]; then
        debug_echo "check_stamp: interval=0"
        # treat as no time has passed
        return 1
    fi

which treats the value as a number and leads to

/usr/lib/apt/apt.systemd.daily: 87: [: Illegal number: 20h

Julian Andres Klode (juliank) wrote :

Oh, yes, but that's incomplete, there's a ton of other places that need adjusting too. In practice, the only non-number value supported is always.

Fixed in https://salsa.debian.org/apt-team/apt/commit/489da40d56075efaa28bfdcfb7b02b3bcc222323

Changed in apt (Ubuntu):
importance: Undecided → Medium
status: New → Fix Committed
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package apt - 1.9.4

---------------
apt (1.9.4) experimental; urgency=medium

  * CMake: Pass -Werror=return-type to gcc
  * CMake: Produce a fatal error if triehash could not be found
  * apt.systemd.daily: Do not numerically check if intervals equal 0
    (LP: #1840995)
  * srvrec: Use re-entrant resolver functions
  * Pass --abort-after=1 to dpkg when using --force-depends (Closes: #935910)
    (LP: #1844634)
  * Fix use of GTest to adjust for GTest 1.9

 -- Julian Andres Klode <email address hidden> Thu, 19 Sep 2019 11:13:47 +0200

Changed in apt (Ubuntu):
status: Fix Committed → Fix Released
description: updated

Hello Ivan, or anyone else affected,

Accepted apt into disco-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/apt/1.8.4 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-disco to verification-done-disco. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-disco. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

Changed in apt (Ubuntu Disco):
status: New → Fix Committed
tags: added: verification-needed verification-needed-disco

All autopkgtests for the newly accepted apt (1.8.4) for disco have finished running.
The following regressions have been reported in tests triggered by the package:

reprotest/0.7.8 (s390x)
gcc-snapshot/unknown (armhf)
apt/1.8.4 (amd64, armhf, s390x, ppc64el, arm64, i386)
autopkgtest/5.10ubuntu1 (amd64, i386)
gcc-7/7.4.0-8ubuntu1 (armhf)

Please visit the excuses page listed below and investigate the failures, proceeding afterwards as per the StableReleaseUpdates policy regarding autopkgtest regressions [1].

https://people.canonical.com/~ubuntu-archive/proposed-migration/disco/update_excuses.html#apt

[1] https://wiki.ubuntu.com/StableReleaseUpdates#Autopkgtest_Regressions

Thank you!

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers