akupila/recorder

crash with mode Auto if replay file exists and is empty

Closed this issue · 0 comments

Hello @akupila, thanks for recorder!

Consider the following:

func TestNewWithExistingEmptyFileDoesntCrashGHXXX(t *testing.T) {
	const emptyFile = "testdata/empty.yml"
	if err := os.MkdirAll(path.Dir(emptyFile), 0755); err != nil {
		t.Fatal("mkdir:", err)
	}
	if f, err := os.OpenFile(emptyFile, os.O_CREATE|os.O_TRUNC, 0644); err != nil {
		t.Fatal("creating the empty file:", err)
	} else {
		f.Close()
	}

	rec := recorder.New(emptyFile)
	client := &http.Client{Transport: rec}

	_, err := client.Get("https://jsonplaceholder.typicode.com/posts/1")
	if err != nil {
		t.Fatal("get:", err)
	}
}

This will crash in recorder.Lookup():

=== RUN   TestNewWithExistingEmptyFileDoesntCrashGHXXX
--- FAIL: TestNewWithExistingEmptyFileDoesntCrashGHXXX (0.00s)
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
        panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x135c72c]

goroutine 116 [running]:
testing.tRunner.func1.2(0x13d4500, 0x16b8b90)
        /usr/local/go/src/testing/testing.go:1143 +0x332
testing.tRunner.func1(0xc000183080)
        /usr/local/go/src/testing/testing.go:1146 +0x4b6
panic(0x13d4500, 0x16b8b90)
        /usr/local/go/src/runtime/panic.go:965 +0x1b9
github.com/akupila/recorder.(*Recorder).Lookup(0xc0001e6900, 0x1437f02, 0x3, 0xc0001d8240, 0x2c, 0x0, 0x0, 0x0)
        /Users/mmolteni/src/recorder/recorder.go:275 +0x8c
github.com/akupila/recorder.(*Recorder).RoundTrip(0xc0001e6900, 0xc00032a200, 0xc0001e6900, 0x0, 0x0)
        /Users/mmolteni/src/recorder/recorder.go:148 +0x13a5
net/http.send(0xc00032a200, 0x14b2a40, 0xc0001e6900, 0x0, 0x0, 0x0, 0xc0001d6178, 0x203000, 0x1, 0x0)
        /usr/local/go/src/net/http/client.go:251 +0x454
net/http.(*Client).send(0xc000320420, 0xc00032a200, 0x0, 0x0, 0x0, 0xc0001d6178, 0x0, 0x1, 0xc00032a200)
        /usr/local/go/src/net/http/client.go:175 +0xff
net/http.(*Client).do(0xc000320420, 0xc00032a200, 0x0, 0x0, 0x0)
        /usr/local/go/src/net/http/client.go:717 +0x45f
net/http.(*Client).Do(...)
        /usr/local/go/src/net/http/client.go:585
net/http.(*Client).Get(0xc000320420, 0x1448fd9, 0x2c, 0x1a4, 0xc0001d6168, 0x0)
        /usr/local/go/src/net/http/client.go:474 +0xbe
github.com/akupila/recorder_test.TestNewWithExistingEmptyFileDoesntCrashGHXXX(0xc000183080)
        /Users/mmolteni/src/recorder/recorder_test.go:589 +0x233

The reason is that in loadFromDisk, the YAML parser will happily parse an empty []byte and give back an

type Entry struct {
	Request  *Request  `yaml:"request"`
	Response *Response `yaml:"response"`
}

where the fields are nil. Then RoundTrip calls Lookup, and:

	for _, e := range r.entries {
		if strings.EqualFold(e.Request.Method, method) && strings.EqualFold(e.Request.URL, url) {
			return e, true
		}
	}

crashes dereferencing e.Request