)]}'
{
  "commit": "aaaa88182266f91ef99ff24847dabc44e08176b2",
  "tree": "22c1460946161b46870522fedd17667e4ed6b409",
  "parents": [
    "268fbcd172cdb306e8a3e7143cc16677c963d6cd"
  ],
  "author": {
    "name": "Jeff King",
    "email": "peff@peff.net",
    "time": "Wed Oct 24 03:27:52 2018 -0400"
  },
  "committer": {
    "name": "Junio C Hamano",
    "email": "gitster@pobox.com",
    "time": "Fri Oct 26 10:30:59 2018 +0900"
  },
  "message": "upload-pack: fix broken if/else chain in config callback\n\nThe upload_pack_config() callback uses an if/else chain\nlike:\n\n  if (!strcmp(var, \"a\"))\n     ...\n  else if (!strcmp(var, \"b\"))\n     ...\n  etc\n\nThis works as long as the conditions are mutually exclusive,\nbut one of them is not. 20b20a22f8 (upload-pack: provide a\nhook for running pack-objects, 2016-05-18) added:\n\n  else if (current_config_scope() !\u003d CONFIG_SCOPE_REPO) {\n     ... check some more options ...\n  }\n\nThat was fine in that commit, because it came at the end of\nthe chain.  But later, 10ac85c785 (upload-pack: add object\nfiltering for partial clone, 2017-12-08) did this:\n\n  else if (current_config_scope() !\u003d CONFIG_SCOPE_REPO) {\n     ... check some more options ...\n  } else if (!strcmp(\"uploadpack.allowfilter\", var))\n     ...\n\nWe\u0027d always check the scope condition first, meaning we\u0027d\n_only_ respect allowfilter when it\u0027s in the repo config. You\ncan see this with:\n\n  git -c uploadpack.allowfilter\u003dtrue upload-pack . | head -1\n\nwhich will not advertise the filter capability (but will\nafter this patch). We never noticed because:\n\n  - our tests always set it in the repo config\n\n  - in protocol v2, we use a different code path that\n    actually calls repo_config_get_bool() separately, so\n    that _does_ work. Real-world people experimenting with\n    this may be using v2.\n\nThe more recent uploadpack.allowrefinwant option is in the\nsame boat.\n\nThere are a few possible fixes:\n\n  1. Bump the scope conditional back to the bottom of the\n     chain. But that just means somebody else is likely to\n     make the same mistake later.\n\n  2. Make the conditional more like the others. I.e.:\n\n       else if (!current_config_scope() !\u003d CONFIG_SCOPE_REPO \u0026\u0026\n                !strcmp(var, \"uploadpack.notallowedinrepo\"))\n\n     This works, but the idea of the original structure was\n     that we may grow multiple sensitive options like this.\n\n  3. Pull it out of the chain entirely. The chain mostly\n     serves to avoid extra strcmp() calls after we\u0027ve found\n     a match. But it\u0027s not worth caring about those. In the\n     worst case, when there isn\u0027t a match, we\u0027re already\n     hitting every strcmp (and this happens regularly for\n     stuff like \"core.bare\", etc).\n\nThis patch does (3).\n\nSigned-off-by: Jeff King \u003cpeff@peff.net\u003e\nReviewed-by: Josh Steadmon \u003csteadmon@google.com\u003e\nSigned-off-by: Junio C Hamano \u003cgitster@pobox.com\u003e\n",
  "tree_diff": [
    {
      "type": "modify",
      "old_id": "87c6722ea58207a9b369e75f7cf7ac16a705330b",
      "old_mode": 33188,
      "old_path": "upload-pack.c",
      "new_id": "a47e4c2692e24085fdbf7f523b334bd193f17305",
      "new_mode": 33188,
      "new_path": "upload-pack.c"
    }
  ]
}