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

How to add a value into the request context from a middleware function? #1180

Closed
Julio-Guerra opened this issue Feb 1, 2019 · 4 comments
Closed
Labels
good first issue A user wrote a good first issue with clear instructions 🚀 status:implemented 🪶 type: feature
Milestone

Comments

@Julio-Guerra
Copy link

Hi!

I am implementing a middleware function and I would like to store a value into the request context so that handlers can access it from the Iris context but also from the request context, in case they only pass the request to sub-functions in their handlers.

Storing a value into the Iris context from a middleware function is indeed easy (c.Values().Set(key, val)), but what about the request's:

req := c.Request()
ctx := context.WithValue(req.Context(), key, val)
req = req.WithContext(req)

But from there, I couldn't find any way to store the new request created by WithContext() into Iris' context.

I thought FromStdWithNext() would help me on this but the request pointer passed to next() is ignored (https://github.com/kataras/iris/blob/master/core/handlerconv/from_std.go#L75).

Thanks for your help!

@kataras
Copy link
Owner

kataras commented Feb 2, 2019

Hello @Julio-Guerra !

I see, so you need a way to override the ctx.request field which is unexported, you have read-only access to the http.Request via ctx.Request(). We have a ctx.ResetResponseWriter, we can add a ctx.ResetRequest and use it on the FromStdWithNext to update the request automatically as well, if you think that's useful, sounds great? [UPDATE: it's implemented on v11.2.0 PR which contains a simple test too]

However, I have an obligation to warn you that the std context.WithValue thing and req.WithContext are bit slow because they do copies. Prefer to stick with iris-defined ctx.Values() whenever you can.

Thanks,
Gerasimos Maropoulos

@kataras kataras added good first issue A user wrote a good first issue with clear instructions 🪶 type: feature 🕰 status:in-progress labels Feb 2, 2019
@kataras kataras added this to the v11.2.0 milestone Feb 2, 2019
kataras added a commit that referenced this issue Feb 2, 2019
… the request for any incoming request changes - #1180
@Julio-Guerra
Copy link
Author

You went fast 😄 But indeed, other Go frameworks have similar SetRequest() methods to set it from middlewares.

However, I have an obligation to warn you that the std context.WithValue thing and req.WithContext are bit slow because they do copies. Prefer to stick with iris-defined ctx.Values() whenever you can.

Yes, it does a shallow copy. I am not a fan of this WithContext() also, but it's the only way for now and some of our users only pass the request or the request context to functions called by handlers.

Thanks for your support!

@kataras
Copy link
Owner

kataras commented Feb 2, 2019

Yes, I was in turbo mode yesterday as well :) We could check the request.Context().Value before returning from ctx.Values().Get(key) but the limitation is that the ctx.Values() returns a memstore.Store from our core package, it is de-coupled by-design and I think it's better to keep it that way, if you think otherwise I can createa new wrapper for store that we can pass the *http.Request value.

I thank you for the question!!

@kataras
Copy link
Owner

kataras commented Jul 18, 2019

Hello @Julio-Guerra, sorry for the delay but I've just read an issue inside your go-agent project 28 days ago, here, I thought that was solved with ctx.ResetRequest(newRequest) as implemented after this issue of yours.

iris/context/context.go

Lines 117 to 128 in 3cd0837

// ResetRequest sets the Context's Request,
// It is useful to store the new request created by a std *http.Request#WithContext() into Iris' Context.
// Use `ResetRequest` when for some reason you want to make a full
// override of the *http.Request.
// Note that: when you just want to change one of each fields you can use the Request() which returns a pointer to Request,
// so the changes will have affect without a full override.
// Usage: you use a native http handler which uses the standard "context" package
// to get values instead of the Iris' Context#Values():
// r := ctx.Request()
// stdCtx := context.WithValue(r.Context(), key, val)
// ctx.ResetRequest(r.WithContext(stdCtx)).
ResetRequest(r *http.Request)

Let's give you a sample code and tell me if that suits your case:

newRequest := ctx.Request().WithContext(yours go-agent's sql standard context.Context)
ctx.ResetRequest(newRequest)

Also the FromStdWithNext sets this for any case the net/http Request's context modified.

Did you mean that you wait the v11.2.0 to be released or the solution doesn't fix your issue? (although you can go get github.com/kataras/[email protected])

Thanks!

@kataras kataras closed this as completed Jul 23, 2019
github-actions bot pushed a commit to goproxies/github.aaakk.us.kg-kataras-iris that referenced this issue Jul 27, 2020
… the request for any incoming request changes - kataras#1180

Former-commit-id: 764bf26bcaa3b7bdae0a2bdbf3bf2b6f8c5c546e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue A user wrote a good first issue with clear instructions 🚀 status:implemented 🪶 type: feature
Projects
None yet
Development

No branches or pull requests

2 participants