@matheusbsbs / matheus-bagliano / commits / d74b1fd54f

attr: fix silently splitting up lines longer than 2048 bytes

When reading attributes from a file we use fgets(3P) with a buffer size of 2048 bytes. This means that as soon as a line exceeds the buffer size we split it up into multiple parts and parse each of them as a separate pattern line. This is of course not what the user intended, and even worse the behaviour is inconsistent with how we read attributes from the index. Fix this bug by converting the code to use `strbuf_getline()` instead. This will indeed read in the whole line, which may theoretically lead to an out-of-memory situation when the gitattributes file is huge. We're about to reject any gitattributes files larger than 100MB in the next commit though, which makes this less of a concern. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Dec 1, 2022 at 15:45 UTC d74b1fd54fdbc45966d12ea907dece11e072fb2b

2 files changed +28 -6

attr.c

+7 -6

@@ -699,21 +699,22 @@ void git_attr_set_direction(enum git_attr_direction new_direction)

699

700 static struct attr_stack *read_attr_from_file(const char *path, int macro_ok)

701 {

702 + struct strbuf buf = STRBUF_INIT;

703 FILE *fp = fopen_or_warn(path, "r");

704 struct attr_stack *res;

704 - char buf[2048];

705 int lineno = 0;

706

707 if (!fp)

708 return NULL;

709 res = xcalloc(1, sizeof(*res));

710 - while (fgets(buf, sizeof(buf), fp)) {

711 - char *bufp = buf;

712 - if (!lineno)

713 - skip_utf8_bom(&bufp, strlen(bufp));

714 - handle_attr_line(res, bufp, path, ++lineno, macro_ok);

710 + while (strbuf_getline(&buf, fp) != EOF) {

711 + if (!lineno && starts_with(buf.buf, utf8_bom))

712 + strbuf_remove(&buf, 0, strlen(utf8_bom));

713 + handle_attr_line(res, buf.buf, path, ++lineno, macro_ok);

714 }

715 +

716 fclose(fp);

717 + strbuf_release(&buf);

718 return res;

719 }

720

t/t0003-attributes.sh

+21

@@ -339,4 +339,25 @@ test_expect_success 'query binary macro directly' '

339 test_cmp expect actual

340 '

341

342 +test_expect_success 'large attributes line ignores trailing content in tree' '

343 + test_when_finished "rm .gitattributes" &&

344 + # older versions of Git broke lines at 2048 bytes; the 2045 bytes

345 + # of 0-padding here is accounting for the three bytes of "a 1", which

346 + # would knock "trailing" to the "next" line, where it would be

347 + # erroneously parsed.

348 + printf "a %02045dtrailing attribute\n" 1 >.gitattributes &&

349 + git check-attr --all trailing >actual 2>err &&

350 + test_must_be_empty err &&

351 + test_must_be_empty actual

352 +'

353 +

354 +test_expect_success 'large attributes line ignores trailing content in index' '

355 + test_when_finished "git update-index --remove .gitattributes" &&

356 + blob=$(printf "a %02045dtrailing attribute\n" 1 | git hash-object -w --stdin) &&

357 + git update-index --add --cacheinfo 100644,$blob,.gitattributes &&

358 + git check-attr --cached --all trailing >actual 2>err &&

359 + test_must_be_empty err &&

360 + test_must_be_empty actual

361 +'

362 +

363 test_done