Merge pull request #731 from jfontan/fix/crash-with-delta-cycles

plumbing: format/packfile, fix crash with cycle deltas

Author: mcuadros

plumbing/format/packfile/delta_selector.go

@@ -196,7 +196,8 @@ func (dw *deltaSelector) restoreOriginal(otp *ObjectToPack) error {
 		return err
 	}
 
-	otp.Original = obj
+	otp.SetOriginal(obj)
+
 	return nil
 }
 

plumbing/format/packfile/encoder.go

@@ -87,6 +87,7 @@ func (e *Encoder) entry(o *ObjectToPack) error {
 		// (for example due to a concurrent repack) and a different base
 		// was chosen, forcing a cycle. Select something other than a
 		// delta, and write this object.
+		e.selector.restoreOriginal(o)
 		o.BackToOriginal()
 	}
 

plumbing/format/packfile/encoder_test.go

@@ -202,6 +202,15 @@ func (s *EncoderSuite) deltaOverDeltaCyclicTest(c *C) {
 	o3 := newObject(plumbing.BlobObject, []byte("011111"))
 	o4 := newObject(plumbing.BlobObject, []byte("01111100000"))
 
+	_, err := s.store.SetEncodedObject(o1)
+	c.Assert(err, IsNil)
+	_, err = s.store.SetEncodedObject(o2)
+	c.Assert(err, IsNil)
+	_, err = s.store.SetEncodedObject(o3)
+	c.Assert(err, IsNil)
+	_, err = s.store.SetEncodedObject(o4)
+	c.Assert(err, IsNil)
+
 	d2, err := GetDelta(o1, o2)
 	c.Assert(err, IsNil)
 
@@ -219,6 +228,18 @@ func (s *EncoderSuite) deltaOverDeltaCyclicTest(c *C) {
 	pd3.SetDelta(pd4, d3)
 	pd4.SetDelta(pd3, d4)
 
+	// SetOriginal is used by delta selector when generating ObjectToPack.
+	// It also fills type, hash and size values to be used when Original
+	// is nil.
+	po1.SetOriginal(po1.Original)
+	pd2.SetOriginal(pd2.Original)
+	pd2.SetOriginal(nil)
+
+	pd3.SetOriginal(pd3.Original)
+	pd3.SetOriginal(nil)
+
+	pd4.SetOriginal(pd4.Original)
+
 	encHash, err := s.enc.encode([]*ObjectToPack{
 		po1,
 		pd2,

plumbing/format/packfile/object_pack.go

@@ -23,6 +23,12 @@ type ObjectToPack struct {
 	// offset in pack when object has been already written, or 0 if it
 	// has not been written yet
 	Offset int64
+
+	// Information from the original object
+	resolvedOriginal bool
+	originalType     plumbing.ObjectType
+	originalSize     int64
+	originalHash     plumbing.Hash
 }
 
 // newObjectToPack creates a correct ObjectToPack based on a non-delta object
@@ -47,7 +53,7 @@ func newDeltaObjectToPack(base *ObjectToPack, original, delta plumbing.EncodedOb
 
 // BackToOriginal converts that ObjectToPack to a non-deltified object if it was one
 func (o *ObjectToPack) BackToOriginal() {
-	if o.IsDelta() {
+	if o.IsDelta() && o.Original != nil {
 		o.Object = o.Original
 		o.Base = nil
 		o.Depth = 0
@@ -71,11 +77,28 @@ func (o *ObjectToPack) WantWrite() bool {
 	return o.Offset == 1
 }
 
+// SetOriginal sets both Original and saves size, type and hash. If object
+// is nil Original is set but previous resolved values are kept
+func (o *ObjectToPack) SetOriginal(obj plumbing.EncodedObject) {
+	o.Original = obj
+
+	if obj != nil {
+		o.originalSize = obj.Size()
+		o.originalType = obj.Type()
+		o.originalHash = obj.Hash()
+		o.resolvedOriginal = true
+	}
+}
+
 func (o *ObjectToPack) Type() plumbing.ObjectType {
 	if o.Original != nil {
 		return o.Original.Type()
 	}
 
+	if o.resolvedOriginal {
+		return o.originalType
+	}
+
 	if o.Base != nil {
 		return o.Base.Type()
 	}
@@ -92,6 +115,10 @@ func (o *ObjectToPack) Hash() plumbing.Hash {
 		return o.Original.Hash()
 	}
 
+	if o.resolvedOriginal {
+		return o.originalHash
+	}
+
 	do, ok := o.Object.(plumbing.DeltaObject)
 	if ok {
 		return do.ActualHash()
@@ -105,6 +132,10 @@ func (o *ObjectToPack) Size() int64 {
 		return o.Original.Size()
 	}
 
+	if o.resolvedOriginal {
+		return o.originalSize
+	}
+
 	do, ok := o.Object.(plumbing.DeltaObject)
 	if ok {
 		return do.ActualSize()