merged lp:~mvo/apt/fix-tagfile-hash

5 files changed, 81 insertions, 4 deletions

diff --git a/apt-pkg/tagfile.cc b/apt-pkg/tagfile.cc
index 79811899..1c79ee74 100644
--- a/apt-pkg/tagfile.cc
+++ b/apt-pkg/tagfile.cc
@@ -282,10 +282,17 @@ void pkgTagSection::Trim()
    for (; Stop > Section + 2 && (Stop[-2] == '\n' || Stop[-2] == '\r'); Stop--);
 }
 									/*}}}*/
+// TagSection::Exists - return True if a tag exists                	/*{{{*/
+bool pkgTagSection::Exists(const char* const Tag)
+{
+   unsigned int tmp;
+   return Find(Tag, tmp);
+}
+									/*}}}*/
 // TagSection::Find - Locate a tag					/*{{{*/
 // ---------------------------------------------------------------------
 /* This searches the section for a tag that matches the given string. */
-bool pkgTagSection::Find(const char *Tag,unsigned &Pos) const
+bool pkgTagSection::Find(const char *Tag,unsigned int &Pos) const
 {
    unsigned int Length = strlen(Tag);
    unsigned int I = AlphaIndexes[AlphaHash(Tag)];
diff --git a/apt-pkg/tagfile.h b/apt-pkg/tagfile.h
index fd24471c..4718f510 100644
--- a/apt-pkg/tagfile.h
+++ b/apt-pkg/tagfile.h
@@ -59,7 +59,7 @@ class pkgTagSection
    inline bool operator !=(const pkgTagSection &rhs) {return Section != rhs.Section;};
    
    bool Find(const char *Tag,const char *&Start, const char *&End) const;
-   bool Find(const char *Tag,unsigned &Pos) const;
+   bool Find(const char *Tag,unsigned int &Pos) const;
    std::string FindS(const char *Tag) const;
    signed int FindI(const char *Tag,signed long Default = 0) const ;
    unsigned long long FindULL(const char *Tag, unsigned long long const &Default = 0) const;
@@ -73,7 +73,7 @@ class pkgTagSection
    virtual void TrimRecord(bool BeforeRecord, const char* &End);
    
    inline unsigned int Count() const {return TagCount;};
-   inline bool Exists(const char* const Tag) {return AlphaIndexes[AlphaHash(Tag)] != 0;}
+   bool Exists(const char* const Tag);
  
    inline void Get(const char *&Start,const char *&Stop,unsigned int I) const
                    {Start = Section + Indexes[I]; Stop = Section + Indexes[I+1];}
diff --git a/debian/changelog b/debian/changelog
index 1ba066a3..59f01c5d 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -34,6 +34,11 @@ apt (0.9.7.8~exp3) UNRELEASEDexperimental; urgency=low
   * add new config options "Acquire::ForceIPv4" and 
     "Acquire::ForceIPv6" to allow focing one or the other
     (closes: #611891)
+  * lp:~mvo/apt/fix-tagfile-hash:
+    - fix false positives in pkgTagSection.Exists(), thanks to
+      Niels Thykier for the testcase (closes: #703240)
+    - this will require rebuilds of the clients as this used to
+      be a inline function
 
  -- Michael Vogt <mvo@debian.org>  Sun, 17 Mar 2013 19:46:23 +0100
 
diff --git a/test/libapt/makefile b/test/libapt/makefile
index 5e225f24..953e455e 100644
--- a/test/libapt/makefile
+++ b/test/libapt/makefile
@@ -93,8 +93,15 @@ SLIBS = -lapt-pkg
 SOURCE = cdromreducesourcelist_test.cc
 include $(PROGRAM_H)
 
-# text IndexCopy::ConvertToSourceList
+# test IndexCopy::ConvertToSourceList
 PROGRAM = IndexCopyToSourceList${BASENAME}
 SLIBS = -lapt-pkg
 SOURCE = indexcopytosourcelist_test.cc
 include $(PROGRAM_H)
+
+# test tagfile
+PROGRAM = PkgTagFile${BASENAME}
+SLIBS = -lapt-pkg
+SOURCE = tagfile_test.cc
+include $(PROGRAM_H)
+
diff --git a/test/libapt/tagfile_test.cc b/test/libapt/tagfile_test.cc
new file mode 100644
index 00000000..d12c74c9
--- /dev/null
+++ b/test/libapt/tagfile_test.cc
@@ -0,0 +1,58 @@
+#include <apt-pkg/fileutl.h>
+#include <apt-pkg/tagfile.h>
+
+#include "assert.h"
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.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";
+   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;
+}