Bug#703240: apt: pkgTagSection.Exists sometimes lies about a field being present, .Find does not
On Sun, Mar 17, 2013 at 03:26:33PM +0100, Niels Thykier wrote:
> Package: apt
> Version: 0.9.7.8
> Severity: normal
Thanks for your bugreport and your testcase!
[..]
> Attached is a prototype test case for test/libapt that triggers this
> flaw.
I pushed a fix (that also removes the inline as Julian suggested) to
lp:~mvo/apt/fix-tagfile-hash
Attached is a bzr bundle for code-review & feedback.
Cheers,
Michael
# Bazaar merge directive format 2 (Bazaar 0.90)
# revision_id: michael.vogt@ubuntu.com-20130318111035-xb3zjpbg9knv3p9r
# target_branch: sftp://mvo@bzr.debian.org/bzr/apt/apt/debian-wheezy/
# testament_sha1: f26237e646426aeb92b52c713f87bb8c6757dd94
# timestamp: 2013-03-18 12:12:20 +0100
# base_revision_id: egon@debian-devbox-20130314132643-9xymnu7o2pt5ysev
#
# Begin patch
=== modified file 'apt-pkg/tagfile.cc'
--- apt-pkg/tagfile.cc 2012-03-04 22:47:05 +0000
+++ apt-pkg/tagfile.cc 2013-03-18 11:10:35 +0000
@@ -282,10 +282,17 @@
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)];
=== modified file 'apt-pkg/tagfile.h'
--- apt-pkg/tagfile.h 2011-12-13 00:22:38 +0000
+++ apt-pkg/tagfile.h 2013-03-18 11:10:35 +0000
@@ -59,7 +59,7 @@
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 @@
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];}
=== modified file 'test/libapt/makefile'
--- test/libapt/makefile 2012-09-02 19:32:40 +0000
+++ test/libapt/makefile 2013-03-18 11:10:35 +0000
@@ -93,8 +93,15 @@
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)
+
=== added file 'test/libapt/tagfile_test.cc'
--- test/libapt/tagfile_test.cc 1970-01-01 00:00:00 +0000
+++ test/libapt/tagfile_test.cc 2013-03-18 11:10:35 +0000
@@ -0,0 +1,57 @@
+#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";
+ 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;
+}
# Begin bundle
IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWXRJb9EABIXfgBY0fXf//3/n
38q////6YAmOfWy+5xQABO13cNBTYYxMmq13hkkaDU9U8UMaNTyT1MEAaAMhoABppoDQxMmjRoDR
piMTQxDAmjTEGI0GEABjmE0BoDRowjQYjTEyYmgwjQMgGTASIiBT0ymQp41NTQyfqaHqaBpMQYJt
AINMTBFIU0yTU2DU0Yo2k9TBGmEGgAABoGgBJIENAEaAE0mjTE00jKemKZqeQg9Rkxqep6h48ywH
n88dJfQ0XH3ppp1ibMoQhSsnwLQ1YVaDbbBImeQxphTN482NDbmkS1LpQvy43FCNUV/Me5n0Z+vK
60EX7GXUnVNXWPuOjlnVwRg81Qy4Ahoo0NfFoAcgDHzMbjr/av5EfldQ8725BbWVlgHENC1phmDY
3ybp70tFdPa44npWQte9GuywhSdO2LnBSBvfKiDyl7XMbisKq6ZUls1ay2v2SoW3FrobUZRrI6y5
/RmCBYjRWzYk7nIugInTVXNiH8M4P3thGTVVFEHsoPHW9sFo6g919JF1jYSCw+uZa89lUkTvlovY
vud/u+carHt6ybQAuwcUTYa6gWOtEAdsLGbBd19jBeDjNwQOuoiTgVWbcM7wc+0hO75DBDFEZQHK
s8EFOkPwHFliN/grnLRo/fczbLDkuhRS28oQyZusl7tSOjh3+3jlquPW46j3YV8fhjS+J5IWDJZQ
dXpfLzJaR+HeA8TCTDDqRazTockxRXqNZ5z8WZovInFeCsHAoHiXUGtcG8n3TfRuLN9EF52pBfaL
Og7LRqlKtGwtGmnMfYl9ATDOebIfNkL1CONwH4fDveFPO7OwIQWZlopSNrJQDY7r4bSYsR3BHjVp
CbQHGftdRlLEBIKtQl1FXR2+p8oBVIIE6hMhk01XIXLiQKTJnRPrT3VJWYOTWQT6TBjDqcC4l959
8DqGBYjgTOtD0iopYzkSuEwOeQxzs6TGkzqQFCeYsdh7RNYmXhf7K1qj36yo0i41iV5RuOU8nmW7
I2WhWJ5QPLpbiONS31lWIrAFQOFYOwtKnM8TnlEAjc+ZQEE43ErE9imw7TGpK8gcOwLyZI12nD3L
zGeFDDq8VOKqGQFbtkh6A4iwHyWqGoTyx2ZShbNMlKQ5rs56zac7buH1BTDmCyuYyyDEliltaLLk
yhKoM6VdiBLpKNEbpays2K9FIrSV47PhTEz0NNmqghPHWi5qNFxcNcNFQCG0Uo1XjGIXReXECl1p
ePeclp000uZ2BkQC6JIcXBFRWdh3xLxU36MBaLmDNR8V9QzXZmDVWEKyoHFJScJIoa6omWiusFlq
KGH4qq6qjVW4K4MAaD6jqDNxHMhQPsjfqNBt23EWJ65BTbCAYytaVejXTIvkaiJoNN5fVUMFxWT6
wWYbQT+aropxcImJvL2BsWtFLTzoDVFphPTKKyJ2uqMUry62+O6Lpu9rXogyZpa53k7gOI65tRAv
S0Y15FDZiQ8QcJo25dxQnd2VIrV2iwgrgAzZUuyAuiSIIUARE5MvZBqcu7J7MxvSPBQV3LLtaOwc
EWkfsZcoDE/mQFP8FU8WNJtyKui3V7qP9H7kfyFxTzjRYKxDdIHpUIgzrDxcQpSlIsqfbkKpej9F
IHKPaHsjxDQ3h3eEeg6IiLg7yWkSiRCGvVMCRapFzjDvn6mRz1GSeM8hU4iKfwriD6xQ3N/QoaZa
JRLj6AsPik9B60rRfm35L8hFfINYfAKsMJoLHhgWC0n4/we2ItLHAIGpIq7KxXdtJFp6h43hhyPE
C4BzMdA7mOPhSbx4x0tQePMFuXxxPQS6F9tgulCzo6KQTHgxo0mDGjtIDjI+JsI6C8VvzF50vG5j
zb+WgNitjbDwu1SfTWN8UzUdREEQQV4JLy7FS9m+kHA+tujZEeoSoFqMJ8ipTN5rccCuUrBaSYtb
7vIX1IMBRKbTpd/cmbxU8FHuVXQasjmeLqQ4kZb8rf5dLzcNiz3g4jRT2EAoKGIQgwyjy5PezpJz
DDe9wLHWePn1vvhuo0G0q66eR6A2g5GORNdTIL0mC4wMdMMlias/eyTA0SvWo875UeO5u30LsV7I
Zxb8sEZ+5I21LhgR3OvbG7S4mJkyGDGN0SjVxjsNRVX4rV9G6KUma4LLFNPSdfdJIopU1v6enx7D
kc/IHUW7zBN7Q97d6jEkhjBD0de1wx6Q6NT0OKyNcwURYEh0bNeypM92OWvC2xocG0FKUDpkgcbi
cLp2QhGrmNXXJBCMhIFwnyksjFlTFPwTuNgbKk80a9MjA7z7ByjPxcCM8hh3EKSr0h3xjQ6+Rwpl
dDUJdiIEIYFqKin6tgKmxYJ4rIdas1XCUtWal8K4SISEr/xW8ew8a27XdESalOhpJhhqPQClx4lu
0f4aVuXYHxYNK4peoFl38y5ioPKmN8EBmLNx5GNTJFCSUWAMpaNjQb3GvXbyb2MpskdoVvE8JYpB
9wy8FOvwa9erRMPv1FpahhOzbhrei8PcuINcu9EqGNgY5qrtJmkSSmCJgUJXBFBMGC9kFDSnQTFu
8CnhpdaJ5BhVVQWKWphEpqWglCotoKirs4X+tejHlavr5klMOSSLA5MAZr7BOpO6InSBfLgFo7AD
JR7XruN0/z8wYz/nbuGTbzgOV/kBOeLLpYIHmBontL8kdTUlutXqHxueuztmwFXIEsAkiYwlyROY
Q9FFyLfqH3jounWy3sfPWpIMfNJHlwVOF6aw55f+H5LEHS1wpqTuyQtY9WPJEB0nvGeLpQSHHZuK
FGB3UqBRHHSkxpo86secWWIcFLFKQ0GbDDLCy5zCW9AodV245rMUqjKxIE4lBsn3QUGQ0vBtL7eU
h6jHxm4Yg4IolEHp9Qaw3IQrLQ/QDNZ9470xDwRkrIq9fpmExqb12Diqb94OwGAxaoZWsQ0Y4msS
1SJjZ4JYJhM8TlkhJtkDOEc4VYkeSjpE8qO3at6GB3/N/TBb3ITqcYXB53Y3guikHrqYiNEGS96F
3cbes83s0lkzTUpAvC2AVujIShIYWsmrKFq+fAuYr7SNkAtw6coPX5OVDBtCegJ8wu5IpwoSDokt
+iA=
Reply to: