Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add context.Context Support to Additional Middlewares #3212

Open
3 tasks
coderabbitai bot opened this issue Nov 21, 2024 · 3 comments
Open
3 tasks

Add context.Context Support to Additional Middlewares #3212

coderabbitai bot opened this issue Nov 21, 2024 · 3 comments

Comments

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 21, 2024

Based on the discussion in Issue #3175, we need to extend context support to other middlewares for consistency.

Middlewares to update:

  • keyauth
  • csrf
  • session

This will ensure that relevant context can be passed through the context.Context in these middlewares.

Requester: @ReneWerner87

@ReneWerner87
Copy link
Member

code from the request id middleware adjustement
https://github.com/gofiber/fiber/pull/3200/files

@gofiber gofiber deleted a comment from coderabbitai bot Nov 21, 2024
@JIeJaitt
Copy link
Contributor

JIeJaitt commented Nov 21, 2024

@ReneWerner87 We need to carefully discuss which middleware need to add requestID, because from the routing to the control layer to the service layer in this process, I see that most of the Go developers will choose to control layer Fiber's Ctx into UserContext, because Fiber as a web framework its function in the whole architecture in the control layer has ended, the service layer will be conducted specific business processing, in the service layer business more concurrent security context, and fiber context is not concurrent security, and requestId in the microservice link tracking is very important to locate the problem, so here will be proposed to increase the request mw UserContext requestID.

The following is the GPT's answer for reference.
Fiber's context (ctx) is not inherently thread-safe. Each context instance is tied to a specific request and should only be used within that request's handler goroutine. If you need to access ctx data from different goroutines, you should.

  1. Pass specific data you need instead of the entire ctx
  2. Use proper synchronization mechanisms (mutexes, channels) if sharing data
  3. Clone required data from ctx before passing to goroutines

Example of what to avoid.

app.Get(“/”, func(c *fiber.Ctx) error {
    go func() {
        // UNSAFE: accessing ctx from different goroutine
        data := c.Query(“someData”)
    }()
    return nil
})

Safe approach.

app.Get(“/”, func(c *fiber.Ctx) error {
    data := c.Query(“someData”) // Get data first
    go func(safeData string) {
        // SAFE: using copied data
        process(safeData)
    }(data)
    return nil
})

@gaby
Copy link
Member

gaby commented Nov 21, 2024

@JIeJaitt This is for using context.Context, not fasthttp.RequestCtx.

@ReneWerner87 ReneWerner87 changed the title Add Context Support to Additional Middlewares Add Context.Context Support to Additional Middlewares Nov 21, 2024
@ReneWerner87 ReneWerner87 changed the title Add Context.Context Support to Additional Middlewares Add context.Context Support to Additional Middlewares Nov 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

No branches or pull requests

3 participants