Massive bug fix.

GITObject subclasses were detecting their SHA1 hash values by hashing their data contents when being created. This is wrong as the sha1 values are calculated from the "$type $size" prefixed objects as they are written to the file system. The SHA1 hash values are now passed directly into the objects as they are created. This alteration required modification to all classes which load objects from either the file system or PACK files.
schacon · Jan 17, 2010 · 81ff768 · 81ff768
1 parent 4c1f6e7
commit 81ff768
Show file tree

Hide file tree

Showing 21 changed files with 83 additions and 50 deletions.
diff --git a/Source/GITBlob.h b/Source/GITBlob.h
@@ -38,6 +38,6 @@
  * \param error NSError describing the error if one occurs
  * \return A blob object
  */
-+ (GITBlob *)blobFromData: (NSData *)data repo: (GITRepo *)repo error: (NSError **)error;
++ (GITBlob *)blobFromData: (NSData *)data sha1: (GITObjectHash *)objectHash repo: (GITRepo *)repo error: (NSError **)error;
 
 @end
diff --git a/Source/GITBlob.m b/Source/GITBlob.m
@@ -23,12 +23,12 @@ + (GITObjectType)type {
     return GITObjectTypeBlob;
 }
 
-+ (GITBlob *)blobFromData: (NSData *)data repo: (GITRepo *)repo error: (NSError **)error {
-    return [[[self alloc] initFromData:data repo:repo error:error] autorelease];
++ (GITBlob *)blobFromData: (NSData *)data sha1: (GITObjectHash *)objectHash repo: (GITRepo *)repo error: (NSError **)error {
+    return [[[self alloc] initFromData:data sha1:objectHash repo:repo error:error] autorelease];
 }
 
-- (id)initFromData: (NSData *)data repo: (GITRepo *)theRepo error: (NSError **)error {
-    if ( ![super initWithType:GITObjectTypeBlob sha1:[GITObjectHash objectHashWithData:[data sha1Digest]] repo:theRepo] )
+- (id)initFromData: (NSData *)data sha1: (GITObjectHash *)objectHash repo: (GITRepo *)theRepo error: (NSError **)error {
+    if ( ![super initWithType:GITObjectTypeBlob sha1:objectHash repo:theRepo] )
         return nil;
 
     self.content = data;

diff --git a/Source/GITCommit.h b/Source/GITCommit.h
@@ -68,7 +68,7 @@
  * \param error NSError describing the error that occurred
  * \return A commit object from the \a data
  */
-+ (GITCommit *)commitFromData: (NSData *)data repo: (GITRepo *)repo error: (NSError **)error;
++ (GITCommit *)commitFromData: (NSData *)data sha1: (GITObjectHash *)objectHash repo: (GITRepo *)repo error: (NSError **)error;
 
 //! \name Properties
 /*!

diff --git a/Source/GITCommit.m b/Source/GITCommit.m
@@ -47,12 +47,12 @@ + (GITObjectType)type {
     return GITObjectTypeCommit;
 }
 
-+ (GITCommit *)commitFromData: (NSData *)data repo: (GITRepo *)repo error: (NSError **)error {
-    return [[[self alloc] initFromData:data repo:repo error:error] autorelease];
++ (GITCommit *)commitFromData: (NSData *)data sha1: (GITObjectHash *)objectHash repo: (GITRepo *)repo error: (NSError **)error {
+    return [[[self alloc] initFromData:data sha1:objectHash repo:repo error:error] autorelease];
 }
 
-- (id)initFromData: (NSData *)data repo: (GITRepo *)theRepo error: (NSError **)error {
-    if ( ![super initWithType:GITObjectTypeCommit sha1:[GITObjectHash objectHashWithData:[data sha1Digest]] repo:theRepo] )
+- (id)initFromData: (NSData *)data sha1: (GITObjectHash *)objectHash repo: (GITRepo *)theRepo error: (NSError **)error {
+    if ( ![super initWithType:GITObjectTypeCommit sha1:objectHash repo:theRepo] )
         return nil;
 
     const char *dataBytes = [data bytes], *start = dataBytes;

diff --git a/Source/GITLooseObject.h b/Source/GITLooseObject.h
@@ -23,12 +23,14 @@
  */
 @interface GITLooseObject : NSObject {
     GITObjectType type;     //!< Type of the object the data represents
-    NSData *data;           //!< Data of the packed object
+    NSData *data;           //!< Data of the object
+    GITObjectHash *sha1;    //!< SHA1 hash of the object
 }
 
 //! \name Properties
 @property (assign) GITObjectType type;
 @property (copy) NSData *data;
+@property (retain) GITObjectHash *sha1;
 @property (assign,readonly) NSUInteger length;  //!< Size of the receivers data
 
 //! \name Creating and Initialising Loose Objects

diff --git a/Source/GITLooseObject.m b/Source/GITLooseObject.m
@@ -16,7 +16,7 @@
 
 @implementation GITLooseObject
 
-@synthesize type, data;
+@synthesize type, data, sha1;
 
 + (GITLooseObject *)looseObjectWithSha1: (GITObjectHash *)objectHash from: (NSString *)directory error: (NSError **)error {
     return [[[self alloc] initWithSha1:objectHash from:directory error:error] autorelease];
@@ -34,7 +34,9 @@ - (id)initWithSha1: (GITObjectHash *)objectHash from: (NSString *)directory erro
         return nil;
     }
 
-    NSString *sha1String = [objectHash unpackedString];
+    self.sha1 = objectHash;
+
+    NSString *sha1String = [self.sha1 unpackedString];
     NSString *pathSuffix = [NSString stringWithFormat:@"%@/%@", [sha1String substringToIndex:2], [sha1String substringFromIndex:2]];
     NSString *objectPath = [directory stringByAppendingPathComponent:pathSuffix];
 
@@ -56,18 +58,24 @@ - (id)initWithSha1: (GITObjectHash *)objectHash from: (NSString *)directory erro
     [typeStr release];
 
     NSRange prefixRange = [fileData rangeFrom:NSRangeEnd(typeRange) toByte:0x0];   // We don't care about the size
-    self.data = [fileData subdataFromIndex:NSRangeEnd(prefixRange)];
+    self.data = [fileData subdataFromIndex:NSRangeEnd(prefixRange) + 1];
     [fileData release];
 
     return self;
 }
 
+- (void)dealloc {
+    self.data = nil;
+    self.sha1 = nil;
+    [super dealloc];
+}
+
 - (NSUInteger)length {
     return [self.data length];
 }
 
 - (GITObject *)objectInRepo: (GITRepo *)repo error: (NSError **)error {
-    return [GITObject objectOfType:self.type withData:self.data repo:repo error:error];
+    return [GITObject objectOfType:self.type withData:self.data sha1:self.sha1 repo:repo error:error];
 }
 
 @end
diff --git a/Source/GITObject.h b/Source/GITObject.h
@@ -58,7 +58,7 @@ typedef enum {
  * \param[out] error NSError describing any errors which occurred
  * \return Instance of the receiving class
  */
-- (id)initFromData: (NSData *)data repo: (GITRepo *)repo error: (NSError **)error;
+- (id)initFromData: (NSData *)data sha1: (GITObjectHash *)objectHash repo: (GITRepo *)repo error: (NSError **)error;
 
 @end
 
@@ -115,7 +115,7 @@ typedef enum {
  * \param error NSError describing the error which occurred
  * \return object from the \a type from the \a data
  */
-+ (id)objectOfType: (GITObjectType)type withData: (NSData *)data repo: (GITRepo *)repo error: (NSError **)error;
++ (id)objectOfType: (GITObjectType)type withData: (NSData *)data sha1: (GITObjectHash *)objectHash repo: (GITRepo *)repo error: (NSError **)error;
 
 /*!
  * Returns a GITObject initialised with the \a type, \a objectHash and \a repo.

diff --git a/Source/GITObject.m b/Source/GITObject.m
@@ -54,9 +54,9 @@ + (GITObjectType)objectTypeForString: (NSString *)type {
     }
 }
 
-+ (id)objectOfType: (GITObjectType)type withData: (NSData *)data repo: (GITRepo *)repo error: (NSError **)error {
++ (id)objectOfType: (GITObjectType)type withData: (NSData *)data sha1: (GITObjectHash *)objectHash repo: (GITRepo *)repo error: (NSError **)error {
     Class objectClass = [self objectClassForType:type];
-    return [[[objectClass alloc] initFromData:data repo:repo error:error] autorelease];
+    return [[[objectClass alloc] initFromData:data sha1:objectHash repo:repo error:error] autorelease];
 }
 
 - (id)initWithType: (GITObjectType)theType sha1: (GITObjectHash *)theSha1 repo: (GITRepo *)theRepo {

diff --git a/Source/GITPackFileVersionTwo.h b/Source/GITPackFileVersionTwo.h
@@ -30,8 +30,8 @@ typedef struct {
 @property (copy) NSData *data;
 @property (retain) GITPackIndex *index;
 
-- (GITPackObject *)unpackObjectAtOffset: (off_t)offset error: (NSError **)error;
-- (GITPackObject *)unpackDeltaPackedObjectAtOffset: (off_t)offset objectHeader: (GITPackFileObjectHeader *)header error: (NSError **)error;
+- (GITPackObject *)unpackObjectAtOffset: (off_t)offset sha1: (GITObjectHash *)objectHash error: (NSError **)error;
+- (GITPackObject *)unpackDeltaPackedObjectAtOffset: (off_t)offset objectHeader: (GITPackFileObjectHeader *)header sha1: (GITObjectHash *)objectHash error: (NSError **)error;
 
 - (NSRange)checksumRange;
 

diff --git a/Source/GITPackFileVersionTwo.m b/Source/GITPackFileVersionTwo.m
@@ -49,7 +49,7 @@ - (GITPackObject *)unpackObjectWithSha1: (GITObjectHash *)objectHash error: (NSE
     off_t packOffset = [self.index packOffsetForSha1:objectHash error:error];
     if ( packOffset == NSNotFound )
         return nil;
-    return [self unpackObjectAtOffset:packOffset error:error];
+    return [self unpackObjectAtOffset:packOffset sha1:objectHash error:error];
 }
 
 - (GITPackFileObjectHeader *)unpackEntryHeaderAtOffset: (off_t *)offset intoHeader: (GITPackFileObjectHeader *)header {
@@ -78,7 +78,7 @@ - (GITPackFileObjectHeader *)unpackEntryHeaderAtOffset: (off_t *)offset intoHead
     return header;
 }
 
-- (GITPackObject *)unpackObjectAtOffset: (off_t)offset error:(NSError **)error {
+- (GITPackObject *)unpackObjectAtOffset: (off_t)offset sha1: (GITObjectHash *)objectHash error:(NSError **)error {
     GITPackFileObjectHeader header;
     [self unpackEntryHeaderAtOffset:&offset intoHeader:&header];
 
@@ -94,11 +94,11 @@ - (GITPackObject *)unpackObjectAtOffset: (off_t)offset error:(NSError **)error {
                 return nil;
             }
 
-            return [GITPackObject packObjectWithData:packData type:header.type];
+            return [GITPackObject packObjectWithData:packData sha1:objectHash type:header.type];
             break;
         case GITPackFileDeltaTypeOfs:
         case GITPackFileDeltaTypeRefs:
-            return [self unpackDeltaPackedObjectAtOffset:offset objectHeader:&header error:error];
+            return [self unpackDeltaPackedObjectAtOffset:offset objectHeader:&header sha1:objectHash error:error];
             break;
         default:
             GITError(error, GITPackFileErrorObjectTypeUnknown, NSLocalizedString(@"Unknown packed object type", @"GITPackFileErrorObjectTypeUnknown"));
@@ -108,7 +108,7 @@ - (GITPackObject *)unpackObjectAtOffset: (off_t)offset error:(NSError **)error {
     return nil;
 }
 
-- (GITPackObject *)unpackDeltaPackedObjectAtOffset: (off_t)offset objectHeader: (GITPackFileObjectHeader *)header error: (NSError **)error {
+- (GITPackObject *)unpackDeltaPackedObjectAtOffset: (off_t)offset objectHeader: (GITPackFileObjectHeader *)header sha1: (GITObjectHash *)objectHash error: (NSError **)error {
     NSData *packedData = [self.data subdataWithRange:NSMakeRange(offset, GITObjectHashPackedLength)];
     off_t baseOffset;
 
@@ -131,7 +131,7 @@ - (GITPackObject *)unpackDeltaPackedObjectAtOffset: (off_t)offset objectHeader:
         offset += used;
     }
 
-    GITPackObject *packObject = [self unpackObjectAtOffset:baseOffset error:error];
+    GITPackObject *packObject = [self unpackObjectAtOffset:baseOffset sha1:objectHash error:error];
     if ( !packObject ) {
         GITError(error, GITPackErrorObjectNotFound, NSLocalizedString(@"Base object for PACK delta not found", @"GITPackErrorObjectNotFound"));
         return nil;

diff --git a/Source/GITPackObject.h b/Source/GITPackObject.h
@@ -24,11 +24,13 @@
 @interface GITPackObject : NSObject {
     GITObjectType type;     //!< Type of the object the data represents
     NSData *data;           //!< Data of the packed object
+    GITObjectHash *sha1;    //!< SHA1 Hash of the packed object
 }
 
 //! \name Properties
 @property (assign) GITObjectType type;
 @property (copy) NSData *data;
+@property (retain) GITObjectHash *sha1;
 @property (readonly,assign) NSUInteger length;      //!< size of the receivers data
 
 //! \name Creating and Initialising GITPackObjects
@@ -40,7 +42,7 @@
  * \return new pack object
  * \sa initWithData:type:
  */
-+ (GITPackObject *)packObjectWithData: (NSData *)packData type: (GITObjectType)objectType;
++ (GITPackObject *)packObjectWithData: (NSData *)packData sha1: (GITObjectHash *)objectHash type: (GITObjectType)objectType;
 
 /*!
  * Create and return a new PACK object.
@@ -50,7 +52,7 @@
  * \return new pack object
  * \sa packObjectWithData:type:
  */
-- (id)initWithData: (NSData *)packData type: (GITObjectType)objectType;
+- (id)initWithData: (NSData *)packData sha1: (GITObjectHash *)objectHash type: (GITObjectType)objectType;
 
 //! \name Obtaining the contained object
 /*!

diff --git a/Source/GITPackObject.m b/Source/GITPackObject.m
@@ -12,38 +12,40 @@
 
 @implementation GITPackObject
 
-@synthesize data, type;
+@synthesize data, type, sha1;
 
-+ (GITPackObject *)packObjectWithData: (NSData *)packData type: (GITObjectType)objectType {
-    return [[[self alloc] initWithData:packData type:objectType] autorelease];
++ (GITPackObject *)packObjectWithData: (NSData *)packData sha1: (GITObjectHash *)objectHash type: (GITObjectType)objectType {
+    return [[[self alloc] initWithData:packData sha1:objectHash type:objectType] autorelease];
 }
 
-- (id)initWithData: (NSData *)packData type: (GITObjectType)objectType {
+- (id)initWithData: (NSData *)packData sha1: (GITObjectHash *)objectHash type: (GITObjectType)objectType {
     if ( ![super init] )
         return nil;
 
     self.data = packData;
     self.type = objectType;
+    self.sha1 = objectHash;
 
     return self;
 }
 
 - (void)dealloc {
     self.type = 0;
     self.data = nil;
+    self.sha1 = nil;
     [super dealloc];
 }
 
 - (id)packObjectByDeltaPatchingWithData: (NSData *)deltaData {
-    return [[self class] packObjectWithData:[self.data dataByDeltaPatchingWithData:deltaData] type:self.type];
+    return [[self class] packObjectWithData:[self.data dataByDeltaPatchingWithData:deltaData] sha1:self.sha1 type:self.type];
 }
 
 - (NSUInteger)length {
     return [self.data length];
 }
 
 - (GITObject *)objectInRepo: (GITRepo *)repo error: (NSError **)error {
-    return [GITObject objectOfType:self.type withData:self.data repo:repo error:error];
+    return [GITObject objectOfType:self.type withData:self.data sha1:self.sha1 repo:repo error:error];
 }
 
 @end
diff --git a/Source/GITTag.h b/Source/GITTag.h
@@ -64,6 +64,6 @@
  * \param error NSError describing the error that occurred
  * \return A tag object from the \a data
  */
-+ (GITTag *)tagFromData: (NSData *)data repo: (GITRepo *)repo error: (NSError **)error;
++ (GITTag *)tagFromData: (NSData *)data sha1: (GITObjectHash *)objectHash repo: (GITRepo *)repo error: (NSError **)error;
 
 @end
diff --git a/Source/GITTag.m b/Source/GITTag.m
@@ -48,12 +48,12 @@ + (GITObjectType)type {
     return GITObjectTypeTag;
 }
 
-+ (GITTag *)tagFromData: (NSData *)data repo: (GITRepo *)repo error: (NSError **)error {
-    return [[[self alloc] initFromData:data repo:repo error:error] autorelease];
++ (GITTag *)tagFromData: (NSData *)data sha1: (GITObjectHash *)objectHash repo: (GITRepo *)repo error: (NSError **)error {
+    return [[[self alloc] initFromData:data sha1:objectHash repo:repo error:error] autorelease];
 }
 
-- (id)initFromData: (NSData *)data repo: (GITRepo *)theRepo error: (NSError **)error {
-    if ( ![super initWithType:GITObjectTypeTag sha1:[GITObjectHash objectHashWithData:[data sha1Digest]] repo:theRepo] )
+- (id)initFromData: (NSData *)data sha1: (GITObjectHash *)objectHash repo: (GITRepo *)theRepo error: (NSError **)error {
+    if ( ![super initWithType:GITObjectTypeTag sha1:objectHash repo:theRepo] )
         return nil;
 
     const char *dataBytes = [data bytes], *start = dataBytes;

diff --git a/Source/GITTree.h b/Source/GITTree.h
@@ -46,6 +46,6 @@
  * \param error NSError describing any errors which occurred
  * \return A tree object
  */
-+ (GITTree *)treeFromData: (NSData *)data repo: (GITRepo *)repo error: (NSError **)error;
++ (GITTree *)treeFromData: (NSData *)data sha1: (GITObjectHash *)objectHash repo: (GITRepo *)repo error: (NSError **)error;
 
 @end
diff --git a/Source/GITTree.m b/Source/GITTree.m
@@ -31,12 +31,12 @@ + (GITObjectType)type {
     return GITObjectTypeTree;
 }
 
-+ (GITTree *)treeFromData: (NSData *)data repo: (GITRepo *)repo error: (NSError **)error {
-    return [[[self alloc] initFromData:data repo:repo error:error] autorelease];
++ (GITTree *)treeFromData: (NSData *)data sha1: (GITObjectHash *)objectHash repo: (GITRepo *)repo error: (NSError **)error {
+    return [[[self alloc] initFromData:data sha1:objectHash repo:repo error:error] autorelease];
 }
 
-- (id)initFromData: (NSData *)data repo: (GITRepo *)theRepo error: (NSError **)error {
-    if ( ![super initWithType:GITObjectTypeTag sha1:[GITObjectHash objectHashWithData:[data sha1Digest]] repo:theRepo] )
+- (id)initFromData: (NSData *)data sha1: (GITObjectHash *)objectHash repo: (GITRepo *)theRepo error: (NSError **)error {
+    if ( ![super initWithType:GITObjectTypeTag sha1:objectHash repo:theRepo] )
         return nil;
 
     GITObjectHash *hashVal;

diff --git a/Test/Unit/GITBlob.rb b/Test/Unit/GITBlob.rb
@@ -10,7 +10,7 @@
     @err = Pointer.new(:object)
     @repo = default_repository
     @data = BLOB_CONTENTS.dataUsingEncoding(NSUTF8StringEncoding)
-    @blob = GITBlob.blobFromData(@data, repo:@repo, error:@err)
+    @blob = GITBlob.blobFromData(@data, sha1:GITObjectHash.objectHashWithString(@sha), repo:@repo, error:@err)
   end
   should 'not be nil' do
     @blob.should.not.be.nil

diff --git a/Test/Unit/GITCommit.rb b/Test/Unit/GITCommit.rb
@@ -2,9 +2,9 @@
   before do
     @err = Pointer.new(:object)
     @repo = default_repository
-    @commitData = NSData.dataWithContentsOfFile("#{TEST_REPO}/.git/objects/fb/473b1d24812eb1b212d444b7be8e651c6077ca").zlibInflate
+    @commitData = NSData.dataWithContentsOfFile("#{@repo.root}/objects/fb/473b1d24812eb1b212d444b7be8e651c6077ca").zlibInflate
     @data = @commitData.subdataWithRange(NSMakeRange(11, 248))    # This is specific to this object
-    @commit = GITCommit.commitFromData(@data, repo:@repo, error:@err)
+    @commit = GITCommit.commitFromData(@data, sha1:GITObjectHash.objectHashWithString("fb473b1d24812eb1b212d444b7be8e651c6077ca"), repo:@repo, error:@err)
     @date = GITDateTime.dateTimeWithTimestamp(1263165097, timeZoneOffset:"+0000")
   end
   should 'not be nil' do

diff --git a/Test/Unit/GITRepo.rb b/Test/Unit/GITRepo.rb
@@ -107,3 +107,22 @@
     @branches.should.include('another')
   end
 end
+
+describe "GITRepo -objectWithSha1:error:" do
+  before do
+    @err = Pointer.new(:object)
+    @repo = default_repository
+    @sha1 = GITObjectHash.objectHashWithString("fb473b1d24812eb1b212d444b7be8e651c6077ca")
+    @commit = @repo.objectWithSha1(@sha1, error:@err)
+  end
+
+  should 'not be nil' do
+    @commit.should.not.be.nil
+  end
+  should 'not have an error' do
+    @err[0].should.be.nil
+  end
+  should 'have sha1' do
+    @commit.sha1.unpackedString.should == @sha1.unpackedString
+  end
+end
diff --git a/Test/Unit/GITTag.rb b/Test/Unit/GITTag.rb
@@ -4,7 +4,7 @@
     @repo = default_repository
     @tagData = NSData.dataWithContentsOfFile("#{TEST_REPO}/.git/objects/e5/0d1a7b4659ad79d146cb75177ba65b045381dd").zlibInflate
     @data = @tagData.subdataWithRange(NSMakeRange(8, 144))    # This is specific to this object
-    @tag = GITTag.tagFromData(@data, repo:@repo, error:@err)
+    @tag = GITTag.tagFromData(@data, sha1:GITObjectHash.objectHashWithString("e50d1a7b4659ad79d146cb75177ba65b045381dd"), repo:@repo, error:@err)
     @date = GITDateTime.dateTimeWithTimestamp(1254663297, timeZoneOffset:"+0100")
   end
   should 'not be nil' do

diff --git a/Test/Unit/GITTree.rb b/Test/Unit/GITTree.rb
@@ -4,7 +4,7 @@
     @repo = default_repository
     @treeData = NSData.dataWithContentsOfFile("#{TEST_REPO}/.git/objects/22/7c6c88ba35e67a1341a068c07d1c1639d6582e").zlibInflate
     @data = @treeData.subdataWithRange(NSMakeRange(8, 78))  # This is specific to this object
-    @tree = GITTree.treeFromData(@data, repo:@repo, error:@err)
+    @tree = GITTree.treeFromData(@data, sha1:GITObjectHash.objectHashWithString("227c6c88ba35e67a1341a068c07d1c1639d6582e"), repo:@repo, error:@err)
   end
 
   should 'not be nil' do