Bug#703240: apt: pkgTagSection.Exists sometimes lies about a field being present, .Find does not

[Niels Thykier <niels@thykier.net>]

Package: apt
Version: 0.9.7.8
Severity: normal

Hi,

There is a flaw in the handling of hash collisions in pkgTagSection's
"Exists" method that makes the "Exists" method return true even if the
"Tag" (field) is not present.

This is visible already in the header file, tagfile.h:

 """
   /* This very simple hash function for the last 8 letters gives
      very good performance on the debian package files */
   inline static unsigned long AlphaHash(const char *Text, const char *End = 0)
   [...]

   inline bool Exists(const char* const Tag) {return AlphaIndexes[AlphaHash(Tag)] != 0;}
"""

However, the "Find{,S}" methods do not have the same flaw.  Instead
they "retry" other locations for the field until they find the
intended tag (or run out of options).  Thus,

"""
   pkgTagSection t;
   const char sometag = "<insert-field-name>";
   const char *start, *end;
   ...
   if (t.Exists(sometag) != t.Find(sometag, start, end)) {
      /* This can happen, but most people would not anticipate this */
   }
"""

Attached is a prototype test case for test/libapt that triggers this
flaw.

~Niels

#include <apt-pkg/fileutl.h>
#include <apt-pkg/tagfile.h>

#include "assert.h"
#include <stdlib.h>
#include <string.h>

char *tempfile = NULL;
int tempfile_fd = -1;

void remove_tmpfile(void)
{
   if (tempfile_fd > 0)
      close(tempfile_fd);
   if (tempfile != NULL) {
      unlink(tempfile);
      free(tempfile);
   }
}

int main(int argc, char *argv[])
{
   FileFd fd;
   const char contents[] = "FieldA-12345678: the value of the field";
   std::atexit(remove_tmpfile);
   tempfile = strdup("apt-test.XXXXXXXX");
   tempfile_fd = mkstemp(tempfile);

   /* (Re-)Open (as FileFd), write and seek to start of the temp file */
   equals(fd.OpenDescriptor(tempfile_fd, FileFd::ReadWrite), true);
   equals(fd.Write(contents, strlen(contents)), true);
   equals(fd.Seek(0), true);

   pkgTagFile tfile(&fd);
   pkgTagSection section;
   equals(tfile.Step(section), true);
  
   /* It has one field */
   equals(section.Count(), 1);

   /* ... and it is called FieldA-12345678 */
   equals(section.Exists("FieldA-12345678"), true);

   /* its value is correct */
   equals(section.FindS("FieldA-12345678"), std::string("the value of the field"));
   /* A non-existent field has an empty string as value */
   equals(section.FindS("FieldB-12345678"), std::string());

   /* ... and Exists does not lie about missing fields... */
   equalsNot(section.Exists("FieldB-12345678"), true); 

   /* There is only one section in this tag file */
   equals(tfile.Step(section), false);

   /* clean up handled by atexit handler, so just return here */
   return 0;
}