package-data-downloader fails to process download requests

[AtesComp]

A persistent bug exists in the update notifier that affects the ttf-mscorefonts-installer and may also affect other packages. On boot, a user is presented with an update notification indicating that the ttf-mscorefonts-installer failed to install and needs installation:
---snip---
The following packages requested additional data downloads after package
installation, but the data could not be downloaded or could not be
processed.

  ttf-mscorefonts-installer

The download will be attempted again later, or you can try the download
again now. Running this command requires an active Internet connection.
---endsnip---
Running the command fails. Manual installation of the ttf-mscorefonts-installer does not correct the notification. The issue was traced to the /usr/lib/update-notifier/package-data-downloader script.

There are several issues in the package-data-downloader script:

1. The function process_download_requests() checks the hook date against the stamp date for a installation package, such as
    Hook File: /usr/share/package-data-downloads/ttf-mscorefonts-installer
    Stamp File: /var/lib/update-notifier/package-data-downloads/ttf-mscorefonts-installer
with the code:
---snip---
        try:
            hook_date = os.stat(file).st_mtime
            stamp_date = os.stat(stampfile).st_mtime
            if hook_date < stamp_date:
                continue
---endsnip---
The stamp file exist if there was a previous installation that did not produce a permanent failure. However, a normal failure sets up the condition to run this script. The failure condition is not cleared on success. The code related to determining success and failure is:
---snip---
                    result = subprocess.call(command)
                    if result:
                        # There's no sense redownloading if the script fails
                        permanent_failures.append(relfile)
                    else:
                        create_or_update_stampfile(stampfile)
---endsnip---
Therefore, in the event of a non-permanent failure, the stamp file is created or updated which prevents an followup download to correct the non-permanent failure.

2. The function process_download_requests() also tests for keywords 'script', 'should-download', 'url', and 'sha256' in the para list from the Hook file to determine action:
---snip---
        for para in hook.iter_paragraphs(open(file)):

* . . . . . if 'script' in para:
                if not files:
                    record_failure(relfile)
                    break
* . . . . . . . command = [para['script']]

* . . . . . . . if 'should-download' in para:
                    db = debconf.DebconfCommunicator('update-notifier')
                    try:
* . . . . . . . . . . . should = db.get(para['should-download'])
                        if should == "false":
                            # Do nothing with this file.
                            break
---endsnip---
---snip---
            # Not in a 'script' stanza, so we should have some urls
            try:
* . . . . . . . files.append(para['url'])
* . . . . . . . sums.append(para['sha256'])
            except Exception as e:
---endsnip---
where * note the lines affected. These keywords begin with an uppercase in the related files:
---snip---
Url: http://downloads.sourceforge.net/corefonts/webdin32.exe
Sha256: 64595b5abc1080fba8610c5c34fab5863408e806aafe84653ca8575bed17d75a

Script: /usr/lib/msttcorefonts/update-ms-fonts
Should-Download: msttcorefonts/accepted-mscorefonts-eula
---endsnip---

3. The function process_download_requests() tracks so called "failures" and "permanent_failures". When a failure occurs, the following code sets up the condition for reprocessing later using the function trigger_update_notifier():
---snip---
    previous_failures = existing_permanent_failures()

    # We only report about "permanent" failures when there are new ones,
    # but we want the whole list of permanently-failing hooks so when
    # we clobber the update-notifier file we don't lose information the
    # user may not have seen yet
    if permanent_failures:
        new_failures = False
        for failure in permanent_failures:
            if failure not in previous_failures:
                mark_hook_failed(failure, permanent=True)
                previous_failures.append(failure)
                new_failures = True
        if new_failures:
            trigger_update_notifier(previous_failures, permanent=True)

    # Filter out new failure reports for permanently-failed packages
    our_failures = [x for x in failures if x not in previous_failures]

    if our_failures:
        for failure in our_failures:
            mark_hook_failed(failure)
        trigger_update_notifier(our_failures)
---endsnip---

SOLUTION:
====================
The existing code set up several conditions based on the existence of the following files:
  stamp file
  notifier file
  notifier permanent file
The conditions are:
  EXISTS ( stamp file )
  EXISTS ( stamp file AND notifier file )
  EXISTS ( notifier file AND notifier permanent file )
  EXISTS ( notifier permanent file )

When no permanent failures are found (no notifier permanent file), the stamp file is set even when normal failures exist (notifier file). This causes the next script execution to skip to the next install item. In the event of success, failure conditions are cleared, but the notifier files are never cleared causing persistent requests to update that never get fulfilled.

For part 1, adding code handle the failure condition when a stamp file exists would be prudent:
---snip---
        try:
> . . . . . if not os.path.exists(NOTIFIER_FILE) and not os.path.exists(NOTIFIER_PERMANENT_FILE):
> . . . . . . . hook_date = os.stat(file).st_mtime
> . . . . . . . stamp_date = os.stat(stampfile).st_mtime
> . . . . . . . if hook_date < stamp_date:
> . . . . . . . . . continue
> . . . . . elif os.path.exists(stampfile):
> . . . . . . . os.unlink(stampfile)
---endsnip---
where > indicated the added/modded code. This corrects the problem by allowing any perceived failures, permanent or not, to reinstall the downloads. It always removes the stampfile so that if a permanent failure develops, it is handled correctly later.

For part 2, the paragraph keyword lookups may or may not care about case, but update the keywords:
    'script', 'should-download', 'url', 'sha256'
to:
    'Script', 'Should-Download', 'Url', 'Sha256'
so that they match the file.

For part 3, the comment suggests that the so called "update-notifier file" will be clobbered, but it never is. On a non-permament failure, the failed and permanent-failure marker files are removed when function create_or_update_stampfile() is called. The code should be changed to handle cleared failure conditions (i.e., empty failure lists) as follows:
---snip---
    previous_failures = existing_permanent_failures()

    # We only report about "permanent" failures when there are new ones,
    # but we want the whole list of permanently-failing hooks so when
    # we clobber the update-notifier file we don't lose information the
    # user may not have seen yet
    if permanent_failures:
        new_failures = False
        for failure in permanent_failures:
            if failure not in previous_failures:
                mark_hook_failed(failure, permanent=True)
                previous_failures.append(failure)
                new_failures = True
        if new_failures:
            trigger_update_notifier(previous_failures, permanent=True)
> . if not previous_failures and os.path.exists(NOTIFIER_PERMANENT_FILE):
> . . . os.unlink(NOTIFIER_PERMANENT_FILE)

    # Filter out new failure reports for permanently-failed packages
    our_failures = [x for x in failures if x not in previous_failures]

    if our_failures:
        for failure in our_failures:
            mark_hook_failed(failure)
        trigger_update_notifier(our_failures)
> . elif os.path.exists(NOTIFIER_FILE):
> . . . os.unlink(NOTIFIER_FILE)
---endsnip---
where > indicated the added code.

In total, when no failure is found, the script will create the stamp file and clear the notifier files. For permanent failures, the stamp file is not created and any notifier files will persist until the next attempted update. For normal failures, the stamp and non-permanent notifier file persists until the next attempted update. On any update, if notifier files exist, the script will attempt to correct the install. If just the stamp file exists, then that install is skipped.

[Ubuntu Foundations Team Bug Bot (crichton)]

The attachment "Solution as per initial comment" seems to be a patch. If it isn't, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are a member of the ~ubuntu-reviewers, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issues please contact him.]

[Brian Murray (brian-murray)]

Thanks for discovering and working on this issue. This all sounds reasonable to me. Looking at the update-notifier package I see there is an automated test for package_data_downloader. Would you mind looking at writing a test for parts 1 and 3 of the problem you've identified? Thanks again!

[Launchpad Janitor (janitor)]

This bug was fixed in the package update-notifier - 3.173

---------------
update-notifier (3.173) yakkety; urgency=medium

  * data/package-data-downloader:
    - handle different failure modes better thanks to Launchpad user AtesComp
      for the detective work and patch. (LP: #1621629)
  * tests/test_pacakge-data-downloader.py:
    - add tests for process_download_requests function
    - add test for a hook file aging out
    - resolve test failure with test_wrong_template_translation
  * Enable autopkgtests and resolve pep8 / pyflakes issues with the package.

 -- Brian Murray <email address hidden> Wed, 21 Sep 2016 11:21:37 -0700

[Amr Ibrahim (amribrahim1987)]

It also happens with flashplugin-installer in Trusty 14.04. So I guess Xenial and Trusty are also affected.

[Brian Murray (brian-murray)]

There ended up being an issue with the proposed change and successful downloads being rerun if there is a failure of a different hook file. This was resolved in r891 of update-notifier and should be included in the SRU.

revno: 891
committer: Brian Murray <email address hidden>
branch nick: trunk
timestamp: Fri 2016-10-07 16:11:57 -0700
message:
  * data/package-data-downloader:
    - a stampfile is an indication of success, so don't try to process it
      again.
  * tests/test_package-data-downloader.py:
    - add a test for the case where there are multiple hook files and only one
      of them has failed.

[Martin Pitt (pitti)]

Hello AtesComp, or anyone else affected,

Accepted update-notifier into xenial-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/update-notifier/3.168.2 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 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 to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

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

[Amr Ibrahim (amribrahim1987)]

I have the same thing in Trusty 14.04. Here is a screenshot of the window. It appeared after logging in. Before that, there had been an update of adobe-flashplugin.

[Brian Murray (brian-murray)]

The autopkgtests all pass in Xenial, setting to verification-done.

http://autopkgtest.ubuntu.com/packages/u/update-notifier

[Launchpad Janitor (janitor)]

This bug was fixed in the package update-notifier - 3.168.2

---------------
update-notifier (3.168.2) xenial; urgency=medium

  * data/package-data-downloader:
    - handle different failure modes better thanks to Launchpad user AtesComp
      for the detective work and patch. (LP: #1621629)
  * tests/test_package-data-downloader.py:
    - add tests for process_download_requests function
    - add test for a hook file aging out
    - resolve test failure with test_wrong_template_translation

 -- Brian Murray <email address hidden> Thu, 06 Oct 2016 11:51:52 -0700

[Brian Murray (brian-murray)]

The verification of the Stable Release Update for update-notifier has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.