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

c.URLParams available in middleware? #32

Closed
phea opened this issue Jun 14, 2014 · 11 comments
Closed

c.URLParams available in middleware? #32

phea opened this issue Jun 14, 2014 · 11 comments

Comments

@phea
Copy link

phea commented Jun 14, 2014

I'm trying to access c.URLParams in a middleware function of mine, however c.URLParams doesn't seem to be initiated. I am either doing something wrong on my end or maybe URLParams is initiated after all the middleware calls. Anyone able to clarify.

@elithrar
Copy link
Contributor

You'll need to initialize the map if it hasn't been done yet. See
https://github.com/zenazn/goji/blob/master/web/middleware/request_id.go#L62-64
for
an example.

On Sunday, June 15, 2014, Phea Duch notifications@github.com wrote:

I'm trying to access c.URLParams in a middleware function of mine, however
c.URLParams doesn't seem to be initiated. I am either doing something wrong
on my end or maybe URLParams is initiated after all the middleware calls.


Reply to this email directly or view it on GitHub
#32.

@phea
Copy link
Author

phea commented Jun 15, 2014

Does not work, I think I will just move the particular code from middleware to the handler. Probably makes more sense anyways.

@elithrar
Copy link
Contributor

Can you post your code?

On Sunday, June 15, 2014, Phea Duch notifications@github.com wrote:

Does not work, I think I will just move the particular code from
middleware to the handler. Probably makes more sense anyways.


Reply to this email directly or view it on GitHub
#32 (comment).

@elithrar
Copy link
Contributor

Ignore what I posted before.

You have to have this in the handler: middleware applies to all routes
under a router or sub router, and there is no guarantee that the map key
(i.e. c.URLParams["id"]) is unique across that router, as URL params are
effectively scoped to a single route.

/things/show/:id and /user/:id would share the same key from a middleware
perspective, which is not what most package users expect.

You can therefore put it into the handler or use r.URL.Path to get the
whole URL path and use that as you wish.

On Sunday, June 15, 2014, Phea Duch notifications@github.com wrote:

Does not work, I think I will just move the particular code from
middleware to the handler. Probably makes more sense anyways.


Reply to this email directly or view it on GitHub
#32 (comment).

@phea
Copy link
Author

phea commented Jun 15, 2014

I see, that makes a lot of sense. I will just move the logic to the handler for better readability. Thanks for the help.

@phea phea closed this as completed Jun 15, 2014
@zenazn
Copy link
Owner

zenazn commented Jun 15, 2014

Yeah—middleware can change the URL of the incoming request, so it's impossible to bind URL parameters until after the middleware stack runs.

If you're interested in using URL parameters in middleware specifically, and your URL structure is amenable to it, you could do something like this:

m := web.New()
m.Use(someMiddlewareThatUsesID)
goji.Handle("/post/:id/*", m)
m.Get("/post/:id/comments", commentsHandler)

@bradrydzewski
Copy link

It would be nice to have a little more control over this behavior.

The reason I ask is that I started down the path of implementing goji for my project: github.com/drone/drone

I was trying to use middleware to verify that a user can access a particular repository. The code looks something like this:

func SetRepo(c *web.C, h http.Handler) http.Handler {
    fn := func(w http.ResponseWriter, r *http.Request) {
        var (
            host  = c.URLParams["host"]
            owner = c.URLParams["owner"]
            name  = c.URLParams["repo"]
            user  = ToUser(c)
        )

        repo, err := datastore.GetRepo(c, host, owner, name)
        ...
        role, err := datastore.GetRole(c, user, repo)
        if err != nil || role.Read != true {
            w.WriteHeader(http.StatusNotFound)
            return
        }
        ...

I was trying to troubleshoot why it wasn't working and came across this thread. I don't modify the path in any of my middleware, so I would not be subject the restriction previously mentioned. Either way, could goji track changes to the path and re-parse the parameters in-between middleware?

Just some food for thought ...

@zenazn
Copy link
Owner

zenazn commented Sep 28, 2014

@bradrydzewski — It's unfortunately a little trickier than that. The fact that route parsing happens after the middleware stack is pretty deeply baked into Goji's design, and it's not going to be easy to change.

The best motivating example is probably Goji's support for custom URL patterns. Both of the built-in pattern types (Sinatra-style /patterns/:id and regular expressions with captures) only look at the path, but there's no reason someone couldn't write a custom pattern that looked at HTTP headers, for example, or even did something completely unrelated (say, kept a global counter and displayed every 10,000th user a custom page). It's impossible for Goji to keep track of all the variables that might go in to route selection, and therefore it's impossible for Goji to know when to update its cached route selection decision.

I do think this behavior is not at all obvious, and it's especially non-obvious why it must be the case. I certainly owe the world more in-depth documentation here (maybe a blog post?), so thanks for pointing this out.

Come to think of it though, there might be an interesting technical solution here: a special "middleware" called web.Router (or something). If it is placed into the middleware stack explicitly, a routing decision will be made at that point in the stack and the corresponding http.Handler will be placed into an Env variable. At the end of the middleware stack, the Handler will be pulled out of the environment, or if no Handler is set there, routing will be run at that time.

Let me think about this some more, but I think this would allow you to place middleware after the routing logic while still playing nice with the rest of Goji.

zenazn added a commit that referenced this issue Apr 12, 2015
This change exposes a new type, Match, which represents a matched route.
When present in the Goji environment (bound to the key MatchKey),
routing will be skipped and the bound Match will be dispatched to
instead.

In addition, the Goji router has been exposed as a middleware using the
Match mechanism above. This allows middleware inserted after the router
access to the Match object and any bound URLParams.

Fixes #76. See also #32.
@kurianCoding
Copy link

is it possible to set URL PARAMS in middleware?

@elithrar
Copy link
Contributor

@kurianCoding You can just save over the map entry or create your own in middleware as you see fit - e.g. c.URLParams["name"] = "Dave" (before calling the next handler)

@kurianCoding
Copy link

@elithrar thanks, I had tried this method. It didn't work. Now its possible to send custom params using the c.Env. I was thinking about using a.URLParams because parameters show on the query string.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants