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

Bug#474065: Lift the 64000-byte restriction on the size of messages sent to subprocesses.



Package: apt
Version: 0.7.12
Severity: normal
Tags: patch

  This is related to #473874, my patch to lift the 1024-byte limit on
configuration lines.  It turns out that once apt is able to parse those
lines, it's unable to run any download or install commands; it generates
this error message:

E: Method http has died unexpectedly!

  I tracked this down to a problem in communicating with the subprocess.
The subprocess isn't able to parse the configuration block sent by apt,
and so it dies.  The culprit is ReadMessages in contrib/strutl.cc: it
assumes that no single message block is more than 64000 bytes long, and
apparently dumping the apt configuration with some of these translations
included requires more than 64000 bytes.

  A patch to fix this is attached.  I've tried to preserve the existing
behavior, including (as noted in the patch) what looks to me like a bug
in the parsing algorithm.  I have a notion of how to fix it, but that
will take another bus ride's worth of time.

  Daniel
  

-- Package-specific info:

-- (/etc/apt/preferences present, but not submitted) --


-- (/etc/apt/sources.list present, but not submitted) --


-- System Information:
Debian Release: lenny/sid
  APT prefers unstable
  APT policy: (500, 'unstable'), (500, 'stable')
Architecture: i386 (i686)

Kernel: Linux 2.6.24-1-686 (SMP w/1 CPU core)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8) (ignored: LC_ALL set to en_US.UTF-8)
Shell: /bin/sh linked to /bin/bash

Versions of packages apt depends on:
ii  debian-archive-keyring        2007.07.31 GnuPG archive keys of the Debian a
ii  libc6                         2.7-10     GNU C Library: Shared libraries
ii  libgcc1                       1:4.3.0-2  GCC support library
ii  libstdc++6                    4.3.0-2    The GNU Standard C++ Library v3

apt recommends no packages.

-- no debconf information
=== modified file 'apt-pkg/contrib/strutl.cc'
--- apt-pkg/contrib/strutl.cc	2007-12-08 14:15:21 +0000
+++ apt-pkg/contrib/strutl.cc	2008-04-02 16:05:28 +0000
@@ -658,11 +658,24 @@
 // ---------------------------------------------------------------------
 /* This pulls full messages from the input FD into the message buffer. 
    It assumes that messages will not pause during transit so no
-   fancy buffering is used. */
+   fancy buffering is used.
+
+   In particular: this reads blocks from the input until it believes
+   that it's run out of input text.  Each block is terminated by a
+   double newline ('\n' followed by '\n').  As noted below, there is a
+   bug in this code: it assumes that all the blocks have been read if
+   it doesn't see additional text in the buffer after the last one is
+   parsed, which will cause it to lose blocks if the last block
+   coincides with the end of the buffer.
+ */
 bool ReadMessages(int Fd, vector<string> &List)
 {
    char Buffer[64000];
    char *End = Buffer;
+   // Represents any left-over from the previous iteration of the
+   // parse loop.  (i.e., if a message is split across the end
+   // of the buffer, it goes here)
+   string PartialMessage;
    
    while (1)
    {
@@ -690,6 +703,7 @@
 	 
 	 // Pull the message out
 	 string Message(Buffer,I-Buffer);
+	 PartialMessage += Message;
 
 	 // Fix up the buffer
 	 for (; I < End && *I == '\n'; I++);
@@ -697,10 +711,32 @@
 	 memmove(Buffer,I,End-Buffer);
 	 I = Buffer;
 	 
-	 List.push_back(Message);
+	 List.push_back(PartialMessage);
+	 PartialMessage.clear();
       }
-      if (End == Buffer)
-	 return true;
+      if (End != Buffer)
+	{
+	  // If there's text left in the buffer, store it
+	  // in PartialMessage and throw the rest of the buffer
+	  // away.  This allows us to handle messages that
+	  // are longer than the static buffer size.
+	  PartialMessage += string(Buffer, End);
+	  End = Buffer;
+	}
+      else
+	{
+	  // BUG ALERT: if a message block happens to end at a
+	  // multiple of 64000 characters, this will cause it to
+	  // terminate early, leading to a badly formed block and
+	  // probably crashing the method.  However, this is the only
+	  // way we have to find the end of the message block.  I have
+	  // an idea of how to fix this, but it will require changes
+	  // to the protocol (essentially to mark the beginning and
+	  // end of the block).
+	  //
+	  //  -- dburrows 2008-04-02
+	  return true;
+	}
 
       if (WaitFd(Fd) == false)
 	 return false;


Reply to: