[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#988662: marked as done (unlock: apt/2.2.4)

Your message dated Fri, 18 Jun 2021 22:13:47 +0200
with message-id <74b55e71-0fa5-b29b-f445-c3a7f8a920c3@debian.org>
and subject line Re: Bug#988662: unblock: apt/2.2.4
has caused the Debian Bug report #988662,
regarding unlock: apt/2.2.4
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@bugs.debian.org
immediately.)


-- 
988662: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=988662
Debian Bug Tracking System
Contact owner@bugs.debian.org with problems
--- Begin Message --- 
Package: release.debian.org
Severity: normal
User: release.debian.org@packages.debian.org
Usertags: unblock
X-Debbugs-Cc: deity@lists.debian.org

Please unblock package apt


[ Reason ]

Bug fixes for JSON hooks, media change prompt, TLS handshakes,
and phased updates, to be more concrete, these 5 groups:

1 Several fixes for the JSON encoder to make it produce
  valid JSON output in more situations
2 Flushing the output streams before running JSON hooks
3 Avoid infinite loop on media change prompt (on EOF / 'c' character)
4 Turn TLS handshake issues into transient, rather than fatal errors
5 Make update phasing apply to uninstalled packages as well

2-5 are fairly trivial. 1 is potentially overkill for what we currently
send in terms of hooks, as it seems to work; but arguably, the JSON
encoding can end up fairly broken, it's a wonder this worked so far :D

[ Impact ]

1 JSON hooks might receive broken JSON
2 JSON hook output might appear in the wrong order relative to APT's.
  Which is bad, seeing how they are intended for close integration and
  make guarantees about where they are placed :/
3 Hanging apt program when it hits EOF (or c). Makes test suite hang
  when running in Debian containers inside lxd on Ubuntu, but also is
  odd if you script apt with </dev/null and it hangs instead of failing
  to media change.
4 Acquire::Retries does not work correctly for HTTPS sources that fail
  handshake; behavior of HTTPS sources on handshake issue is different
  from HTTP sources.
5 Installing new binaries from the same source package as
  already-installed binaries fails, if the binaries are phased and there
  are `=` dependency relations between them.


[ Tests ]

Added unit tests for the JSON encoder, and added/changed integration
tests for the other changes. 

Tests have passed on Salsa, for both root and regular user; as well
as local autopkgtest run.

[ Risks ]

1/2 The risk potential for the JSON hooks is restricted to the JSON
hooks themselves, and we don't ship any in the archive atm, nor does
stable have them, so users can't be regressed really.

3 Trivially correct change
4 Trivially correct change
5 While the change seems trivially correct, it might lead to unintended
  results in e.g. image building for people using phased updates. This
  remains to be seen.

[ Checklist ]
  [x] all changes are documented in the d/changelog
  [x] I reviewed all changes and I approve them
  [x] attach debdiff against the package in testing

[ Other info ]

If you follow the changelog entries to the LP issues, you'll
find a more detailed discussion of impact, tests, and regression
potential for the indivdual changes.

unblock apt/2.2.4

-- 
debian developer - deb.li/jak | jak-linux.org - free software dev
ubuntu core developer                              i speak de, en

diff -Nru apt-2.2.3/apt-pkg/policy.cc apt-2.2.4/apt-pkg/policy.cc
--- apt-2.2.3/apt-pkg/policy.cc	2021-04-13 17:53:32.000000000 +0200
+++ apt-2.2.4/apt-pkg/policy.cc	2021-05-17 16:20:59.000000000 +0200
@@ -310,7 +310,7 @@
 }
 APT_PURE signed short pkgPolicy::GetPriority(pkgCache::VerIterator const &Ver, bool ConsiderFiles)
 {
-   if (Ver.ParentPkg()->CurrentVer && Ver.PhasedUpdatePercentage() != 100)
+   if (Ver.PhasedUpdatePercentage() != 100)
    {
       if (ExcludePhased(d->machineID, Ver))
 	 return 1;
diff -Nru apt-2.2.3/apt-private/acqprogress.cc apt-2.2.4/apt-private/acqprogress.cc
--- apt-2.2.3/apt-private/acqprogress.cc	2021-04-13 17:53:32.000000000 +0200
+++ apt-2.2.4/apt-private/acqprogress.cc	2021-05-17 16:20:59.000000000 +0200
@@ -322,8 +322,10 @@
    while (C != '\n' && C != '\r')
    {
       int len = read(STDIN_FILENO,&C,1);
-      if(C == 'c' || len <= 0)
+      if(C == 'c' || len <= 0) {
 	 bStatus = false;
+	 break;
+      }
    }
 
    if(bStatus)
diff -Nru apt-2.2.3/apt-private/private-json-hooks.cc apt-2.2.4/apt-private/private-json-hooks.cc
--- apt-2.2.3/apt-private/private-json-hooks.cc	2021-04-13 17:53:32.000000000 +0200
+++ apt-2.2.4/apt-private/private-json-hooks.cc	2021-05-17 16:20:59.000000000 +0200
@@ -11,7 +11,9 @@
 #include <apt-pkg/macros.h>
 #include <apt-pkg/strutl.h>
 #include <apt-private/private-json-hooks.h>
+#include <apt-private/private-output.h>
 
+#include <iomanip>
 #include <ostream>
 #include <sstream>
 #include <stack>
@@ -23,7 +25,7 @@
 /**
  * @brief Simple JSON writer
  *
- * This performs no error checking, or string escaping, be careful.
+ * This performs no error checking, so be careful.
  */
 class APT_HIDDEN JsonWriter
 {
@@ -78,6 +80,7 @@
    void popState()
    {
       this->state = old_states.top();
+      old_states.pop();
    }
 
    public:
@@ -109,22 +112,40 @@
       os << '}';
       return *this;
    }
+   std::ostream &encodeString(std::ostream &out, std::string const &str)
+   {
+      out << '"';
+
+      for (std::string::const_iterator c = str.begin(); c != str.end(); c++)
+      {
+	 if (*c <= 0x1F || *c == '"' || *c == '\\')
+	    ioprintf(out, "\\u%04X", *c);
+	 else
+	    out << *c;
+      }
+
+      out << '"';
+      return out;
+   }
    JsonWriter &name(std::string const &name)
    {
       maybeComma();
-      os << '"' << name << '"' << ':';
+      encodeString(os, name) << ':';
       return *this;
    }
    JsonWriter &value(std::string const &value)
    {
       maybeComma();
-      os << '"' << value << '"';
+      encodeString(os, value);
       return *this;
    }
    JsonWriter &value(const char *value)
    {
       maybeComma();
-      os << '"' << value << '"';
+      if (value == nullptr)
+	 os << "null";
+      else
+	 encodeString(os, value);
       return *this;
    }
    JsonWriter &value(int value)
@@ -315,6 +336,13 @@
       return true;
    Opts = Opts->Child;
 
+   // Flush output before calling hooks
+   std::clog.flush();
+   std::cerr.flush();
+   std::cout.flush();
+   c2out.flush();
+   c1out.flush();
+
    sighandler_t old_sigpipe = signal(SIGPIPE, SIG_IGN);
    sighandler_t old_sigint = signal(SIGINT, SIG_IGN);
    sighandler_t old_sigquit = signal(SIGQUIT, SIG_IGN);
diff -Nru apt-2.2.3/CMakeLists.txt apt-2.2.4/CMakeLists.txt
--- apt-2.2.3/CMakeLists.txt	2021-04-13 17:53:32.000000000 +0200
+++ apt-2.2.4/CMakeLists.txt	2021-05-17 16:20:59.000000000 +0200
@@ -200,7 +200,7 @@
 # Configure some variables like package, version and architecture.
 set(PACKAGE ${PROJECT_NAME})
 set(PACKAGE_MAIL "APT Development Team <deity@lists.debian.org>")
-set(PACKAGE_VERSION "2.2.3")
+set(PACKAGE_VERSION "2.2.4")
 string(REGEX MATCH "^[0-9.]+" PROJECT_VERSION ${PACKAGE_VERSION})
 
 if (NOT DEFINED DPKG_DATADIR)
diff -Nru apt-2.2.3/debian/changelog apt-2.2.4/debian/changelog
--- apt-2.2.3/debian/changelog	2021-04-13 17:53:32.000000000 +0200
+++ apt-2.2.4/debian/changelog	2021-05-17 16:20:59.000000000 +0200
@@ -1,3 +1,24 @@
+apt (2.2.4) unstable; urgency=medium
+
+  * Various bugfixes to the JSON hooks:
+    - encoder fixes:
+      + json: Escape strings using \u escape sequences, add test
+      + json: Actually pop states
+      + json: Encode NULL strings as null
+    - json: Flush standard file descriptors before calling hooks
+      (this avoids output from hooks in middle of apt output)
+    - Non-installed JSON changes:
+      + test/json: Make the test hook more reliable
+      + Fix a typo in json-hooks-protocol.md (thanks to Brian Murray)
+  * Avoid infinite loop on EOF on media change prompt (LP: #1928687)
+  * Turn TLS handshake issues into transient errors (LP: #1928100),
+    this makes behavior consistent with TCP and enables Acquire::Retries
+  * policy: Apply phasing to uninstalled packages too (LP: #1925745),
+    this prevents inconsistencies when installing new binaries that depend
+    on the same version of an already installed binary.
+
+ -- Julian Andres Klode <jak@debian.org>  Mon, 17 May 2021 16:20:59 +0200
+
 apt (2.2.3) unstable; urgency=medium
 
   * tests: Check for and discard expected warning from MaybeAddAuth. For some
diff -Nru apt-2.2.3/doc/apt-verbatim.ent apt-2.2.4/doc/apt-verbatim.ent
--- apt-2.2.3/doc/apt-verbatim.ent	2021-04-13 17:53:32.000000000 +0200
+++ apt-2.2.4/doc/apt-verbatim.ent	2021-05-17 16:20:59.000000000 +0200
@@ -274,7 +274,7 @@
 ">
 
 <!-- this will be updated by 'prepare-release' -->
-<!ENTITY apt-product-version "2.2.3">
+<!ENTITY apt-product-version "2.2.4">
 
 <!-- (Code)names for various things used all over the place -->
 <!ENTITY debian-oldstable-codename "buster">
diff -Nru apt-2.2.3/doc/json-hooks-protocol.md apt-2.2.4/doc/json-hooks-protocol.md
--- apt-2.2.3/doc/json-hooks-protocol.md	2021-04-13 17:53:32.000000000 +0200
+++ apt-2.2.4/doc/json-hooks-protocol.md	2021-05-17 16:20:59.000000000 +0200
@@ -23,7 +23,7 @@
 1. Hook is started
 2. Hello handshake is exchanged
 3. One or more calls or notifications are sent from apt to the hook
-4. Bye notification is send
+4. Bye notification is sent
 
 It is unspecified whether a hook is sent one or more messages. For
 example, a hook may be started only once for the lifetime of the apt
diff -Nru apt-2.2.3/doc/po/apt-doc.pot apt-2.2.4/doc/po/apt-doc.pot
--- apt-2.2.3/doc/po/apt-doc.pot	2021-04-13 17:53:32.000000000 +0200
+++ apt-2.2.4/doc/po/apt-doc.pot	2021-05-17 16:20:59.000000000 +0200
@@ -5,9 +5,9 @@
 #, fuzzy
 msgid ""
 msgstr ""
-"Project-Id-Version: apt-doc 2.2.3\n"
+"Project-Id-Version: apt-doc 2.2.4\n"
 "Report-Msgid-Bugs-To: APT Development Team <deity@lists.debian.org>\n"
-"POT-Creation-Date: 2021-04-13 15:38+0000\n"
+"POT-Creation-Date: 2021-05-17 16:28+0200\n"
 "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
 "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
 "Language-Team: LANGUAGE <LL@li.org>\n"
diff -Nru apt-2.2.3/methods/connect.cc apt-2.2.4/methods/connect.cc
--- apt-2.2.3/methods/connect.cc	2021-04-13 17:53:32.000000000 +0200
+++ apt-2.2.4/methods/connect.cc	2021-05-17 16:20:59.000000000 +0200
@@ -1045,7 +1045,7 @@
    err = tlsFd->DoTLSHandshake();
 
    if (err < 0)
-      return ResultState::FATAL_ERROR;
+      return ResultState::TRANSIENT_ERROR;
 
    return ResultState::SUCCESSFUL;
 }
diff -Nru apt-2.2.3/po/apt-all.pot apt-2.2.4/po/apt-all.pot
--- apt-2.2.3/po/apt-all.pot	2021-04-13 17:53:32.000000000 +0200
+++ apt-2.2.4/po/apt-all.pot	2021-05-17 16:20:59.000000000 +0200
@@ -5,9 +5,9 @@
 #, fuzzy
 msgid ""
 msgstr ""
-"Project-Id-Version: apt 2.2.3\n"
+"Project-Id-Version: apt 2.2.4\n"
 "Report-Msgid-Bugs-To: APT Development Team <deity@lists.debian.org>\n"
-"POT-Creation-Date: 2021-04-13 15:38+0000\n"
+"POT-Creation-Date: 2021-05-17 16:28+0200\n"
 "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
 "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
 "Language-Team: LANGUAGE <LL@li.org>\n"
diff -Nru apt-2.2.3/test/integration/test-apt-cli-json-hooks apt-2.2.4/test/integration/test-apt-cli-json-hooks
--- apt-2.2.3/test/integration/test-apt-cli-json-hooks	2021-04-13 17:53:32.000000000 +0200
+++ apt-2.2.4/test/integration/test-apt-cli-json-hooks	2021-05-17 16:20:59.000000000 +0200
@@ -21,13 +21,12 @@
 
 cat >> json-hook.sh << EOF
 #!/bin/bash
+trap '' SIGPIPE
 while true; do
-	read request <&\$APT_HOOK_SOCKET
-	read empty <&\$APT_HOOK_SOCKET
+	read request <&\$APT_HOOK_SOCKET || exit 1
 
 	if echo "\$request" | grep -q ".hello"; then
 		echo "HOOK: HELLO"
-		printf '{"jsonrpc": "2.0", "result": {"version": "0.1"}, "id": 0}\n\n' >&\$APT_HOOK_SOCKET
 	fi
 
 	if echo "\$request" | grep -q ".bye"; then
@@ -36,7 +35,16 @@
 	fi
 
 	echo HOOK: request \$request
+
+	read empty <&\$APT_HOOK_SOCKET || exit 1
+
 	echo HOOK: empty \$empty
+
+	if echo "\$request" | grep -q ".hello"; then
+		printf '{"jsonrpc": "2.0", "result": {"version": "0.1"}, "id": 0}\n\n' >&\$APT_HOOK_SOCKET 2>/dev/null || exit 1
+	fi
+
+
 done
 EOF
 
diff -Nru apt-2.2.3/test/integration/test-apt-https-transient apt-2.2.4/test/integration/test-apt-https-transient
--- apt-2.2.3/test/integration/test-apt-https-transient	1970-01-01 01:00:00.000000000 +0100
+++ apt-2.2.4/test/integration/test-apt-https-transient	2021-05-17 16:20:59.000000000 +0200
@@ -0,0 +1,43 @@
+#!/bin/sh
+set -e
+
+TESTDIR="$(readlink -f "$(dirname "$0")")"
+. "$TESTDIR/framework"
+
+setupenvironment
+configarchitecture "i386"
+
+# Disable sandbox to avoid W: down below
+echo 'APT::Sandbox::User "root";' > rootdir/etc/apt/apt.conf.d/no-acquire-sandbox
+
+echo 'alright' > aptarchive/working
+changetohttpswebserver
+
+msgtest 'download of a file works via' 'http'
+testsuccess --nomsg downloadfile "http://localhost:${APTHTTPPORT}/working"; httpsfile
+testfileequal httpsfile 'alright'
+rm -f httpfile httpsfile
+
+msgtest 'download of a file works via' 'https'
+testsuccess --nomsg downloadfile "https://localhost:${APTHTTPSPORT}/working"; httpfile
+testfileequal httpfile 'alright'
+rm -f httpfile httpsfile
+
+# Speak wrong protocols (https on http port and vice versa). We check that they can be retried.
+
+msgtest 'protocol negotiation error is transient for' 'https'
+testfailureequal "Ign:1 https://localhost:${APTHTTPPORT}/working
+  Could not wait for server fd - select (11: Resource temporarily unavailable)
+Err:1 https://localhost:${APTHTTPPORT}/working
+  Could not wait for server fd - select (11: Resource temporarily unavailable)
+E: Failed to fetch https://localhost:${APTHTTPPORT}/working  Could not wait for server fd - select (11: Resource temporarily unavailable)
+E: Download Failed" apthelper download-file "https://localhost:${APTHTTPPORT}/working"; httpfile -oAcquire::https::Timeout=1 -oAcquire::Retries=1
+
+# Speak wrong protocols (https on http port and vice versa)
+msgtest 'protocol negotiation error is transient for' 'http'
+testfailureequal "Ign:1 http://localhost:${APTHTTPSPORT}/working
+  Connection failed
+Err:1 http://localhost:${APTHTTPSPORT}/working
+  Connection failed
+E: Failed to fetch http://localhost:${APTHTTPSPORT}/working  Connection failed
+E: Download Failed" apthelper download-file "http://localhost:${APTHTTPSPORT}/working"; httpfile -oAcquire::https::Timeout=1 -oAcquire::Retries=1
diff -Nru apt-2.2.3/test/integration/test-phased-updates apt-2.2.4/test/integration/test-phased-updates
--- apt-2.2.3/test/integration/test-phased-updates	2021-04-13 17:53:32.000000000 +0200
+++ apt-2.2.4/test/integration/test-phased-updates	2021-05-17 16:20:59.000000000 +0200
@@ -59,7 +59,7 @@
         500 file:${TMPWORKINGDIRECTORY}/aptarchive unstable/main all Packages
      50 500 (phased 50%)
         500 file:${TMPWORKINGDIRECTORY}/aptarchive unstable/main all Packages
-     10 500 (phased 10%)
+     10 1 (phased 10%)
         500 file:${TMPWORKINGDIRECTORY}/aptarchive unstable/main all Packages" aptcache policy phased1 phased2 phased3
 
 msgmsg "Test for always-include-phased-updates"
@@ -132,9 +132,9 @@
   Version table:
      100 500
         500 file:${TMPWORKINGDIRECTORY}/aptarchive unstable/main all Packages
-     50 500 (phased 50%)
+     50 1 (phased 50%)
         500 file:${TMPWORKINGDIRECTORY}/aptarchive unstable/main all Packages
-     10 500 (phased 10%)
+     10 1 (phased 10%)
         500 file:${TMPWORKINGDIRECTORY}/aptarchive unstable/main all Packages" aptcache policy phased1 phased2 phased3 -o $never=true
 done
 
diff -Nru apt-2.2.3/test/libapt/json_test.cc apt-2.2.4/test/libapt/json_test.cc
--- apt-2.2.3/test/libapt/json_test.cc	1970-01-01 01:00:00.000000000 +0100
+++ apt-2.2.4/test/libapt/json_test.cc	2021-05-17 16:20:59.000000000 +0200
@@ -0,0 +1,69 @@
+#include <config.h>
+#include "../../apt-private/private-cachefile.cc"
+#include "../../apt-private/private-json-hooks.cc"
+#include <gtest/gtest.h>
+#include <string>
+
+TEST(JsonTest, JsonString)
+{
+   std::ostringstream os;
+
+   // Check for escaping backslash and quotation marks, and ensure that we do not change number formatting
+   JsonWriter(os).value("H al\"l\\o").value(17);
+
+   EXPECT_EQ("\"H al\\u0022l\\u005Co\"17", os.str());
+
+   for (int i = 0; i <= 0x1F; i++)
+   {
+      os.str("");
+
+      JsonWriter(os).encodeString(os, std::string("X") + char(i) + "Z");
+
+      std::string exp;
+      strprintf(exp, "\"X\\u%04XZ\"", i);
+
+      EXPECT_EQ(exp, os.str());
+   }
+}
+
+TEST(JsonTest, JsonObject)
+{
+   std::ostringstream os;
+
+   JsonWriter(os).beginObject().name("key").value("value").endObject();
+
+   EXPECT_EQ("{\"key\":\"value\"}", os.str());
+}
+
+TEST(JsonTest, JsonArrayAndValues)
+{
+   std::ostringstream os;
+
+   JsonWriter(os).beginArray().value(0).value("value").value(1).value(true).endArray();
+
+   EXPECT_EQ("[0,\"value\",1,true]", os.str());
+}
+TEST(JsonTest, JsonStackRegression)
+{
+   std::ostringstream os;
+
+   JsonWriter w(os);
+
+   // Nest those things deeply such that we transition states:
+   //    object -> array -> object; -> array -> object
+   // Older versions never popped back and got stuck on array state.
+   w.beginObject();
+   w.name("a").beginArray().beginObject().endObject().endArray();
+   w.name("b").beginArray().beginObject().endObject().endArray();
+   w.endObject();
+
+   EXPECT_EQ("{\"a\":[{}],\"b\":[{}]}", os.str());
+}
+TEST(JsonTest, JsonNull)
+{
+   std::ostringstream os;
+
+   JsonWriter(os).value(nullptr);
+
+   EXPECT_EQ("null", os.str());
+}

--- End Message ---
--- Begin Message --- 
Hi,

On 10-06-2021 12:02, Julian Andres Klode wrote:
>>>> Please unblock package apt

unblocked.

Paul

Attachment: OpenPGP_signature
Description: OpenPGP digital signature


--- End Message ---
Reply to: