{"thread":{"id":"5487","subject":"remote_get_heads: reference length limit","startedAt":"2006-09-05T12:50:37Z","lastAt":"2006-09-05T19:00:17Z","messageCount":3,"participants":["Andy Whitcroft","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"26360","messageId":"44FD729D.4020208@shadowen.org","threadId":"5487","inReplyTo":null,"subject":"remote_get_heads: reference length limit","fromName":"Andy Whitcroft","fromEmail":"apw@shadowen.org","sentAt":"2006-09-05T12:50:37Z","receivedAt":"2006-09-05T12:50:37Z","isPatch":false,"sender":{"key":"apw@shadowen.org","avatar":"https://gravatar.com/avatar/d3088262854661a913ef35cc40fedcc270142d4461791142bc1ea0b2a4e2e147?d=mp&s=160"},"body":"I've been having trouble with git push apparently resending the entire\ncommit trace for the branch each and every time I push.  Poking at the\nsource it seems this is due to a length limit on reference names as\npulled from the remote repository.\n\nWhen we are building the pack to send we are sent a list of remote\nheads.  get_remote_heads() reads these in, validates them and finally\nadds them to the remote_refs list.  Part of the validation is a simple\ncheck for size and form; check_ref().\n\nstatic int check_ref(const char *name, int len, unsigned int flags)\n{\n        if (!flags)\n                return 1;\n\n        if (len > 45 || memcmp(name, \"refs/\", 5))\n                return 0;\n[...]\n}\n\nWith the refs/heads/ prefix included this limits the head names to 34\ncharacters.  From what I can see there is no good reason for this limit\nto be so low.  I can see we don't want the remote end bloating us out of\ncontrol, but we are already limiting the lines which contain these\nreferences to 1000 bytes and making no attempt to limit the number of\nthem the remote server can send us.  There seems to be no limits imposed\non the name length other than MAX_PATHLEN.\n\nCan anyone see a reason to keep this (len > 45) check?\n\n-apw\n"},{"id":"26361","messageId":"7v64g2i9p8.fsf@assigned-by-dhcp.cox.net","threadId":"5487","inReplyTo":"44FD729D.4020208@shadowen.org","subject":"Re: remote_get_heads: reference length limit","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-09-05T16:01:39Z","receivedAt":"2006-09-05T16:01:39Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andy Whitcroft <apw@shadowen.org> writes:\n\n> I've been having trouble with git push apparently resending the entire\n> commit trace for the branch each and every time I push.  Poking at the\n> source it seems this is due to a length limit on reference names as\n> pulled from the remote repository.\n>\n> When we are building the pack to send we are sent a list of remote\n> heads.  get_remote_heads() reads these in, validates them and finally\n> adds them to the remote_refs list.  Part of the validation is a simple\n> check for size and form; check_ref().\n>\n> static int check_ref(const char *name, int len, unsigned int flags)\n> {\n>         if (!flags)\n>                 return 1;\n>\n>         if (len > 45 || memcmp(name, \"refs/\", 5))\n>                 return 0;\n> [...]\n> }\n>\n> With the refs/heads/ prefix included this limits the head names to 34\n> characters.  From what I can see there is no good reason for this limit\n> to be so low.  I can see we don't want the remote end bloating us out of\n> control, but we are already limiting the lines which contain these\n> references to 1000 bytes and making no attempt to limit the number of\n> them the remote server can send us.  There seems to be no limits imposed\n> on the name length other than MAX_PATHLEN.\n>\n> Can anyone see a reason to keep this (len > 45) check?\n\nYes, it was a mis-conversion done with commit 2718ff0.\nThe data this part (and the caller) is dealing with are\npeek-remote output (40-byte hex object name, tab and\n\"refs/...\").  Before that commit we had a check like this:\n\n+\t\tif (ignore_funny && 45 < len && !memcmp(name, \"refs/\", 5) &&\n+\t\t    check_ref_format(name + 5))\n+\t\t\tcontinue;\n\nThat is, \"if we are ignoring funny fake refs, continue when the\nname begins with refs/ and is magic (e.g. refs/heads/master^{});\nby the way that can only happen if the entire line length is\nmore than 45 bytes (because of 40-byte hex plus tab, anything\nshorter than that cannot even fit \"refs/\" at the beginning of\nthe name).\n\nBut when the check_ref() function was written it was made to\ntake the name length as \"len\", so I do not think there is a\nreason to check the name against 45 at all.  Also the direction\nof the comparison is wrong in the new code.  It was meant to be\n\"line must be at least this long otherwise it is not right\".\n\nI think you can just drop \"len > 45 || \" part.\n"},{"id":"26367","messageId":"20060905190017.GA24102@shadowen.org","threadId":"5487","inReplyTo":"7v64g2i9p8.fsf@assigned-by-dhcp.cox.net","subject":"[PATCH] send pack remove remote reference limit","fromName":"Andy Whitcroft","fromEmail":"apw@shadowen.org","sentAt":"2006-09-05T19:00:17Z","receivedAt":"2006-09-05T19:00:17Z","isPatch":true,"sender":{"key":"apw@shadowen.org","avatar":"https://gravatar.com/avatar/d3088262854661a913ef35cc40fedcc270142d4461791142bc1ea0b2a4e2e147?d=mp&s=160"},"body":"send-pack: remove remote reference limit\n\nWhen build a pack for a push we query the remote copy for existant\nheads.  These are used to prune unnecessary objects from the pack.\nAs we receive the remote references in get_remote_heads() we validate\nthe reference names via check_ref() which includes a length check;\nrejecting those >45 characters in size.\n\nThis is a miss converted change, it was originally designed to reject\nmessages which were less than 45 characters in length (a 40 character\nsha1 and refs/) to prevent comparing unitialised memory.  check_ref()\nnow gets the raw length so check for at least 5 characters.\n\nSigned-off-by: Andy Whitcroft <apw@shadowen.org>\n---\ndiff --git a/connect.c b/connect.c\nindex e501ccc..371aea1 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -17,7 +17,7 @@ static int check_ref(const char *name, i\n \tif (!flags)\n \t\treturn 1;\n \n-\tif (len > 45 || memcmp(name, \"refs/\", 5))\n+\tif (len < 5 || memcmp(name, \"refs/\", 5))\n \t\treturn 0;\n \n \t/* Skip the \"refs/\" part */\n"}]}