diff mbox series

[10/20] t5319: make test work with SHA-256

Message ID 20191221194936.1346664-11-sandals@crustytoothpaste.net (mailing list archive)
State New, archived
Headers show
Series SHA-256 test fixes, part 7 | expand

Commit Message

brian m. carlson Dec. 21, 2019, 7:49 p.m. UTC 
This test corrupts various locations in a multi-pack index to test
various error responses.  However, these offsets differ between SHA-1
indexes and SHA-256 indexes due to differences in object length.  Use
test_oid to look up the correct offsets based on the algorithm.
---
 t/t5319-multi-pack-index.sh | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

Comments

Eric Sunshine Dec. 22, 2019, 12:06 a.m. UTC | #1 
On Sat, Dec 21, 2019 at 2:52 PM brian m. carlson
<sandals@crustytoothpaste.net> wrote:
> This test corrupts various locations in a multi-pack index to test
> various error responses.  However, these offsets differ between SHA-1
> indexes and SHA-256 indexes due to differences in object length.  Use
> test_oid to look up the correct offsets based on the algorithm.
> ---

Missing sign-off.
brian m. carlson Dec. 22, 2019, 5:47 p.m. UTC | #2 
On 2019-12-22 at 00:06:47, Eric Sunshine wrote:
> On Sat, Dec 21, 2019 at 2:52 PM brian m. carlson
> <sandals@crustytoothpaste.net> wrote:
> > This test corrupts various locations in a multi-pack index to test
> > various error responses.  However, these offsets differ between SHA-1
> > indexes and SHA-256 indexes due to differences in object length.  Use
> > test_oid to look up the correct offsets based on the algorithm.
> > ---
> 
> Missing sign-off.

Will fix.  In the odd case that no reroll is necessary, I'll include it
here:

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Michael Clark Dec. 23, 2019, 1:25 a.m. UTC | #3 
> On 23/12/2019, at 6:48 AM, brian m. carlson <sandals@crustytoothpaste.net> wrote:
> 
> On 2019-12-22 at 00:06:47, Eric Sunshine wrote:
>>> On Sat, Dec 21, 2019 at 2:52 PM brian m. carlson
>>> <sandals@crustytoothpaste.net> wrote:
>>> This test corrupts various locations in a multi-pack index to test
>>> various error responses.  However, these offsets differ between SHA-1
>>> indexes and SHA-256 indexes due to differences in object length.  Use
>>> test_oid to look up the correct offsets based on the algorithm.
>>> ---
>> 
>> Missing sign-off.
> 
> Will fix.  In the odd case that no reroll is necessary, I'll include it
> here:
> 
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>

You have a fan for your work! This is the hard work. I missed adding sign-off in my patch series also. Fixed in the branch mentioned in the email I sent to the list, about alternative SHA algorithms.

I can review this patch series if you like. I’ve applied them in my local tree and run tests. Nothing blew up...

Happy holidays.
Derrick Stolee Dec. 26, 2019, 2:50 p.m. UTC | #4 
On 12/21/2019 2:49 PM, brian m. carlson wrote:
> This test corrupts various locations in a multi-pack index to test
> various error responses.  However, these offsets differ between SHA-1
> indexes and SHA-256 indexes due to differences in object length.  Use
> test_oid to look up the correct offsets based on the algorithm.
> ---
>  t/t5319-multi-pack-index.sh | 22 ++++++++++++++++++----
>  1 file changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh
> index 464bb68e89..43a7a66c9d 100755
> --- a/t/t5319-multi-pack-index.sh
> +++ b/t/t5319-multi-pack-index.sh
> @@ -28,6 +28,20 @@ midx_read_expect () {
>  	test_cmp expect actual
>  }
>  
> +test_expect_success 'setup' '
> +	test_oid_init &&
> +	test_oid_cache <<-EOF
> +	idxoff sha1:2999
> +	idxoff sha256:3739
> +
> +	packnameoff sha1:652
> +	packnameoff sha256:940
> +
> +	fanoutoff sha1:1
> +	fanoutoff sha256:3
> +	EOF
> +'
> +
>  test_expect_success 'write midx with no packs' '
>  	test_when_finished rm -f pack/multi-pack-index &&
>  	git multi-pack-index --object-dir=. write &&
> @@ -225,7 +239,7 @@ test_expect_success 'verify bad signature' '
>  		"multi-pack-index signature"
>  '
>  
> -HASH_LEN=20
> +HASH_LEN=$(test_oid rawsz)

I'm glad this was an easy update.

> +MIDX_OFFSET_OID_FANOUT=$(($MIDX_OFFSET_PACKNAMES + $(test_oid packnameoff)))
> +MIDX_BYTE_OID_FANOUT_ORDER=$((MIDX_OFFSET_OID_FANOUT + 250 * $MIDX_OID_FANOUT_WIDTH + $(test_oid fanoutoff)))

I see why these are necessary: we need to find an exact position that
causes an order check to fail. In that sense, the offsets need to match
the early bytes of a packfile name or an OID fanout value.

> @@ -387,7 +401,7 @@ test_expect_success 'force some 64-bit offsets with pack-objects' '
>  	pack64=$(git pack-objects --index-version=2,0x40 objects64/pack/test-64 <obj-list) &&
>  	idx64=objects64/pack/test-64-$pack64.idx &&
>  	chmod u+w $idx64 &&
> -	corrupt_data $idx64 2999 "\02" &&
> +	corrupt_data $idx64 $(test_oid idxoff) "\02" &&

Sorry that this was not a better-calculated value, but
your approach works well here.

Thanks,
-Stolee
brian m. carlson Dec. 27, 2019, 9:35 p.m. UTC | #5 
On 2019-12-26 at 14:50:09, Derrick Stolee wrote:
> On 12/21/2019 2:49 PM, brian m. carlson wrote:
> > @@ -387,7 +401,7 @@ test_expect_success 'force some 64-bit offsets with pack-objects' '
> >  	pack64=$(git pack-objects --index-version=2,0x40 objects64/pack/test-64 <obj-list) &&
> >  	idx64=objects64/pack/test-64-$pack64.idx &&
> >  	chmod u+w $idx64 &&
> > -	corrupt_data $idx64 2999 "\02" &&
> > +	corrupt_data $idx64 $(test_oid idxoff) "\02" &&
> 
> Sorry that this was not a better-calculated value, but
> your approach works well here.

That's okay.  I appreciate that you attempted to make things robust, and
the documentary value of the variables was helpful in fixing these
tests.
diff mbox series

Patch

diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh
index 464bb68e89..43a7a66c9d 100755
--- a/t/t5319-multi-pack-index.sh
+++ b/t/t5319-multi-pack-index.sh
@@ -28,6 +28,20 @@  midx_read_expect () {
 	test_cmp expect actual
 }
 
+test_expect_success 'setup' '
+	test_oid_init &&
+	test_oid_cache <<-EOF
+	idxoff sha1:2999
+	idxoff sha256:3739
+
+	packnameoff sha1:652
+	packnameoff sha256:940
+
+	fanoutoff sha1:1
+	fanoutoff sha256:3
+	EOF
+'
+
 test_expect_success 'write midx with no packs' '
 	test_when_finished rm -f pack/multi-pack-index &&
 	git multi-pack-index --object-dir=. write &&
@@ -225,7 +239,7 @@  test_expect_success 'verify bad signature' '
 		"multi-pack-index signature"
 '
 
-HASH_LEN=20
+HASH_LEN=$(test_oid rawsz)
 NUM_OBJECTS=74
 MIDX_BYTE_VERSION=4
 MIDX_BYTE_OID_VERSION=5
@@ -238,9 +252,9 @@  MIDX_CHUNK_LOOKUP_WIDTH=12
 MIDX_OFFSET_PACKNAMES=$(($MIDX_HEADER_SIZE + \
 			 $MIDX_NUM_CHUNKS * $MIDX_CHUNK_LOOKUP_WIDTH))
 MIDX_BYTE_PACKNAME_ORDER=$(($MIDX_OFFSET_PACKNAMES + 2))
-MIDX_OFFSET_OID_FANOUT=$(($MIDX_OFFSET_PACKNAMES + 652))
+MIDX_OFFSET_OID_FANOUT=$(($MIDX_OFFSET_PACKNAMES + $(test_oid packnameoff)))
 MIDX_OID_FANOUT_WIDTH=4
-MIDX_BYTE_OID_FANOUT_ORDER=$((MIDX_OFFSET_OID_FANOUT + 250 * $MIDX_OID_FANOUT_WIDTH + 1))
+MIDX_BYTE_OID_FANOUT_ORDER=$((MIDX_OFFSET_OID_FANOUT + 250 * $MIDX_OID_FANOUT_WIDTH + $(test_oid fanoutoff)))
 MIDX_OFFSET_OID_LOOKUP=$(($MIDX_OFFSET_OID_FANOUT + 256 * $MIDX_OID_FANOUT_WIDTH))
 MIDX_BYTE_OID_LOOKUP=$(($MIDX_OFFSET_OID_LOOKUP + 16 * $HASH_LEN))
 MIDX_OFFSET_OBJECT_OFFSETS=$(($MIDX_OFFSET_OID_LOOKUP + $NUM_OBJECTS * $HASH_LEN))
@@ -387,7 +401,7 @@  test_expect_success 'force some 64-bit offsets with pack-objects' '
 	pack64=$(git pack-objects --index-version=2,0x40 objects64/pack/test-64 <obj-list) &&
 	idx64=objects64/pack/test-64-$pack64.idx &&
 	chmod u+w $idx64 &&
-	corrupt_data $idx64 2999 "\02" &&
+	corrupt_data $idx64 $(test_oid idxoff) "\02" &&
 	midx64=$(git multi-pack-index --object-dir=objects64 write) &&
 	midx_read_expect 1 63 5 objects64 " large-offsets"
 '