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

Re: [PATCH v3 1/4] pack-protocol.txt: accept error packets in any context

From
Josh Steadmon <steadmon@google.com>
Date
Dec 13, 2018, 22:18 UTC
Message-ID
<20181213221826.GE37614@google.com>
In-Reply-To
<CAJB1erXRqQW0yQyZutJAJKC7WbdVhBAYUMWM+8ZutxA-W-7S8w@mail.gmail.com>
On 2018.12.12 17:17, Masaya Suzuki wrote:
Show 26 quoted lines
> On Wed, Dec 12, 2018 at 3:02 AM Jeff King <peff@peff.net> wrote:
> > This ERR handling has been moved to a very low level. What happens if
> > we're passing arbitrary data via the packet_read() code? Could we
> > erroneously trigger an error if a packfile happens to have the bytes
> > "ERR " at a packet boundary?
> >
> > For packfiles via upload-pack, I _think_ we're OK, because we only
> > packetize it when a sideband is in use. In which case this would never
> > match, because we'd have "\1" in the first byte slot.
> >
> > But are there are other cases we need to worry about? Just
> > brainstorming, I can think of:
> >
> >   1. We also pass packetized packfiles between git-remote-https and
> >      the stateless-rpc mode of fetch-pack/send-pack. And I don't think
> >      we use sidebands there.
> >
> >   2. The packet code is used for long-lived clean/smudge filters these
> >      days, which also pass arbitrary data.
> >
> > So I think it's probably not a good idea to unconditionally have callers
> > of packet_read_with_status() handle this. We'd need a flag like
> > PACKET_READ_RESPECT_ERR, and to trigger it from the appropriate callers.
> 
> This is outside of the Git pack protocol so having a separate parsing
> mode makes sense to me.

This sounds like it could be a significant refactoring. Should we go back to V2 of this series, and then work on the new parsing mode separately?

Previous: Jeff KingNext: Jeff King
Message 32 of 41 in “remote-curl: die on server-side errors”
  1. remote-curl: die on server-side errorssteadmon@google.com, Nov 12, 2018
  2. Stefan BellerNov 12, 2018
  3. Junio C HamanoNov 13, 2018
  4. Junio C HamanoNov 13, 2018
  5. Josh SteadmonNov 13, 2018
  6. Jeff KingNov 13, 2018
  7. Josh SteadmonNov 13, 2018
  8. Jeff KingNov 14, 2018
  9. Jeff KingNov 14, 2018
  10. Josh SteadmonNov 15, 2018
  11. 0/3 remote-curl smart-http discovery cleanupJeff King, Nov 16, 2018
  12. 1/3 remote-curl: refactor smart-http discoveryJeff King, Nov 16, 2018
  13. Josh SteadmonNov 16, 2018
  14. Junio C HamanoFeb 5, 2019
  15. Jeff KingFeb 6, 2019
  16. 1/3 remote-curl: refactor smart-http discoveryJeff King, Feb 6, 2019
  17. Josh SteadmonFeb 6, 2019
  18. Junio C HamanoFeb 6, 2019
  19. Jeff KingFeb 6, 2019
  20. 2/3 remote-curl: tighten "version 2" check for smart-httpJeff King, Feb 6, 2019
  21. 3/3 t5551: test server-side ERR packetJeff King, Feb 6, 2019
  22. Junio C HamanoFeb 6, 2019
  23. 2/3 remote-curl: tighten "version 2" check for smart-httpJeff King, Nov 16, 2018
  24. Josh SteadmonNov 16, 2018
  25. 3/3 remote-curl: die on server-side errorsJeff King, Nov 16, 2018
  26. Josh SteadmonNov 16, 2018
  27. 0/4 Unify pkt-line error handling and refactor smart-httpJosh Steadmon, Dec 12, 2018
  28. 1/4 pack-protocol.txt: accept error packets in any contextJosh Steadmon, Dec 12, 2018
  29. Jeff KingDec 12, 2018
  30. Masaya SuzukiDec 13, 2018
  31. Jeff KingDec 13, 2018
  32. Josh SteadmonDec 13, 2018
  33. Jeff KingDec 17, 2018
  34. Josh SteadmonDec 19, 2018
  35. Jeff KingDec 20, 2018
  36. 2/4 remote-curl: refactor smart-http discoveryJosh Steadmon, Dec 12, 2018
  37. 3/4 remote-curl: tighten "version 2" check for smart-httpJosh Steadmon, Dec 12, 2018
  38. 4/4 lib-httpd, t5551: check server-side HTTP errorsJosh Steadmon, Dec 12, 2018
  39. Junio C HamanoDec 12, 2018
  40. Junio C HamanoNov 13, 2018
  41. Josh SteadmonNov 13, 2018

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.