Browse Source

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: bf53f39c9a
Closes: #909155
tags/debian/1.7.0_rc2
David Kalnischkies 2 years ago
parent
commit
6f1d622c84
2 changed files with 126 additions and 13 deletions
  1. +58
    -11
      apt-private/private-show.cc
  2. +68
    -2
      test/integration/test-bug-624218-Translation-file-handling

+ 58
- 11
apt-private/private-show.cc View File

@@ -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)


+ 68
- 2
test/integration/test-bug-624218-Translation-file-handling View File

@@ -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

Loading…
Cancel
Save