@matheusbsbs / matheus-bagliano / commits / ec91ffca04

Fork 0

verify_repository_format(): complain about new extensions in v0 repo

We made the mistake in the past of respecting extensions.* even when the repository format version was set to 0. This is bad because forgetting to bump the repository version means that older versions of Git (which do not know about our extensions) won't complain. I.e., it's not a problem in itself, but it means your repository is in a state which does not give you the protection you think you're getting from older versions. For compatibility reasons, we are stuck with that decision for existing extensions. However, we'd prefer not to extend the damage further. We can do that by catching any newly-added extensions and complaining about the repository format. Note that this is a pretty heavy hammer: we'll refuse to work with the repository at all. A lesser option would be to ignore (possibly with a warning) any new extensions. But because of the way the extensions are handled, that puts the burden on each new extension that is added to remember to "undo" itself (because they are handled before we know for sure whether we are in a v1 repo or not, since we don't insist on a particular ordering of config entries). So one option would be to rewrite that handling to record any new extensions (and their values) during the config parse, and then only after proceed to handle new ones only if we're in a v1 repository. But I'm not sure if it's worth the trouble: - ignoring extensions is likely to end up with broken results anyway (e.g., ignoring a proposed objectformat extension means parsing any object data is likely to encounter errors) - this is a sign that whatever tool wrote the extension field is broken. We may be better off notifying immediately and forcefully so that such tools don't even appear to work accidentally. The only downside is that fixing the situation is a little tricky, because programs like "git config" won't want to work with the repository. But: git config --file=.git/config core.repositoryformatversion 1 should still suffice. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Jul 16, 2020 at 08:25 UTC ec91ffca0455e1a8a0525fb2a10814447d80c456

3 files changed +85 -16

cache.h

@@ -1044,6 +1044,7 @@ struct repository_format {

1044 int hash_algo;

1045 char *work_tree;

1046 struct string_list unknown_extensions;

1047 + struct string_list v1_only_extensions;

1048 };

1049

1050 /*

@@ -1057,6 +1058,7 @@ struct repository_format {

1058 .is_bare = -1, \

1059 .hash_algo = GIT_HASH_SHA1, \

1060 .unknown_extensions = STRING_LIST_INIT_DUP, \

1061 + .v1_only_extensions = STRING_LIST_INIT_DUP, \

1062 }

1063

1064 /*

setup.c

+80 -16

@@ -447,6 +447,54 @@ static int read_worktree_config(const char *var, const char *value, void *vdata)

447 return 0;

448 }

449

450 +enum extension_result {

451 + EXTENSION_ERROR = -1, /* compatible with error(), etc */

452 + EXTENSION_UNKNOWN = 0,

453 + EXTENSION_OK = 1

454 +};

455 +

456 +/*

457 + * Do not add new extensions to this function. It handles extensions which are

458 + * respected even in v0-format repositories for historical compatibility.

459 + */

460 +static enum extension_result handle_extension_v0(const char *var,

461 + const char *value,

462 + const char *ext,

463 + struct repository_format *data)

464 +{

465 + if (!strcmp(ext, "noop")) {

466 + return EXTENSION_OK;

467 + } else if (!strcmp(ext, "preciousobjects")) {

468 + data->precious_objects = git_config_bool(var, value);

469 + return EXTENSION_OK;

470 + } else if (!strcmp(ext, "partialclone")) {

471 + if (!value)

472 + return config_error_nonbool(var);

473 + data->partial_clone = xstrdup(value);

474 + return EXTENSION_OK;

475 + } else if (!strcmp(ext, "worktreeconfig")) {

476 + data->worktree_config = git_config_bool(var, value);

477 + return EXTENSION_OK;

478 + }

479 +

480 + return EXTENSION_UNKNOWN;

481 +}

482 +

483 +/*

484 + * Record any new extensions in this function.

485 + */

486 +static enum extension_result handle_extension(const char *var,

487 + const char *value,

488 + const char *ext,

489 + struct repository_format *data)

490 +{

491 + if (!strcmp(ext, "noop-v1")) {

492 + return EXTENSION_OK;

493 + }

494 +

495 + return EXTENSION_UNKNOWN;

496 +}

497 +

498 static int check_repo_format(const char *var, const char *value, void *vdata)

499 {

500 struct repository_format *data = vdata;

@@ -455,23 +503,25 @@ static int check_repo_format(const char *var, const char *value, void *vdata)

503 if (strcmp(var, "core.repositoryformatversion") == 0)

504 data->version = git_config_int(var, value);

505 else if (skip_prefix(var, "extensions.", &ext)) {

458 - /*

459 - * record any known extensions here; otherwise,

460 - * we fall through to recording it as unknown, and

461 - * check_repository_format will complain

462 - */

463 - if (!strcmp(ext, "noop"))

464 - ;

465 - else if (!strcmp(ext, "preciousobjects"))

466 - data->precious_objects = git_config_bool(var, value);

467 - else if (!strcmp(ext, "partialclone")) {

468 - if (!value)

469 - return config_error_nonbool(var);

470 - data->partial_clone = xstrdup(value);

471 - } else if (!strcmp(ext, "worktreeconfig"))

472 - data->worktree_config = git_config_bool(var, value);

473 - else

506 + switch (handle_extension_v0(var, value, ext, data)) {

507 + case EXTENSION_ERROR:

508 + return -1;

509 + case EXTENSION_OK:

510 + return 0;

511 + case EXTENSION_UNKNOWN:

512 + break;

513 + }

514 +

515 + switch (handle_extension(var, value, ext, data)) {

516 + case EXTENSION_ERROR:

517 + return -1;

518 + case EXTENSION_OK:

519 + string_list_append(&data->v1_only_extensions, ext);

520 + return 0;

521 + case EXTENSION_UNKNOWN:

522 string_list_append(&data->unknown_extensions, ext);

523 + return 0;

524 + }

525 }

526

527 return read_worktree_config(var, value, vdata);

@@ -510,6 +560,7 @@ static int check_repository_format_gently(const char *gitdir, struct repository_

560 set_repository_format_partial_clone(candidate->partial_clone);

561 repository_format_worktree_config = candidate->worktree_config;

562 string_list_clear(&candidate->unknown_extensions, 0);

563 + string_list_clear(&candidate->v1_only_extensions, 0);

564

565 if (repository_format_worktree_config) {

566 /*

@@ -588,6 +639,7 @@ int read_repository_format(struct repository_format *format, const char *path)

639 void clear_repository_format(struct repository_format *format)

640 {

641 string_list_clear(&format->unknown_extensions, 0);

642 + string_list_clear(&format->v1_only_extensions, 0);

643 free(format->work_tree);

644 free(format->partial_clone);

645 init_repository_format(format);

@@ -613,6 +665,18 @@ int verify_repository_format(const struct repository_format *format,

665 return -1;

666 }

667

668 + if (format->version == 0 && format->v1_only_extensions.nr) {

669 + int i;

670 +

671 + strbuf_addstr(err,

672 + _("repo version is 0, but v1-only extensions found:"));

673 +

674 + for (i = 0; i < format->v1_only_extensions.nr; i++)

675 + strbuf_addf(err, "\n\t%s",

676 + format->v1_only_extensions.items[i].string);

677 + return -1;

678 + }

679 +

680 return 0;

681 }

682

t/t1302-repo-version.sh

@@ -83,6 +83,9 @@ allow 1

83 allow 1 noop

84 abort 1 no-such-extension

85 allow 0 no-such-extension

86 +allow 0 noop

87 +abort 0 noop-v1

88 +allow 1 noop-v1

89 EOF

91 test_expect_success 'precious-objects allowed' '