[Bug 1840995] Re: check_stamp() function of apt.systemd.daily should not assume interval is a number

[Julian Andres Klode]

** Description changed:

+ [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
+     # 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
+     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

-- 
You received this bug notification because you are a member of Ubuntu
Foundations Bugs, which is subscribed to apt in Ubuntu.
https://bugs.launchpad.net/bugs/1840995

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

Status in apt package in Ubuntu:
  Fix Released

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

To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/apt/+bug/1840995/+subscriptions