fast-export: fix anonymized tag using original length
PATCH 1/1 fast-export: fix anonymized tag using original length
From: Tal Kelrich <[email protected]>
Commit 7f4075949686 (fast-export: tighten anonymize_mem() interface to handle only strings, 2020-06-23) changed the interface used in anonymizing strings, but failed to update the size of annotated tag messages to match the new anonymized string.
As a result, exporting tags having messages longer than 13 characters would create output that couldn't be parsed by fast-import, as the data length indicated was larger than the data output.
Reset the message size when anonymizing, and add a tag with a "long" message to the test.
---
fast-export: fix anonymized tag using original length
Fixes an issue with fast-export anonymization, where any original annotated tag message longer than 13 characters would make the output unimportable by fast-import (as the data length indicated the original length).
This also resolves a more minor issue, where if the original message was short, the anonymized tag message would be truncated.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1030%2Fhasturkun%2Ffast-export-tag-length-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1030/hasturkun/fast-export-tag-length-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1030
builtin/fast-export.c | 1 + t/t9351-fast-export-anonymize.sh | 10 ++++++---- 2 files changed, 7 insertions(+), 4 deletions(-)
builtin/fast-export.c +1 -0
@@ -821,6 +821,7 @@ static void handle_tag(const char *name, struct tag *tag)static struct hashmap tags; message =anonymize_str (&tags, anonymize_tag, message, message_size,NULL ); + message_size =strlen (message); } }
t/t9351-fast-export-anonymize.sh +6 -4
@@ -18,7 +18,8 @@ test_expect_success 'setup simple repo' ' git update-index --add --cacheinfo 160000,$fake_commit,link1 && git update-index --add --cacheinfo 160000,$fake_commit,link2 && git commit -m "add gitlink" && - git tag -m "annotated tag" mytag + git tag -m "annotated tag" mytag && + git tag -m "annotated tag with long message" longtag ' test_expect_success 'export anonymized stream' '
@@ -55,7 +56,8 @@ test_expect_success 'stream retains other as refname' ' test_expect_success 'stream omits other refnames' ' ! grep main stream && - ! grep mytag stream + ! grep mytag stream && + ! grep longtag stream ' test_expect_success 'stream omits identities' '
@@ -118,9 +120,9 @@ test_expect_success 'identical gitlinks got identical oid' ' test_line_count = 1 commits ' -test_expect_success 'tag points to branch tip' ' +test_expect_success 'all tags point to branch tip' ' git rev-parse $other_branch >expect && - git for-each-ref --format="%(*objectname)" | grep . >actual && + git for-each-ref --format="%(*objectname)" | grep . | uniq >actual && test_cmp expect actual '
base-commit: 6c40894d2466d4e7fddc047a05116aa9d14712ee
Signature · 1 line
gitgitgadget
Quoted 18 lines · Tal Kelrich via GitGitGadget
From: Tal Kelrich <[email protected]>
Commit
7f4075949686(fast-export: tightenanonymize_mem()interface to handle only strings, 2020-06-23) changed the interface used in anonymizing strings, but failed to update the size of annotated tag messages to match the new anonymized string.As a result, exporting tags having messages longer than 13 characters would create output that couldn't be parsed by fast-import, as the data length indicated was larger than the data output.
Reset the message size when anonymizing, and add a tag with a "long" message to the test.
Signed-off-by Tal Kelrich <[email protected]>---
fast-export: fix anonymized tag using original length
Quoted 7 lines
Fixes an issue with fast-export anonymization, where any original annotated tag message longer than 13 characters would make the output unimportable by fast-import (as the data length indicated the original length).
This also resolves a more minor issue, where if the original message was short, the anonymized tag message would be truncated.
Thanks. I've looked at all places where anonymize_str() is used and found that this is the only problematic caller.
Will queue.
Quoted 7 lines · Tal Kelrich via GitGitGadget
From: Tal Kelrich <[email protected]>
Commit
7f4075949686(fast-export: tightenanonymize_mem()interface to handle only strings, 2020-06-23) changed the interface used in anonymizing strings, but failed to update the size of annotated tag messages to match the new anonymized string.
Quoted 6 lines
As a result, exporting tags having messages longer than 13 characters would create output that couldn't be parsed by fast-import, as the data length indicated was larger than the data output.
Reset the message size when anonymizing, and add a tag with a "long" message to the test.
Thanks, good catch and nicely explained. I wondered where the "13" came from, and it is "tag message %d" (so really, it depends how many tags you have :) ).
builtin/fast-export.c static void handle_tag(const char *name, struct tag *tag) · line 826
Quoted 11 lines
builtin/fast-export.c +1
-0@@ -821,6 +821,7 @@ static void handle_tag(const char *name, struct tag *tag)static struct hashmap tags; message =anonymize_str (&tags, anonymize_tag, message, message_size,NULL ); + message_size =strlen (message); } }
In the other callers, we just treat the return value from anonymize_str() like a string afterwards, so they naturally adapted to the anonymized content. But here the rest of the tag code uses the size, so we have to update it. Makes sense (both why the fix here, but why a similar fix is not needed elsewhere).
t/t9351-fast-export-anonymize.sh test_expect_success 'setup simple repo' ' · line 23
Quoted 12 lines
t/t9351-fast-export-anonymize.sh +2
-1@@ -18,7 +18,8 @@ test_expect_success 'setup simple repo' ' git update-index --add --cacheinfo 160000,$fake_commit,link1 && git update-index --add --cacheinfo 160000,$fake_commit,link2 && git commit -m "add gitlink" && - git tag -m "annotated tag" mytag + git tag -m "annotated tag" mytag && + git tag -m "annotated tag with long message" longtag '
I was curious why the existing tests did not catch even the truncation. I think we only assert that the original tag content does not appear. But moreover, the values before/after anonymization coincidentally have exactly the same length:
annotated tag
tag message 1
so it did not even trigger the bug.
-Peff