Gin Why no call to Request.MultipartForm.RemoveAll

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.

Comment From: fengqi

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

Comment From: 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()?

Comment From: FarmerChillax

I add middleware to add opentelemery.

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

Comment From: FarmerChillax

Can you provide a minimal reproducible case?

Comment From: 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.

Comment From: 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.

Comment From: 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

Comment From: 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

Comment From: 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.

Comment From: 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

Comment From: 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

Comment From: 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"'