Remote.Push pushes corrupted objects to the server

[erizocosmico]

Related to src-d/borges#264

Remote.Push pushes some corrupted objects to the server, so the repo becomes unreadable. This happens at least with https://github.com/zfcampus/zf-oauth2. We're aware of a lot more repos with this issue, but this was the first I could get my hands on.

The error:

[2858] error: delta replay has gone wild
[2858] fatal: pack has bad object at offset 470810: failed to apply delta

Reproduction

Setup the server:

mkdir /tmp/server-repo
cd /tmp/server-repo
git --bare init --shared
git daemon --verbose --base-path=/tmp --export-all --reuseaddr --enable=receive-pack

Push with go-git:

package main

import (
	"log"

	git "gopkg.in/src-d/go-git.v4"
	"gopkg.in/src-d/go-git.v4/config"
)

func main() {
	r, err := git.PlainClone("/tmp/client-repo", false, &git.CloneOptions{
		URL: "https://github.com/zfcampus/zf-oauth2",
	})

	assert(err)

	err = r.Fetch(&git.FetchOptions{
		RefSpecs: []config.RefSpec{
			"refs/*:refs/*",
			"HEAD:refs/heads/HEAD",
		},
	})

	assert(err)

	remote, err := r.CreateRemote(&config.RemoteConfig{
		Name: "external",
		URLs: []string{"git://localhost/server-repo"},
	})

	assert(err)

	err = remote.Push(&git.PushOptions{
		RemoteName: "external",
		RefSpecs: []config.RefSpec{
			"refs/*:refs/*",
			"HEAD:refs/heads/HEAD",
		},
	})
	assert(err)
}

func assert(err error) {
	if err != nil {
		log.Fatal(err)
	}
}

Doing the same operation with git push does not result in any corrupted object.

I've checked and the repo is not corrupted neither before pushing nor afterwards, the problem only occurs pushing.

I'm currently trying to trace where the issue is.

UPDATE: setting config.Pack.Window to 0 makes the issue go away, but that turns off delta compression, though.

[erizocosmico]

On the given repo, this just happens when object 72bf04bda1466acfd65bf09143f0372457f95136 is deltified with 8f751eef719ef101181bfd18aa8333546a917d19 as its base.

It can either be a problem in the packfile encoder, which is not likely, or in the delta selection.

[ajnavarro]

I was able to reproduce the error using that two files and creating a packfile just with them:

package main

import (
	"log"

	"os"

	git "gopkg.in/src-d/go-git.v4"
	"gopkg.in/src-d/go-git.v4/config"
	"gopkg.in/src-d/go-git.v4/plumbing"
	"gopkg.in/src-d/go-git.v4/plumbing/format/packfile"
	"gopkg.in/src-d/go-git.v4/storage/memory"
)

func main() {
	//r, err := git.PlainClone("/tmp/client-repo", false, &git.CloneOptions{
	//	URL: "https://github.com/zfcampus/zf-oauth2",
	//})

	r, err := git.PlainOpen("/tmp/client-repo")
	assert(err)

	err = r.Fetch(&git.FetchOptions{
		RefSpecs: []config.RefSpec{
			"refs/*:refs/*",
			"HEAD:refs/heads/HEAD",
		},
	})
	h1 := plumbing.NewHash("72bf04bda1466acfd65bf09143f0372457f95136")
	obj1, err := r.Storer.EncodedObject(plumbing.AnyObject, h1)
	assert(err)

	h2 := plumbing.NewHash("8f751eef719ef101181bfd18aa8333546a917d19")
	obj2, err := r.Storer.EncodedObject(plumbing.AnyObject, h2)
	assert(err)

	// Delta generated by the encoder will be this one
	delta, err := packfile.GetDelta(obj2, obj1)
	println(delta.Size())

	mem := memory.NewStorage()
	_, err = mem.ObjectStorage.SetEncodedObject(obj2)
	assert(err)
	_, err = mem.ObjectStorage.SetEncodedObject(obj1)
	assert(err)

	f, err := os.Create("packfile.pack")
	assert(err)

	enc := packfile.NewEncoder(f, mem, false)

	_, err = enc.Encode([]plumbing.Hash{h1, h2}, 2)
	assert(err)
}

func assert(err error) {
	if err != nil {
		log.Fatal(err)
	}
}

After that, we have a packfile.pack file with two objects. Trying to create the index:

17:47 $ git index-pack packfile.pack
error: delta replay has gone wild
fatal: pack has bad object at offset 1884: failed to apply delta

Changing the window from 2 to 0 (so no delta is generated):

17:48 $ git index-pack packfile.pack
8d1e8b9058eb4b716778d379de039d5df87a9268

I think the problem is on diff_delta logic.

[kuba--]

@ajnavarro - following fix in delta_diff.go:

			for rl > 0 {
				if rl < maxCopySize {
					buf.Write(encodeCopyOperation(aOffset, rl))
					break
				}

				buf.Write(encodeCopyOperation(aOffset, maxCopySize))
				rl -= maxCopySize
				aOffset += maxCopySize
			}

[kuba--]

PR. with fix: #856