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.v...@ubuntu.com-20130318111035-xb3zjpbg9knv3p9r # target_branch: sftp://m...@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=