Don't use invalid iterator in Fallback-Of handling

cppcheck reports: (error) Iterator 't' used after element has been erased.

The loop is actually fashioned to deal with this (not in the most
efficient way, but in simplest and speed isn't really a concern here)
IF this codepath had a "break" at the end… so I added one.

Note that the tests aren't failing before (and hopefully after) the
change as the undefined behavior we encounter is too stable.

Thanks: David Binderman for reporting

3 files changed, 24 insertions, 2 deletions

diff --git a/apt-pkg/deb/debmetaindex.cc b/apt-pkg/deb/debmetaindex.cc
index 6cbed85a7..9c7c70784 100644
--- a/apt-pkg/deb/debmetaindex.cc
+++ b/apt-pkg/deb/debmetaindex.cc
@@ -1043,6 +1043,7 @@ class APT_HIDDEN debSLTypeDebian : public pkgSourceList::Type		/*{{{*/
 	    auto const tv = *t;
 	    mytargets.erase(t);
 	    mytargets.emplace_back(tv);
+	    break;
 	 }
 	 if (Changed == false)
 	    break;
diff --git a/test/integration/test-apt-acquire-additional-files b/test/integration/test-apt-acquire-additional-files
index ff04b6346..928c703e1 100755
--- a/test/integration/test-apt-acquire-additional-files
+++ b/test/integration/test-apt-acquire-additional-files
@@ -214,9 +214,12 @@ testequal "$(readfile Contents-all)" aptget indextargets --format '$(FILENAME)'
 testsuccess cmp "rootdir/var/lib/apt/lists/localhost:${APTHTTPPORT}_dists_unstable_main_Contents-all" 'aptarchive/dists/unstable/main/Contents-all'
 
 rm ./rootdir/var/lib/apt/lists/localhost:${APTHTTPPORT}_dists_unstable_main_Contents-all
-rm -f rootdir/etc/apt/apt.conf.d/content-target.conf
 msgmsg "No Contents file"
-
+cat > rootdir/etc/apt/apt.conf.d/content-target-notdefault.conf <<EOF
+Acquire::IndexTargets::deb::Contents::DefaultEnabled "no";
+Acquire::IndexTargets::deb::Contents2::DefaultEnabled "no";
+Acquire::IndexTargets::deb::Contents3::DefaultEnabled "no";
+EOF
 testequal "'http://localhost:${APTHTTPPORT}/dists/unstable/InRelease' localhost:${APTHTTPPORT}_dists_unstable_InRelease 0 
 'http://localhost:${APTHTTPPORT}/dists/unstable/main/source/Sources.xz' localhost:${APTHTTPPORT}_dists_unstable_main_source_Sources 0 
 'http://localhost:${APTHTTPPORT}/dists/unstable/main/binary-amd64/Packages.xz' localhost:${APTHTTPPORT}_dists_unstable_main_binary-amd64_Packages 0 
@@ -227,3 +230,17 @@ testsuccessequal "Hit:1 http://localhost:${APTHTTPPORT} unstable InRelease
 Reading package lists..." aptget update
 
 testempty find rootdir/var/lib/apt/lists -name '*Contents*'
+
+msgmsg "Order of Contents files"
+sameupdateoutput() {
+	testsuccessequal "'http://localhost:${APTHTTPPORT}/dists/unstable/InRelease' localhost:${APTHTTPPORT}_dists_unstable_InRelease 0 
+'http://localhost:${APTHTTPPORT}/dists/unstable/main/Contents-i386.xz' localhost:${APTHTTPPORT}_dists_unstable_main_Contents-i386 0 " aptget update --print-uris
+}
+sed -i -e 's#deb\(-src\)\? http#deb\1 [target=Contents3,Contents2,Contents] http#' rootdir/etc/apt/sources.list.d/*
+sameupdateoutput
+sed -i -e 's#deb\(-src\)\? \[target=[^]]\+\] http#deb\1 [target=Contents2,Contents3,Contents] http#' rootdir/etc/apt/sources.list.d/*
+sameupdateoutput
+sed -i -e 's#deb\(-src\)\? \[target=[^]]\+\] http#deb\1 [target=Contents3,Contents,Contents2] http#' rootdir/etc/apt/sources.list.d/*
+sameupdateoutput
+sed -i -e 's#deb\(-src\)\? \[target=[^]]\+\] http#deb\1 [target=Contents,Contents3,Contents2] http#' rootdir/etc/apt/sources.list.d/*
+sameupdateoutput
diff --git a/test/integration/test-sourceslist-target-plusminus-options b/test/integration/test-sourceslist-target-plusminus-options
index e4ce8e282..5c79af2d9 100755
--- a/test/integration/test-sourceslist-target-plusminus-options
+++ b/test/integration/test-sourceslist-target-plusminus-options
@@ -122,3 +122,7 @@ testtargets 'explicit + activate' 'Packages' 'Contents' 'Contents2'
 
 echo 'deb [Contents=yes,target+=Contents] http://example.org/debian stable rocks' > rootdir/etc/apt/sources.list
 testtargets 'duplications are okay' 'Packages' 'Translations' 'Contents' 'Contents2'
+
+# we can't check if it really reorders here
+echo 'deb [target+=Contents2,Contents] http://example.org/debian stable rocks' > rootdir/etc/apt/sources.list
+testtargets 'reordered fallback order' 'Packages' 'Translations' 'Contents' 'Contents2'