git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCHv2 04/10] pkt-line: change error message for oversized packet

From
Jeff King <peff@peff.net>
Date
Feb 18, 2013, 09:22 UTC
Message-ID
<20130218092221.GD5096@sigill.intra.peff.net>
In-Reply-To
<20130218091203.GB17003@sigill.intra.peff.net>

If we get a packet from the remote side that is too large to fit in our buffer, we currently complain "protocol error: bad line length". This is a bit vague. The line length the other side sent is not "bad" per se; the actual problem is that it exceeded our expectation for buffer length.

This will generally not happen between two git-core implementations, because the sender limits themselves to either 1000, or to LARGE_PACKET_MAX (depending on what is being sent, sideband-64k negotiation, etc), and the receiver uses a buffer of the appropriate size.

The protocol document indicates the LARGE_PACKET_MAX limit (of 65520), but does not actually specify the 1000-byte limit for ref lines. It is likely that other implementations just create a packet as large as they need, and this doesn't come up often because nobody has 1000-character ref names (or close to it, counting sha1 and other boilerplate).

We may want to increase the size of our receive buffers for ref lines to prepare for such writers (whether they are other implementations, or if we eventually want to bump the write size in git-core). In the meantime, this patch tries to give a more clear message in case it does come up.

Signed-off-by: Jeff King <peff@peff.net>
---
I'm really tempted to bump all of our 1000-byte buffers to just use
LARGE_PACKET_MAX. If we provided a packet_read variant that used a
static buffer (which is fine for all but one or two callers), then it
would not take much memory (right now we stick some LARGE_PACKET_MAX
buffers on the stack, which is slightly questionable for
stack-restricted systems). But I left that for a different topic (and
even if we do, we would still want this message to catch anything over
the bizarre 65520 limit).

Out of curiosity, I grepped the list archives, and found only one instance of this message. And it was somebody whose data stream was tainted with random crud that happened to be numbers (much more common is "bad line length character", when the crud does not look like a packet length).

 pkt-line.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/pkt-line.c b/pkt-line.c
index 62479d3..f2a7575 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -160,7 +160,8 @@ static int packet_read_internal(int fd, char *buffer, unsigned size, int gently)
 	}
 	len -= 4;
 	if (len >= size)
-		die("protocol error: bad line length %d", len);
+		die("protocol error: line too large: (expected %u, got %d)",
+		    size, len);
 	ret = safe_read(fd, buffer, len, gently);
 	if (ret < 0)
 		return ret;
-- 
1.8.1.20.g7078b03
Previous: Jeff KingNext: Junio C Hamano
Message 21 of 50 in “make smart-http more robust against bogus server input”
  1. 0/3 make smart-http more robust against bogus server inputJeff King, Feb 16, 2013
  2. 1/3 pkt-line: teach packet_get_line a no-op modeJeff King, Feb 16, 2013
  3. Jonathan NiederFeb 17, 2013
  4. 2/3 remote-curl: verify smart-http metadata linesJeff King, Feb 16, 2013
  5. Jonathan NiederFeb 17, 2013
  6. Jeff KingFeb 17, 2013
  7. Jonathan NiederFeb 18, 2013
  8. Jeff KingFeb 18, 2013
  9. 3/3 remote-curl: sanity check ref advertisement from serverJeff King, Feb 16, 2013
  10. Jonathan NiederFeb 17, 2013
  11. Jeff KingFeb 17, 2013
  12. Jonathan NiederFeb 18, 2013
  13. 0/10 pkt-line and remote-curl cleanups serverJeff King, Feb 18, 2013
  14. 01/10 pkt-line: move a misplaced commentJeff King, Feb 18, 2013
  15. 02/10 pkt-line: drop safe_write functionJeff King, Feb 18, 2013
  16. Jonathan NiederFeb 18, 2013
  17. Jeff KingFeb 18, 2013
  18. 03/10 pkt-line: clean up "gentle" reading functionJeff King, Feb 18, 2013
  19. Jonathan NiederFeb 18, 2013
  20. Jeff KingFeb 18, 2013
  21. 04/10 pkt-line: change error message for oversized packetJeff King, Feb 18, 2013
  22. Junio C HamanoFeb 18, 2013
  23. Jeff KingFeb 18, 2013
  24. Junio C HamanoFeb 18, 2013
  25. Jeff KingFeb 18, 2013
  26. Jeff KingFeb 20, 2013
  27. Junio C HamanoFeb 20, 2013
  28. Jonathan NiederFeb 18, 2013
  29. Jeff KingFeb 18, 2013
  30. 05/10 pkt-line: rename s/packet_read_line/packet_read/Jeff King, Feb 18, 2013
  31. Jonathan NiederFeb 18, 2013
  32. Jeff KingFeb 18, 2013
  33. Jonathan NiederFeb 18, 2013
  34. 06/10 pkt-line: share buffer/descriptor reading implementationJeff King, Feb 18, 2013
  35. Jonathan NiederFeb 18, 2013
  36. Jeff KingFeb 18, 2013
  37. Jonathan NiederFeb 18, 2013
  38. 07/10 teach get_remote_heads to read from a memory bufferJeff King, Feb 18, 2013
  39. 08/10 remote-curl: pass buffer straight to get_remote_headsJeff King, Feb 18, 2013
  40. Jonathan NiederFeb 18, 2013
  41. Junio C HamanoFeb 18, 2013
  42. Jeff KingFeb 18, 2013
  43. Junio C HamanoFeb 18, 2013
  44. Jeff KingFeb 18, 2013
  45. Shawn PearceFeb 20, 2013
  46. 09/10 remote-curl: move ref-parsing code up in fileJeff King, Feb 18, 2013
  47. 10/10 remote-curl: always parse incoming refsJeff King, Feb 18, 2013
  48. Jonathan NiederFeb 18, 2013
  49. Shawn PearceFeb 20, 2013
  50. Shawn PearceFeb 20, 2013

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.