Why no call to Request.MultipartForm.RemoveAll

[zihuyishi]

I found if upload large file, a file in /tmp will keep forevery until next booting.
But go net/server package will call RemoveAll when request finished(https://github.com/golang/go/blob/b5d555991ab73e06e09741952a66dd7eeaf2a185/src/net/http/server.go#L1700)

	if w.req.MultipartForm != nil {
		w.req.MultipartForm.RemoveAll()
	}

My request and response seem work, but /tmp/multipart-xxxx file not delete by net/server. If i mannual call RemoveAll, it work ok.

[fengqi]

You need to check your middleware to see if they are overriding the  ctx.Request

[zihuyishi]

You need to check your middleware to see if they are overriding the  ctx.Request

I add middleware to add opentelemery. It override c.Request = c.Request.WithContext(ctx).
So I must call w.req.MultipartForm.RemoveAll() after c.Next()?

[FarmerChillax]

I add middleware to add opentelemery.

Why choose to write your own middleware instead of using an existing lib?

[FarmerChillax]

Can you provide a minimal reproducible case?

[zihuyishi]

I add middleware to add opentelemery.

Why choose to write your own middleware instead of using an existing lib?

I write myself middleware like this:

func OpenTelemetry() gin.HandlerFunc {
	return func(c *gin.Context) {
		if c.Request.URL.Path == "/health" {
			c.Next()
			return
		}
		method := c.Request.Method
		router := c.FullPath()
		spanName := fmt.Sprintf("%s %s", method, router)
		ctx := c.Request.Context()
		ctx = otel.GetTextMapPropagator().Extract(ctx, propagation.HeaderCarrier(c.Request.Header))
		ctx, span := traceutil.GlobalTracer().Start(ctx, spanName,
			trace.WithSpanKind(trace.SpanKindServer),
			trace.WithAttributes(attribute.String("http.method", method), attribute.String("http.path", c.Request.URL.Path), attribute.String("url.scheme", "https")))
		defer span.End()
		c.Request = c.Request.WithContext(ctx)
		otel.GetTextMapPropagator().Inject(ctx, propagation.HeaderCarrier(c.Writer.Header()))
		c.Next()
		if c.Writer.Status() >= 500 {
			span.SetStatus(codes.Error, "server error")
		}
	}
}

now I will add defer c.MultipartForm.RemoveAll() to resolve.

[zihuyishi]

Sorry, after review my code, I think change code c.Request = c.Request.WithContext(ctx) to c.Request = c.Request.Clone(ctx) will solve it.

[FarmerChillax]

I add middleware to add opentelemery.

Why choose to write your own middleware instead of using an existing lib?

I write myself middleware like this:

func OpenTelemetry() gin.HandlerFunc {
return func(c *gin.Context) {
if c.Request.URL.Path == "/health" {
c.Next()
return
}
method := c.Request.Method
router := c.FullPath()
spanName := fmt.Sprintf("%s %s", method, router)
ctx := c.Request.Context()
ctx = otel.GetTextMapPropagator().Extract(ctx, propagation.HeaderCarrier(c.Request.Header))
ctx, span := traceutil.GlobalTracer().Start(ctx, spanName,
trace.WithSpanKind(trace.SpanKindServer),
trace.WithAttributes(attribute.String("http.method", method), attribute.String("http.path", c.Request.URL.Path), attribute.String("url.scheme", "https")))
defer span.End()
c.Request = c.Request.WithContext(ctx)
otel.GetTextMapPropagator().Inject(ctx, propagation.HeaderCarrier(c.Writer.Header()))
c.Next()
if c.Writer.Status() >= 500 {
span.SetStatus(codes.Error, "server error")
}
}
}
now I will add defer c.MultipartForm.RemoveAll() to resolve.

why not use the otelgin lib?
example: https://github.com/gin-gonic/examples/tree/master/otel

[zihuyishi]

I add middleware to add opentelemery.

Why choose to write your own middleware instead of using an existing lib?

I write myself middleware like this:
func OpenTelemetry() gin.HandlerFunc {
return func(c *gin.Context) {
if c.Request.URL.Path == "/health" {
c.Next()
return
}
method := c.Request.Method
router := c.FullPath()
spanName := fmt.Sprintf("%s %s", method, router)
ctx := c.Request.Context()
ctx = otel.GetTextMapPropagator().Extract(ctx, propagation.HeaderCarrier(c.Request.Header))
ctx, span := traceutil.GlobalTracer().Start(ctx, spanName,
trace.WithSpanKind(trace.SpanKindServer),
trace.WithAttributes(attribute.String("http.method", method), attribute.String("http.path", c.Request.URL.Path), attribute.String("url.scheme", "https")))
defer span.End()
c.Request = c.Request.WithContext(ctx)
otel.GetTextMapPropagator().Inject(ctx, propagation.HeaderCarrier(c.Writer.Header()))
c.Next()
if c.Writer.Status() >= 500 {
span.SetStatus(codes.Error, "server error")
}
}
}
now I will add defer c.MultipartForm.RemoveAll() to resolve.

why not use the otelgin lib? example: https://github.com/gin-gonic/examples/tree/master/otel

I don't known this repo before.
But i found it also use c.Request = c.Request.WithContext(ctx), so it's work ok in this middleware?
code here: https://github.com/open-telemetry/opentelemetry-go-contrib/blob/ad44bed7174e6a8380780e861aec1dc8520d1ac0/instrumentation/github.com/gin-gonic/gin/otelgin/gin.go#L109C3-L110C1

[zihuyishi]

after test, c.Request.Clone not work. Because net/server hold the original Request object, but in handler use new Request object to parse multipartForm, So I must call Request.MultipartForm.RemoveAll() on new Request object.

[FarmerChillax]

I add middleware to add opentelemery.

Why choose to write your own middleware instead of using an existing lib?

I write myself middleware like this:
func OpenTelemetry() gin.HandlerFunc {
return func(c *gin.Context) {
if c.Request.URL.Path == "/health" {
c.Next()
return
}
method := c.Request.Method
router := c.FullPath()
spanName := fmt.Sprintf("%s %s", method, router)
ctx := c.Request.Context()
ctx = otel.GetTextMapPropagator().Extract(ctx, propagation.HeaderCarrier(c.Request.Header))
ctx, span := traceutil.GlobalTracer().Start(ctx, spanName,
trace.WithSpanKind(trace.SpanKindServer),
trace.WithAttributes(attribute.String("http.method", method), attribute.String("http.path", c.Request.URL.Path), attribute.String("url.scheme", "https")))
defer span.End()
c.Request = c.Request.WithContext(ctx)
otel.GetTextMapPropagator().Inject(ctx, propagation.HeaderCarrier(c.Writer.Header()))
c.Next()
if c.Writer.Status() >= 500 {
span.SetStatus(codes.Error, "server error")
}
}
}
now I will add defer c.MultipartForm.RemoveAll() to resolve.

why not use the otelgin lib? example: https://github.com/gin-gonic/examples/tree/master/otel

I don't known this repo before. But i found it also use c.Request = c.Request.WithContext(ctx), so it's work ok in this middleware? code here: https://github.com/open-telemetry/opentelemetry-go-contrib/blob/ad44bed7174e6a8380780e861aec1dc8520d1ac0/instrumentation/github.com/gin-gonic/gin/otelgin/gin.go#L109C3-L110C1

I just have doubts about not using lib, If there is a problem with the library, it should be solved upstream

[zihuyishi]

I add middleware to add opentelemery.

Why choose to write your own middleware instead of using an existing lib?

I write myself middleware like this:
func OpenTelemetry() gin.HandlerFunc {
return func(c *gin.Context) {
if c.Request.URL.Path == "/health" {
c.Next()
return
}
method := c.Request.Method
router := c.FullPath()
spanName := fmt.Sprintf("%s %s", method, router)
ctx := c.Request.Context()
ctx = otel.GetTextMapPropagator().Extract(ctx, propagation.HeaderCarrier(c.Request.Header))
ctx, span := traceutil.GlobalTracer().Start(ctx, spanName,
trace.WithSpanKind(trace.SpanKindServer),
trace.WithAttributes(attribute.String("http.method", method), attribute.String("http.path", c.Request.URL.Path), attribute.String("url.scheme", "https")))
defer span.End()
c.Request = c.Request.WithContext(ctx)
otel.GetTextMapPropagator().Inject(ctx, propagation.HeaderCarrier(c.Writer.Header()))
c.Next()
if c.Writer.Status() >= 500 {
span.SetStatus(codes.Error, "server error")
}
}
}
now I will add defer c.MultipartForm.RemoveAll() to resolve.

why not use the otelgin lib? example: https://github.com/gin-gonic/examples/tree/master/otel

I don't known this repo before. But i found it also use c.Request = c.Request.WithContext(ctx), so it's work ok in this middleware? code here: https://github.com/open-telemetry/opentelemetry-go-contrib/blob/ad44bed7174e6a8380780e861aec1dc8520d1ac0/instrumentation/github.com/gin-gonic/gin/otelgin/gin.go#L109C3-L110C1

I just have doubts about not using lib, If there is a problem with the library, it should be solved upstream

I think the issue lies in the fact that c.Request can be overridden. Once it's overridden and a new Request object is used to call ParseMultipartForm, currently no one takes responsibility for the new Request object's MultipartForm, resulting in RemoveAll not being called. Caller must handle itself

[zihuyishi]

Demo code here:

package main

import (
	"context"
	"net/http"

	"github.com/gin-gonic/gin"
)

func main() {
	router := gin.Default()
	router.Use(func(c *gin.Context) {
		ctx := c.Request.Context()
		ctx = context.WithValue(ctx, "something", 1)
		c.Request = c.Request.WithContext(ctx)
		c.Next()
	})
	router.POST("/upload", func(c *gin.Context) {
		file, err := c.FormFile("file")
		if err != nil {
			c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()})
			return
		}
		c.SaveUploadedFile(file, "test.txt")
		c.JSON(http.StatusOK, gin.H{"message": "File uploaded successfully"})
	})
	router.Run()
}

then just upload file bigger than 32mb, you will see multipart-xxxxxx in /tmp folder.

curl --location 'http://localhost:8080/upload' \
--form 'file=@"/path/to/file"'