{"thread":{"id":"49925","subject":"Parsing a git HTTP protocol response","startedAt":"2018-12-01T01:05:48Z","lastAt":"2018-12-01T03:00:03Z","messageCount":3,"participants":["Farhan Khan","Bryan Turner"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"364407","messageId":"CAFd4kYAzOd8qwEDgVf-W+1S3NoZFUa_vZsUfuNNjL_XsuesO5w@mail.gmail.com","threadId":"49925","inReplyTo":null,"subject":"Parsing a git HTTP protocol response","fromName":"Farhan Khan","fromEmail":"khanzf@gmail.com","sentAt":"2018-12-01T01:05:34Z","receivedAt":"2018-12-01T01:05:48Z","isPatch":false,"sender":{"key":"khanzf@gmail.com","avatar":null},"body":"Hi all,\n\nI am writing an implementation of the git HTTP pack protocol in C. It\njust does a request to clone a repository. It works pretty well for\nsmall repositories, but seems to fail on larger repositories and I do\nnot understand why.\n\nAll that my code does is send a hard-coded \"want\" request. As I\nunderstand it, responses come with a four-byte size prefix, then the\ndata of the size - 4. The first four bytes are the size of the object\nin ASCII and after that size, a new object should begin by specifying\nits size. The final \"0000\" string should signify the end.\n\nOn small repositories my code works fine. But when use a large git\nrepository, I hit an object size that cannot be interpreted by ASCII\ninto a number. In particular, right before the crash I am pretty\nconsistently getting a size starting with 0x32,0x00 or 0x32,0x30,0x00\n(note, it starts with 0x32 and has 0x00 in there). This is not a\nreadable four byte ascii value, likely an erroneous size value, which\ncauses the next object size calculation to land incorrectly and\nsubsequently the program to malfunction.\n\nMy questions:\n1. Am I misunderstanding the protocol?\n2. Does 0x32 signify something?\n3. Also, where can I see in the source code git parses the data it\nreceives from a \"want\" request?\n\nIf you would like to see my code, it is located here:\nhttp://git.farhan.codes/farhan/post\nTo compile to Linux run: gcc post.c main.c -lcurl -o post\nTo compile on FreeBSD you can use the Makefile or: cc -L\n/usr/local/lib -I /usr/local/include -lcurl main.c post.c -o post\nIn both cases you need to have libcurl installed.\n\nTo run do: ./post [a git http url] [a commit value]\nie: ./post http://git.farhan.codes/farhan/openbsd\n373dd5ba116d42cddf1fdcb58b578a4274f6d589\n\nI have a lot of debugging printf() messages, but it eventually hits a\npoint where you see this:\n\n========Start=========\nCurrent stats: Current state: 999 Received: 1448\nWe have enough bytes, moving forward\n== New Object\nNo Error, object is 32 bytes\nSize bytes: 32 30 00 00\n\nThe program interprets the size of {0x32,0x30,0x00,0x00} to be \"20\"\nwhich in decimal is 32, causing the next read to fail.\nThis problem repeats on a few different repositories.\n\nAny assistance is welcome, I am very stuck on how the HTTP git protocol works.\nThanks,\n--\nFarhan Khan\nPGP Fingerprint: B28D 2726 E2BC A97E 3854 5ABE 9A9F 00BC D525 16EE\n"},{"id":"364410","messageId":"CAGyf7-Eq6tQQocjXFzngX=vo3FWba=tvmAqGybcLxLXn6ZZkpA@mail.gmail.com","threadId":"49925","inReplyTo":"CAFd4kYAzOd8qwEDgVf-W+1S3NoZFUa_vZsUfuNNjL_XsuesO5w@mail.gmail.com","subject":"Re: Parsing a git HTTP protocol response","fromName":"Bryan Turner","fromEmail":"bturner@atlassian.com","sentAt":"2018-12-01T02:58:09Z","receivedAt":"2018-12-01T02:58:23Z","isPatch":false,"sender":{"key":"bturner@atlassian.com","avatar":"https://gravatar.com/avatar/16bcf3167981c1ef7c804e502642366d888a35b0d0b0a4ca01fdc442aa1acb1e?d=mp&s=160"},"body":"On Fri, Nov 30, 2018 at 5:05 PM Farhan Khan <khanzf@gmail.com> wrote:\n>\n> Hi all,\n>\n> I am writing an implementation of the git HTTP pack protocol in C. It\n> just does a request to clone a repository. It works pretty well for\n> small repositories, but seems to fail on larger repositories and I do\n> not understand why.\n>\n> All that my code does is send a hard-coded \"want\" request. As I\n> understand it, responses come with a four-byte size prefix, then the\n> data of the size - 4. The first four bytes are the size of the object\n> in ASCII and after that size, a new object should begin by specifying\n> its size. The final \"0000\" string should signify the end.\n>\n> On small repositories my code works fine. But when use a large git\n> repository, I hit an object size that cannot be interpreted by ASCII\n> into a number. In particular, right before the crash I am pretty\n> consistently getting a size starting with 0x32,0x00 or 0x32,0x30,0x00\n> (note, it starts with 0x32 and has 0x00 in there). This is not a\n> readable four byte ascii value, likely an erroneous size value, which\n> causes the next object size calculation to land incorrectly and\n> subsequently the program to malfunction.\n>\n> My questions:\n> 1. Am I misunderstanding the protocol?\n> 2. Does 0x32 signify something?\n> 3. Also, where can I see in the source code git parses the data it\n> receives from a \"want\" request?\n>\n> If you would like to see my code, it is located here:\n> http://git.farhan.codes/farhan/post\n> To compile to Linux run: gcc post.c main.c -lcurl -o post\n> To compile on FreeBSD you can use the Makefile or: cc -L\n> /usr/local/lib -I /usr/local/include -lcurl main.c post.c -o post\n> In both cases you need to have libcurl installed.\n>\n> To run do: ./post [a git http url] [a commit value]\n> ie: ./post http://git.farhan.codes/farhan/openbsd\n> 373dd5ba116d42cddf1fdcb58b578a4274f6d589\n>\n> I have a lot of debugging printf() messages, but it eventually hits a\n> point where you see this:\n>\n> ========Start=========\n> Current stats: Current state: 999 Received: 1448\n> We have enough bytes, moving forward\n> == New Object\n> No Error, object is 32 bytes\n> Size bytes: 32 30 00 00\n\nI modified your debugging output a little bit, and right before the\nfailure happens I see this in my output:\n\n========Start=========\nCurrent stats: Current state: 999 Received: 1298\nRead complete; still missing 1296 bytes (6901 read of 8197)\nOffset: 1298\n\n========Start=========\nCurrent stats: Current state: 999 Received: 1298\nRead complete; 2 more bytes in buffer\n== New Object\nSize bytes: 32 30 00 2b\n\nBased on that, it appears what's happening is you're not handling the\ncase where you end up with fewer than 4 bytes in the buffer. As a\nresult, memcpy reads past the end of the response buffer and gets\nwhatever it gets, resulting in an incorrect parsed size.\n\nThe while loop in pack_protocol_line is checking +1, but I think it\nshould be checking +3 to ensure there's at least 4 bytes so it can get\na complete size. The \"parseread\" struct will need to grow a couple\nmore fields to allow buffering some additional bytes between\npack_protocol_line calls (because curl requires the write function to\nalways consume the full buffer) and, at the start of the loop, when\nreading/parsing a size, the code will need to consider any buffered\nbytes from the previous function call. That requires some knock-on\nchanges to how the offset is handled, as well as the the initial\n\"psize\" set when starting a new packet.\n\nOnce you start accounting for reads that cut off in 1, 2 or 3 bytes\ninto the 4 required to parse a size, your program should be able to\nrun to completion.\n\nHere's a (very ugly) patch I threw together on top of your code:\n\ndiff --git a/main.c b/main.c\nindex 956362b..8873fd5 100644\n--- a/main.c\n+++ b/main.c\n@@ -71,7 +71,7 @@ int main(int argc, char *argv[]) {\n  curl_easy_setopt(curl, CURLOPT_HTTPHEADER, list);\n  curl_easy_setopt(curl, CURLOPT_POSTFIELDSIZE, (long)content_length);\n  curl_easy_setopt(curl, CURLOPT_WRITEDATA, &parseread);\n- curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, pack_protocol_line);\n+ curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, &pack_protocol_line);\n\n  res = curl_easy_perform(curl);\n  if (curl != CURLE_OK)\ndiff --git a/post.c b/post.c\nindex cfffd5c..4f8512c 100644\n--- a/post.c\n+++ b/post.c\n@@ -36,89 +36,83 @@ size_t pack_protocol_line(void *buffer, size_t\nsize, size_t nmemb, void *userp)\n {\n  unsigned char *reply = buffer;\n  struct parseread *parseread = userp;\n- int offset = 0;\n+ int offset = 0, pending = 0, remaining = 0;\n  char tmp[5];\n- int check;\n-\n- int pool = size * nmemb;\n\n  printf(\"\\n\\n========Start=========\\n\");\n- printf(\"Current stats: Current state: %d Received: %ld\\n\",\nparseread->state, size*nmemb);\n+ printf(\"Current stats: Current state: %d Received: %ld\\n\",\nparseread->state, nmemb);\n\n- while(offset + 1 < size + nmemb) {\n+ while (offset + 3 < nmemb) {\n  if (parseread->state == STATE_NEWLINE) {\n  printf(\"== New Object\\n\");\n  bzero(tmp, 5);\n- memcpy(tmp, buffer+offset, 4); tmp[4] = '\\0';\n- check = sscanf(tmp, \"%04x\", &parseread->osize);\n\n- if (parseread->osize == 0) {\n- printf(\"Only happens if reached terminating 0000\\n\");\n+ if (parseread->bsize > 0) {\n+ // If there are buffered size bytes, consume them\n+ memcpy(tmp, parseread->buffer, parseread->bsize);\n+ memcpy(tmp + parseread->bsize, buffer + offset, 4 - parseread->bsize);\n+ offset += 4 - parseread->bsize;\n+ parseread->bsize = 0;\n+ } else {\n+ memcpy(tmp, buffer+offset, 4);\n  offset += 4;\n- break;\n  }\n+ tmp[4] = '\\0';\n\n- if (check == 0) {\n- printf(\"Error, unable to parse object size\\n\");\n- printf(\"Size bytes: %02x %02x %02x %02x\\n\",\n- reply[offset+0],\n- reply[offset+1],\n- reply[offset+2],\n- reply[offset+3]);\n- printf(\"Data: %02x %02x %02x %02x\\n\",\n- reply[offset+4],\n- reply[offset+5],\n- reply[offset+6],\n- reply[offset+7]);\n- exit(-1);\n- }\n- else {\n- printf(\"No Error, object is %d bytes\\n\", parseread->osize);\n- printf(\"Size bytes: %02x %02x %02x %02x\\n\",\n- reply[offset+0],\n- reply[offset+1],\n- reply[offset+2],\n- reply[offset+3]);\n- printf(\"%02x %02x %02x %02x\\n\",\n- reply[offset+4],\n- reply[offset+5],\n- reply[offset+6],\n- reply[offset+7]);\n+ printf(\"Size bytes: %02x %02x %02x %02x\\n\", tmp[0], tmp[1], tmp[2], tmp[3]);\n+ parseread->osize = (int) strtol(tmp, NULL, 16);\n+ printf(\"Parsed size: %d\\n\", parseread->osize);\n+\n+ if (parseread->osize == 0) {\n+ printf(\"Only happens if reached terminating 0000\\n\");\n+ break;\n  }\n\n- printf(\"Size: %d\\n\", parseread->osize);\n- parseread->psize = 0;\n+ parseread->psize = 4;\n\n  // This classifies the object\n- if ( strncmp(reply+offset+4, \"NAK\\n\", 4)==0)\n+ if ( strncmp(reply+offset, \"NAK\\n\", 4)==0)\n  parseread->state = STATE_NAK;\n- else if (reply[offset+4] == 0x02)\n+ else if (reply[offset] == 0x02)\n  parseread->state = STATE_REMOTE;\n  else\n  parseread->state = STATE_UNKNOWN;\n  }\n\n- if ( (pool - offset) > (parseread->osize - parseread->psize) ) {\n- printf(\"We have enough bytes, moving forward\\n\");\n+ pending = parseread->osize - parseread->psize;\n+ remaining = nmemb - offset;\n+\n+ if ( remaining > pending ) {\n+ printf(\"Read complete; %d more bytes in buffer\\n\", remaining - pending);\n  process_objects(reply+offset, parseread);\n  offset += (parseread->osize - parseread->psize);\n  parseread->state = STATE_NEWLINE;\n  }\n- else if ( (pool - offset) ==(parseread->osize - parseread->psize) ) {\n- printf(\"They are the same length\\n\");\n+ else if ( remaining == pending ) {\n+ printf(\"Read complete; buffer exhausted\\n\");\n  process_objects(reply+offset, parseread);\n  offset += (parseread->osize - parseread->psize);\n  parseread->state = STATE_NEWLINE;\n  }\n- else if ( (pool - offset) < (parseread->osize - parseread->psize)) {\n- printf(\"We do not have enough bytes\\n\");\n+ else if ( remaining < pending ) {\n+ printf(\"Read complete; still missing %d bytes (%d read of %d)\\n\",\n+ pending - remaining, parseread->psize + remaining, parseread->osize);\n  process_objects(reply+offset, parseread);\n- parseread->psize += (pool - offset);\n- offset = pool;\n+ parseread->psize += remaining;\n+ offset = nmemb;\n  }\n  }\n\n  printf(\"Offset: %d\\n\", offset);\n+ if (offset < nmemb) {\n+ // We don't have enough bytes to read the next size, so buffer it\n+ printf(\"Short read; %d bytes left\", nmemb - offset);\n+ bzero(parseread->buffer, 4);\n+ parseread->bsize = nmemb - offset;\n+ memcpy(parseread->buffer, buffer+offset, parseread->bsize);\n+\n+ offset = nmemb;\n+ }\n\n  return offset;\n }\ndiff --git a/post.h b/post.h\nindex a229342..01a2669 100644\n--- a/post.h\n+++ b/post.h\n@@ -5,6 +5,8 @@ struct parseread {\n  int state; // current state\n  int osize; // object size\n  int psize; // processed size\n+ char buffer[4];\n+ int bsize;\n };\n\n #define STATE_NEWLINE 0\n\nAgain, it's not pretty, but with those changes a `./post` on your\nopenbsd URL succeeds.\n\n========Start=========\nCurrent stats: Current state: 999 Received: 503\nRead complete; 77 more bytes in buffer\n== New Object\nSize bytes: 30 30 30 36\nParsed size: 6\nRead complete; 71 more bytes in buffer\n== New Object\nSize bytes: 30 30 34 33\nParsed size: 67\nRead complete; 4 more bytes in buffer\nRemote: l 1798261 (delta 1438982), reused 1795494 (delta 1436215)\n0000\n== New Object\nSize bytes: 30 30 30 30\nParsed size: 0\nOnly happens if reached terminating 0000\nOffset: 503\ncurl_easy_perform() failed: No error\n\nBryan\n"},{"id":"364411","messageId":"CAGyf7-HvPTPS-N8vtEngkhw-J+Egfq=MzxL8swhD2h+ck87V6A@mail.gmail.com","threadId":"49925","inReplyTo":"CAGyf7-Eq6tQQocjXFzngX=vo3FWba=tvmAqGybcLxLXn6ZZkpA@mail.gmail.com","subject":"Re: Parsing a git HTTP protocol response","fromName":"Bryan Turner","fromEmail":"bturner@atlassian.com","sentAt":"2018-12-01T02:59:49Z","receivedAt":"2018-12-01T03:00:03Z","isPatch":false,"sender":{"key":"bturner@atlassian.com","avatar":"https://gravatar.com/avatar/16bcf3167981c1ef7c804e502642366d888a35b0d0b0a4ca01fdc442aa1acb1e?d=mp&s=160"},"body":"On Fri, Nov 30, 2018 at 6:58 PM Bryan Turner <bturner@atlassian.com> wrote:\n>\n> Here's a (very ugly) patch I threw together on top of your code:\n...snip\n\nGmail butchered my patch, so here it is as an attachment.\n\nBryan\n\n\ndiff --git a/main.c b/main.c\nindex 956362b..8873fd5 100644\n--- a/main.c\n+++ b/main.c\n@@ -71,7 +71,7 @@ int main(int argc, char *argv[]) {\n \t\tcurl_easy_setopt(curl, CURLOPT_HTTPHEADER, list);\n \t\tcurl_easy_setopt(curl, CURLOPT_POSTFIELDSIZE, (long)content_length);\n \t\tcurl_easy_setopt(curl, CURLOPT_WRITEDATA, &parseread);\n-\t\tcurl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, pack_protocol_line);\n+\t\tcurl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, &pack_protocol_line);\n \n \t\tres = curl_easy_perform(curl);\n \t\tif (curl != CURLE_OK)\ndiff --git a/post.c b/post.c\nindex cfffd5c..4f8512c 100644\n--- a/post.c\n+++ b/post.c\n@@ -36,89 +36,83 @@ size_t pack_protocol_line(void *buffer, size_t size, size_t nmemb, void *userp)\n {\n \tunsigned char *reply = buffer;\n \tstruct parseread *parseread = userp;\n-\tint offset = 0;\n+\tint offset = 0, pending = 0, remaining = 0;\n \tchar tmp[5];\n-\tint check;\n-\n-\tint pool = size * nmemb;\n \n \tprintf(\"\\n\\n========Start=========\\n\");\n-\tprintf(\"Current stats: Current state: %d Received: %ld\\n\", parseread->state, size*nmemb);\n+\tprintf(\"Current stats: Current state: %d Received: %ld\\n\", parseread->state, nmemb);\n \n-\twhile(offset + 1 < size + nmemb) {\n+\twhile (offset + 3 < nmemb) {\n \t\tif (parseread->state == STATE_NEWLINE) { \n \t\t\tprintf(\"== New Object\\n\");\n \t\t\tbzero(tmp, 5);\n-\t\t\tmemcpy(tmp, buffer+offset, 4); tmp[4] = '\\0';\n-\t\t\tcheck = sscanf(tmp, \"%04x\", &parseread->osize);\n \n-\t\t\tif (parseread->osize == 0) {\n-\t\t\t\tprintf(\"Only happens if reached terminating 0000\\n\");\n+\t\t\tif (parseread->bsize > 0) {\n+\t\t\t\t// If there are buffered size bytes, consume them\n+\t\t\t\tmemcpy(tmp, parseread->buffer, parseread->bsize);\n+\t\t\t\tmemcpy(tmp + parseread->bsize, buffer + offset, 4 - parseread->bsize);\n+\t\t\t\toffset += 4 - parseread->bsize;\n+\t\t\t\tparseread->bsize = 0;\n+\t\t\t} else {\n+\t\t\t\tmemcpy(tmp, buffer+offset, 4);\n \t\t\t\toffset += 4;\n-\t\t\t\tbreak;\n \t\t\t}\n+\t\t\ttmp[4] = '\\0';\n \n-\t\t\tif (check == 0) {\n-\t\t\t\tprintf(\"Error, unable to parse object size\\n\");\n-\t\t\t\tprintf(\"Size bytes: %02x %02x %02x %02x\\n\",\n-\t\t\t\t\treply[offset+0],\n-\t\t\t\t\treply[offset+1],\n-\t\t\t\t\treply[offset+2],\n-\t\t\t\t\treply[offset+3]);\n-\t\t\t\tprintf(\"Data: %02x %02x %02x %02x\\n\",\n-\t\t\t\t\treply[offset+4],\n-\t\t\t\t\treply[offset+5],\n-\t\t\t\t\treply[offset+6],\n-\t\t\t\t\treply[offset+7]);\n-\t\t\t\texit(-1);\n-\t\t\t}\n-\t\t\telse {\n-\t\t\t\tprintf(\"No Error, object is %d bytes\\n\", parseread->osize);\n-\t\t\t\tprintf(\"Size bytes: %02x %02x %02x %02x\\n\",\n-\t\t\t\t\treply[offset+0],\n-\t\t\t\t\treply[offset+1],\n-\t\t\t\t\treply[offset+2],\n-\t\t\t\t\treply[offset+3]);\n-\t\t\t\tprintf(\"%02x %02x %02x %02x\\n\",\n-\t\t\t\t\treply[offset+4],\n-\t\t\t\t\treply[offset+5],\n-\t\t\t\t\treply[offset+6],\n-\t\t\t\t\treply[offset+7]);\n+\t\t\tprintf(\"Size bytes: %02x %02x %02x %02x\\n\", tmp[0], tmp[1], tmp[2], tmp[3]);\n+\t\t\tparseread->osize = (int) strtol(tmp, NULL, 16);\n+\t\t\tprintf(\"Parsed size: %d\\n\", parseread->osize);\n+\n+\t\t\tif (parseread->osize == 0) {\n+\t\t\t\tprintf(\"Only happens if reached terminating 0000\\n\");\n+\t\t\t\tbreak;\n \t\t\t}\n \n-\t\t\tprintf(\"Size: %d\\n\", parseread->osize);\n-\t\t\tparseread->psize = 0;\n+\t\t\tparseread->psize = 4;\n \n \t\t\t// This classifies the object\n-\t\t\tif ( strncmp(reply+offset+4, \"NAK\\n\", 4)==0)\n+\t\t\tif ( strncmp(reply+offset, \"NAK\\n\", 4)==0)\n \t\t\t\tparseread->state = STATE_NAK;\n-\t\t\telse if (reply[offset+4] == 0x02)\n+\t\t\telse if (reply[offset] == 0x02)\n \t\t\t\tparseread->state = STATE_REMOTE;\n \t\t\telse\n \t\t\t\tparseread->state = STATE_UNKNOWN;\n \t\t}\n \n-\t\tif ( (pool - offset) > (parseread->osize - parseread->psize) ) {\n-\t\t\tprintf(\"We have enough bytes, moving forward\\n\");\n+\t\tpending = parseread->osize - parseread->psize;\n+\t\tremaining = nmemb - offset;\n+\n+\t\tif ( remaining > pending ) {\n+\t\t\tprintf(\"Read complete; %d more bytes in buffer\\n\", remaining - pending);\n \t\t\tprocess_objects(reply+offset, parseread);\n \t\t\toffset += (parseread->osize - parseread->psize); \n \t\t\tparseread->state = STATE_NEWLINE;\n \t\t}\n-\t\telse if ( (pool - offset) ==(parseread->osize - parseread->psize) ) {\n-\t\t\tprintf(\"They are the same length\\n\");\n+\t\telse if ( remaining == pending ) {\n+\t\t\tprintf(\"Read complete; buffer exhausted\\n\");\n \t\t\tprocess_objects(reply+offset, parseread);\n \t\t\toffset += (parseread->osize - parseread->psize);\n \t\t\tparseread->state = STATE_NEWLINE;\n \t\t}\n-\t\telse if ( (pool - offset) < (parseread->osize - parseread->psize)) {\n-\t\t\tprintf(\"We do not have enough bytes\\n\");\n+\t\telse if ( remaining < pending ) {\n+\t\t\tprintf(\"Read complete; still missing %d bytes (%d read of %d)\\n\",\n+\t\t\t\t\tpending - remaining, parseread->psize + remaining, parseread->osize);\n \t\t\tprocess_objects(reply+offset, parseread);\n-\t\t\tparseread->psize += (pool - offset);\n-\t\t\toffset = pool;\n+\t\t\tparseread->psize += remaining;\n+\t\t\toffset = nmemb;\n \t\t}\n \t}\n \n \tprintf(\"Offset: %d\\n\", offset);\n+\tif (offset < nmemb) {\n+\t\t// We don't have enough bytes to read the next size, so buffer it\n+\t\tprintf(\"Short read; %d bytes left\", nmemb - offset);\n+\t\tbzero(parseread->buffer, 4);\n+\t\tparseread->bsize = nmemb - offset;\n+\t\tmemcpy(parseread->buffer, buffer+offset, parseread->bsize);\n+\n+\t\toffset = nmemb;\n+\t}\n \n \treturn offset;\n }\ndiff --git a/post.h b/post.h\nindex a229342..01a2669 100644\n--- a/post.h\n+++ b/post.h\n@@ -5,6 +5,8 @@ struct parseread {\n \tint state; // current state\n \tint osize; // object size\n \tint psize; // processed size\n+\tchar buffer[4];\n+\tint bsize;\n };\n \n #define STATE_NEWLINE\t\t0\n"}]}