Deal with descriptions embedded in displayed record correctly

The implementation of "apt-cache show" (not "apt show") incorrectly
resets the currently used parser if the record itself and the
description to show come from the same file (as it is the case if no
Translation-* files are available e.g. after debootstrap).

The code is more complex than you would hope to support some rather
unusual setups involving Descriptions and their translations as tested
for by ./test-bug-712435-missing-descriptions as otherwise this could
be a one-line change.

Regression-Of: bf53f39c9a0221b670ffff74053ed36fc502d5a0
Closes: #909155

2 files changed, 126 insertions, 13 deletions

diff --git a/apt-private/private-show.cc b/apt-private/private-show.cc
index 15c05d420..b69008ec9 100644
--- a/apt-private/private-show.cc
+++ b/apt-private/private-show.cc
@@ -42,7 +42,7 @@ pkgRecords::Parser &LookupParser(pkgRecords &Recs, pkgCache::VerIterator const &
    return Recs.Lookup(Vf);
 }
 									/*}}}*/
-static APT_PURE char const *skipDescriptionFields(char const *DescP, size_t const Length) /*{{{*/
+static APT_PURE char const *skipDescription(char const *DescP, size_t const Length, bool fields) /*{{{*/
 {
    auto const backup = DescP;
    char const * const TagName = "\nDescription";
@@ -51,7 +51,7 @@ static APT_PURE char const *skipDescriptionFields(char const *DescP, size_t cons
    {
       if (DescP[1] == ' ')
 	 DescP += 2;
-      else if (strncmp((char*)DescP, TagName, TagLen) == 0)
+      else if (fields && strncmp((char *)DescP, TagName, TagLen) == 0)
 	 DescP += TagLen;
       else
 	 break;
@@ -78,19 +78,43 @@ static APT_PURE char const *findDescriptionField(char const *DescP, size_t const
    return DescP;
 }
 									/*}}}*/
+static APT_PURE char const *skipColonSpaces(char const *Buffer, size_t const Length) /*{{{*/
+{
+   // skipping withspace before and after the field-value separating colon
+   char const *const Start = Buffer;
+   for (; isspace(*Buffer) != 0 && Length - (Buffer - Start) > 0; ++Buffer)
+      ;
+   if (*Buffer != ':')
+      return nullptr;
+   ++Buffer;
+   for (; isspace(*Buffer) != 0 && Length - (Buffer - Start) > 0; ++Buffer)
+      ;
+   if (Length - (Buffer - Start) <= 0)
+      return nullptr;
+   return Buffer;
+}
+									/*}}}*/
 
 bool DisplayRecordV1(pkgCacheFile &, pkgRecords &Recs, /*{{{*/
-		     pkgCache::VerIterator const &V, pkgCache::VerFileIterator const &,
+		     pkgCache::VerIterator const &V, pkgCache::VerFileIterator const &Vf,
 		     char const *Buffer, size_t Length, std::ostream &out)
 {
-   if (unlikely(Length == 0))
+   if (unlikely(Length < 4))
       return false;
 
    auto const Desc = V.TranslatedDescription();
    if (Desc.end())
    {
-      // we have no translation output whatever we have got
-      return FileFd::Write(STDOUT_FILENO, Buffer, Length);
+      /* This handles the unusual case that we have no description whatsoever.
+	 The slightly more common case of only having a short-description embedded
+	 in the record could be handled here, but apt supports also having multiple
+	 descriptions embedded in the record, so we deal with that case later */
+      if (FileFd::Write(STDOUT_FILENO, Buffer, Length) == false)
+	 return false;
+      if (strncmp((Buffer + Length - 4), "\r\n\r\n", 4) != 0 &&
+	  strncmp((Buffer + Length - 2), "\n\n", 2) != 0)
+	 out << std::endl;
+      return true;
    }
 
    // Get a pointer to start of Description field
@@ -111,18 +135,41 @@ bool DisplayRecordV1(pkgCacheFile &, pkgRecords &Recs, /*{{{*/
    else
       snprintf(desctag, sizeof(desctag), "\nDescription-%s", langcode);
 
-   out << desctag + 1 << ": ";
+   out << desctag + 1 << ": " << std::flush;
    auto const Df = Desc.FileList();
    if (Df.end() == false)
    {
-      pkgRecords::Parser &P = Recs.Lookup(Df);
-      out << P.LongDesc();
+      if (Desc.FileList()->File == Vf->File)
+      {
+	 /* If we have the file already open look in the buffer for the
+	    description we want to display. Note that this might not be the
+	    only one we can encounter in this record */
+	 char const *Start = DescP;
+	 do
+	 {
+	    if (strncmp(Start, desctag + 1, strlen(desctag) - 1) != 0)
+	       continue;
+	    Start += strlen(desctag) - 1;
+	    Start = skipColonSpaces(Start, Length - (Start - Buffer));
+	    if (Start == nullptr)
+	       continue;
+	    char const *End = skipDescription(Start, Length - (Start - Buffer), false);
+	    if (likely(End != nullptr))
+	       FileFd::Write(STDOUT_FILENO, Start, End - (Start + 1));
+	    break;
+	 } while ((Start = findDescriptionField(Start, Length - (Start - Buffer))) != nullptr);
+      }
+      else
+      {
+	 pkgRecords::Parser &P = Recs.Lookup(Df);
+	 out << P.LongDesc();
+      }
    }
 
    out << std::endl << "Description-md5: " << Desc.md5() << std::endl;
 
    // Find the first field after the description (if there is any)
-   DescP = skipDescriptionFields(DescP, Length - (DescP - Buffer));
+   DescP = skipDescription(DescP, Length - (DescP - Buffer), true);
 
    // write the rest of the buffer, but skip mixed in Descriptions* fields
    while (DescP != nullptr)
@@ -150,7 +197,7 @@ bool DisplayRecordV1(pkgCacheFile &, pkgRecords &Recs, /*{{{*/
 	 ++End;
       }
       else
-	 DescP = skipDescriptionFields(End + strlen("Description"), Length - (End - Buffer));
+	 DescP = skipDescription(End + strlen("Description"), Length - (End - Buffer), true);
 
       size_t const length = End - Start;
       if (length != 0 && FileFd::Write(STDOUT_FILENO, Start, length) == false)
diff --git a/test/integration/test-bug-624218-Translation-file-handling b/test/integration/test-bug-624218-Translation-file-handling
index b629dd665..c116278ee 100755
--- a/test/integration/test-bug-624218-Translation-file-handling
+++ b/test/integration/test-bug-624218-Translation-file-handling
@@ -6,24 +6,78 @@ TESTDIR="$(readlink -f "$(dirname "$0")")"
 setupenvironment
 configarchitecture 'i386'
 
-buildsimplenativepackage 'coolstuff' 'all' '1.0' 'unstable'
+insertpackage 'unstable' 'unrelated' 'i386' '1'
+insertpackage 'unstable' 'ancientstuff' 'all' '1'
+insertpackage 'unstable' 'boringstuff' 'all' '1' '' '' 'shared short description'
+insertpackage 'unstable' 'coolstuff' 'all' '1'
+insertpackage 'unstable' 'dullstuff' 'all' '1' '' '' 'shared short description'
+insertpackage 'unstable' 'evilstuff' 'all' '1'
+insertpackage 'unstable' 'foostuff' 'all' '1' '' '' 'shared short description'
+insertpackage 'unstable' 'goodstuff' 'all' '1'
+insertpackage 'unstable' "longdesc" 'all' '1' '' '' "$(for i in $(seq 0 100); do printf '%s' 'lorem ipsum '; done)"
 
 setupaptarchive --no-update
-
 changetowebserver
+
+testsuccess aptget update
+PKGORDER='coolstuff foostuff coolstuff foostuff'
+msgtest 'Prepare expectation for' 'aptcache show'
+if aptcache show $PKGORDER | grep -v '^ ' > aptcacheshow.out 2>&1; then
+	msgpass
+else
+	cat aptcacheshow.out || true
+	msgfail
+fi
+testsuccessequal '4' grep -c '^Package: ' aptcacheshow.out
+msgtest 'Prepare expectation for' 'apt show'
+if apt show $PKGORDER | grep -v -e '^ ' -e '^[A-Z][a-z]\+-Size: ' > aptshow.out 2>&1; then
+	msgpass
+else
+	cat aptshow.out || true
+	msgfail
+fi
+testsuccessequal '4' grep -c '^Package: ' aptshow.out
 rm -rf rootdir/var/lib/apt/lists
 
+checkaptshow() {
+	testsuccess aptcache show $PKGORDER
+	sed -i -e 's#^Description: #Description-en: #' rootdir/tmp/testsuccess.output
+	testequal "$(cat aptcacheshow.out)
+" grep -v '^ ' rootdir/tmp/testsuccess.output
+
+	testsuccess apt show $PKGORDER
+	sed -i -e 's#^Description-en: #Description: #' rootdir/tmp/testsuccess.output
+	testequal "$(cat aptshow.out)
+" grep -v -e '^ ' -e '^[A-Z][a-z]\+-Size: ' rootdir/tmp/testsuccess.output
+
+	if [ -n "$(ls rootdir/var/lib/apt/lists/*Translation* 2>/dev/null)" ]; then
+		testsuccess find rootdir/var/lib/apt/lists/ -name '*Translation*' -delete
+
+		testsuccess aptcache show $PKGORDER
+		sed -i -e 's#^Description: #Description-en: #' rootdir/tmp/testsuccess.output
+		testequal "$(cat aptcacheshow.out)
+" grep -v '^ ' rootdir/tmp/testsuccess.output
+
+		testsuccess apt show $PKGORDER
+		sed -i -e 's#^Description-en: #Description: #' rootdir/tmp/testsuccess.output
+		testequal "$(cat aptshow.out)
+" grep -v -e '^ ' -e '^[A-Z][a-z]\+-Size: ' rootdir/tmp/testsuccess.output
+	fi
+}
+
 translationslisted() {
 	msgtest 'No download of non-existent locals' "$1"
 	export LC_ALL=""
 	testsuccess --nomsg aptget update -o Acquire::Languages=en
 	testfailure grep -q -e 'Translation-[^e][^n] ' rootdir/tmp/testsuccess.output
+	checkaptshow
 	rm -rf rootdir/var/lib/apt/lists
 
 	msgtest 'Download of existent locals' "$1"
 	testsuccess --nomsg aptget update
 	cp rootdir/tmp/testsuccess.output testsuccess.output
 	testsuccess grep -q -e 'Translation-en ' testsuccess.output
+	checkaptshow
 	rm -rf rootdir/var/lib/apt/lists
 
 	msgtest 'Download of en in LC_ALL=C' "$1"
@@ -31,6 +85,7 @@ translationslisted() {
 	testsuccess --nomsg aptget update
 	cp rootdir/tmp/testsuccess.output testsuccess.output
 	testsuccess grep -q -e 'Translation-en ' testsuccess.output
+	checkaptshow
 	rm -rf rootdir/var/lib/apt/lists
 	unset LC_ALL
 
@@ -38,21 +93,25 @@ translationslisted() {
 	testsuccess --nomsg aptget update -o Acquire::Languages=en
 	cp rootdir/tmp/testsuccess.output testsuccess.output
 	testsuccess grep -q -e 'Translation-en ' testsuccess.output
+	checkaptshow
 	rm -rf rootdir/var/lib/apt/lists
 
 	msgtest 'Download of nothing else in forced language' "$1"
 	testsuccess --nomsg aptget update -o Acquire::Languages=en
 	testfailure grep -q -e 'Translation-[^e][^n] ' rootdir/tmp/testsuccess.output
+	checkaptshow
 	rm -rf rootdir/var/lib/apt/lists
 
 	msgtest 'Download no Translation- if forced language is non-existent' "$1"
 	testsuccess --nomsg aptget update -o Acquire::Languages=ast_DE
 	testfailure grep -q -e 'Translation-' rootdir/tmp/testsuccess.output
+	checkaptshow
 	rm -rf rootdir/var/lib/apt/lists
 
 	msgtest 'Download of nothing if none is forced' "$1"
 	testsuccess --nomsg aptget update -o Acquire::Languages=none
 	testfailure grep -q -e 'Translation' rootdir/tmp/testsuccess.output
+	checkaptshow
 	rm -rf rootdir/var/lib/apt/lists
 }
 
@@ -66,26 +125,31 @@ echo 'Acquire::AllowInsecureRepositories "true";' >  rootdir/etc/apt/apt.conf.d/
 msgtest 'Download of en as forced language' 'without Index'
 testwarning --nomsg aptget update -o Acquire::Languages=en
 testsuccess grep -q -e 'Translation-en ' rootdir/tmp/testwarning.output
+checkaptshow
 rm -rf rootdir/var/lib/apt/lists
 
 msgtest 'Download of nothing else in forced language' 'without Index'
 testwarning --nomsg aptget update -o Acquire::Languages=en
 testfailure grep -q -e 'Translation-[^e][^n] ' rootdir/tmp/testwarning.output
+checkaptshow
 rm -rf rootdir/var/lib/apt/lists
 
 msgtest 'Download of ast_DE as forced language' 'without Index'
 testwarning --nomsg aptget update -o Acquire::Languages=ast_DE
 testsuccess grep -q -e 'Translation-ast_DE$' rootdir/tmp/testwarning.output
+checkaptshow
 rm -rf rootdir/var/lib/apt/lists
 
 msgtest 'Download of nothing else in forced language' 'without Index'
 testwarning --nomsg aptget update -o Acquire::Languages=ast_DE
 testfailure grep -q -e 'Translation-[^a][^s]' rootdir/tmp/testwarning.output
+checkaptshow
 rm -rf rootdir/var/lib/apt/lists
 
 msgtest 'Download of nothing if none is forced' 'without Index'
 testwarning --nomsg aptget update -o Acquire::Languages=none
 testfailure grep -q -e 'Translation' rootdir/tmp/testwarning.output
+checkaptshow
 rm -rf rootdir/var/lib/apt/lists
 
 mkdir -p rootdir/var/lib/apt/lists
@@ -94,6 +158,7 @@ touch rootdir/var/lib/apt/lists/localhost:${APTHTTPPORT}_dists_unstable_main_i18
 msgtest 'Download of builtin files' 'without Index'
 testwarning --nomsg aptget update
 testsuccess grep -q -e 'Translation-ast_DE' rootdir/tmp/testwarning.output
+checkaptshow
 rm -rf rootdir/var/lib/apt/lists
 
 mkdir -p rootdir/var/lib/apt/lists
@@ -102,4 +167,5 @@ touch rootdir/var/lib/apt/lists/localhost:${APTHTTPPORT}_dists_unstable_main_i18
 msgtest 'Download of nothing (even builtin) if none is forced' 'without Index'
 testwarning --nomsg aptget update -o Acquire::Languages=none
 testfailure grep -q -e 'Translation' rootdir/tmp/testwarning.output
+checkaptshow
 rm -rf rootdir/var/lib/apt/lists