Make eagerRequestDataCache middleware optional for custom routes

[novuscy]

Hello, thanks for making this amazing project!

We use PocketBase as a framework and register a reverse proxy using OnBeforeServe hook with routes start with "/proxy/*", it works very well before v0.15.0(when eagerRequestDataCache was added), after upgrading to v0.15.0 and later versions, the reverse proxy reports 502 error with "http: proxy error: http: ContentLength=83 with Body length 0" when proxied endpoints accept POST method and form-data, other endpoints which accept GET or POST with JSON data work as usual.

It seems that the eagerRequestDataCache consumes the form-data request body, when I delete the bindFormData() function call in rest.BindBody(), the form-data endpoints report no error.

Is there a way to remove eagerRequestDataCache just for "/proxy/*" routes?

Thanks.

[ganigeorgiev]

The body of a JSON request is explicitly copied but for the multipart/form-data requests I don't think this is necessary.

Could you provide a code sample showing how do you define the proxy route and where it is failing?

[novuscy]

func EdgeProxy(e *core.ServeEvent) error {
	g := e.Router.Group("/proxy")

	g.Any("/:device/*", func(c echo.Context) error {
		return c.String(http.StatusOK, "test")
	}, EdgeProxyMiddleware())

	return nil
}

func EdgeProxyMiddleware() echo.MiddlewareFunc {
	return func(next echo.HandlerFunc) echo.HandlerFunc {
		return func(c echo.Context) error {
			device := c.PathParam("device")
			id := strings.Split(device, "_")[0]
                        // get target device virtual IP from database
			ip, err := getVirtualIP(id)
			if err != nil {
				return err
			}
			target := fmt.Sprintf("http://%s", ip)
			u, err := url.Parse(target)
			if err != nil {
				return err
			}

			r := c.Request()
			r.URL.Host = u.Host
			r.URL.Scheme = u.Scheme
			r.Host = u.Host

			splat := c.PathParam("*")
			path := fmt.Sprintf("/%s", splat)
			r.URL.Path = path

			proxy := httputil.NewSingleHostReverseProxy(u)
			proxy.ServeHTTP(c.Response(), r)

			return nil
		}
	}
}

The code is something like this, I made some changes in the tools/rest/multi_binder.go:

func BindBody(c echo.Context, i interface{}) error {
	req := c.Request()
	if req.ContentLength == 0 {
		return nil
	}

	ctype := req.Header.Get(echo.HeaderContentType)
	switch {
	case strings.HasPrefix(ctype, echo.MIMEApplicationJSON):
		err := CopyJsonBody(c.Request(), i)
		if err != nil {
			return echo.NewHTTPErrorWithInternal(http.StatusBadRequest, err, err.Error())
		}
		return nil
	case strings.HasPrefix(ctype, echo.MIMEApplicationForm), strings.HasPrefix(ctype, echo.MIMEMultipartForm):
                // change here:
		// return bindFormData(c, i)
		return nil
	}

	// fallback to the default binder
	return echo.BindBody(c, i)
}

and the 502 error is gone.

[ganigeorgiev]

Usually copying the body reader for multipart/form-data requests shouldn't be necessary because when calling c.FormValues() (or Request.ParseMultipartForm()) the parsed body state is stored in the request.Form/request.MultipartForm fields and any consequent calls will just return the values from there, but in your case the proxied request fails I guess due to some internal validation where the already read r.Body stream and the the r.ContentLength mismatches.

I'll consider a little bit later today to register the eagerRequestDataCache middleware only for the builtin api group routes since it is not universal anyway (for example we are not handling XML content-type requests) and in cases like yours it may produce unexpected results.

[ganigeorgiev]

Should be resolved in v0.16.9 release.