v3.26.5 seems to have broken middleware

[nomad-software]

I have middleware defined like this:

func LogRequest(c *req.Client, r *req.Request) error {
	c.GetLogger().Debugf("%s request, path: %s, retry attempt: %d",
		r.Method,
		r.URL.RequestURI(),
		r.RetryAttempt,
	)
	return nil
}

and then used like this:

	req := req.C().
		SetLogger(c.Log).
		SetTimeout(c.Timeout).
		SetCommonRetryCount(c.RetryCount).
		SetCommonRetryBackoffInterval(1*time.Second, 3*time.Second).
		OnBeforeRequest(log.LogRequest).
		OnAfterResponse(log.LogResponse)

When I request something I get:

2023/01/03 16:25:13 http2: panic serving 127.0.0.1:33124: runtime error: invalid memory address or nil pointer dereference
goroutine 14 [running]:
golang.org/x/net/http2.(*serverConn).runHandler.func1()
	/home/gary/Projects/em-ohip-reservation/vendor/golang.org/x/net/http2/server.go:2298 +0x145
panic({0xbb0360, 0x15aefd0})
	/opt/go/src/runtime/panic.go:884 +0x212
net/url.(*URL).RequestURI(0x0)
	/opt/go/src/net/url/url.go:1122 +0x1d
github.com/company/go-ohip-client/v2/internal/log.LogRequest(0x45507c?, 0xc0001121e0)
	/home/gary/Projects/em-ohip-reservation/vendor/github.com/company/go-ohip-client/v2/internal/log/middleware.go:9 +0x156
github.com/imroc/req/v3.(*Request).do(0xc0001121e0)
	/home/gary/Projects/em-ohip-reservation/vendor/github.com/imroc/req/v3/request.go:533 +0x36b
github.com/imroc/req/v3.(*Request).Do(0xc0001121e0, {0x0?, 0x10?, 0xdca54a?})
	/home/gary/Projects/em-ohip-reservation/vendor/github.com/imroc/req/v3/request.go:517 +0x17f
github.com/imroc/req/v3.(*Request).Send(...)
	/home/gary/Projects/em-ohip-reservation/vendor/github.com/imroc/req/v3/request.go:596
github.com/imroc/req/v3.(*Request).Post(0xc000114990?, {0xc000526180?, 0xdc52ac?})
	/home/gary/Projects/em-ohip-reservation/vendor/github.com/imroc/req/v3/request.go:627 +0x86
snip..

It seems to be something that changed here: 2522eb2

Reverting back to v3.26.4 fixes the issue.

Any ideas how to get around this? Thanks.

[imroc]

In order to support dynamic modification of the request in the request middleware, the user-defined request middleware will be executed first, that is, r.URL is still nil at this time, and r.URL.RequestURI() will panic due to the nil pointer, references to auto-generated fields in request middleware should be avoided.

Two solutions.

1. Use client middleware:

package main

import (
	"github.com/imroc/req/v3"
)

func LogRequest(rt req.RoundTripper) req.RoundTripFunc {
	return func(r *req.Request) (resp *req.Response, err error) {
		r.GetClient().GetLogger().Debugf("%s request, path: %s, retry attempt: %d",
			r.Method,
			r.URL.RequestURI(),
			r.RetryAttempt,
		)
		return rt.RoundTrip(r)
	}
}

func main() {
	c := req.C().EnableDebugLog().WrapRoundTripFunc(LogRequest)
	c.R().MustGet("https://httpbin.org/get")
}

2. Use request middleware, not using r.URL:

package main

import (
	"github.com/imroc/req/v3"
	"net/url"
)

func LogRequest(c *req.Client, r *req.Request) error {
	u, err := url.Parse(r.RawURL)
	if err != nil {
		return err
	}
	c.GetLogger().Debugf("%s request, path: %s, retry attempt: %d",
		r.Method,
		u.RequestURI(),
		r.RetryAttempt,
	)
	return nil
}

func main() {
	c := req.C().EnableDebugLog().OnBeforeRequest(LogRequest)
	c.R().MustGet("https://httpbin.org/get")
}

[nomad-software]

Thanks for the workarounds.

This is a massive breaking change and perhaps should have been more than a patch version bump.

[imroc]

You're right, but the version has been released, I will pay attention next time, thanks