Chi: Required path parameters requires regular expressions

[leighmcculloch]

Path parameters in chi are optional whenever a path has components following that parameter. For example the following route registered on a chi router can be matched with the following requests.

router.Get("/accounts/{account_id}/payments", ...)

GET /accounts/GBWQZBF26VAAWPSJ6M6O5CKK4F5TNRHWDRXGERV4VIPO6QAK6LCLLXKF/payments
GET /accounts//payments

To make a path parameter required a regular expression can be used. Using the same example the account_id parameter can be made required with the following.

router.Get("/accounts/{account_id:\\w+}/payments", ...)

Using regular expressions in routes seems overkill given we just want to require the field to have some value and we do no need to validate its format at this point.

[leighmcculloch]

One solution to remove the need for regular expressions in this instance could be to add syntax to path parameters for specifying that a parameter is required.

As an example and using @bartekn's idea that was posted here https://github.com/stellar/go/pull/2155#discussion_r370574999, we could prefix the parameter with !.

router.Get("/accounts/{!account_id}/payments", ...)

I'm open to implementing this or something like this if there would be interest in accepting it.

[pkieltyka]

@leighmcculloch what you should do is check chi.URLParam("account") in a middleware and return an error response that the account is missing or invalid.

For example,

func ValidateURL(next http.Handler) http.Handler {
    return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

        accountParam := chi.URLParam(r, "account")
        if accountParam == "" {
            w.WriteHeader(404)
            w.Write([]byte(http.StatusText(404)))
            return
        }

        next.ServeHTTP(w, r)
    })
}

however, i'd recommend a helper response method that returns error codes and payload likely some kind of json response.

even better is to do something like this which handles validation and also prepares the data for the account and puts it on the request context -- see https://github.com/go-chi/chi/blob/master/_examples/rest/main.go#L88 and https://github.com/go-chi/chi/blob/master/_examples/rest/main.go#L124-L148

[pkieltyka]

FYI, you can do..

r.With(SomeMiddleware).Get("/etc/{account}", h)

for a specific route, or of course use .Use() or .Group() to a group of routes. And finally, you could get fancy and write a middleware like

r.Use(RequireParams("account_id", "blah", "another"))

which would be a higher-order middleware that takes params ...string and you'd loop to ensure they are included, etc..

all depends on your logic. Point is, chi is designed to be a minimal system, and middlewares give you full control of everything on the request flow

[leighmcculloch]

@pkieltyka Thanks for the tips!

We validate path parameters a little differently because we keep that close to where the normal request parsing logic lives rather than disconnecting that farther high up the chain.

Regardless, I see what you're saying that we can use a middleware to catch if a parameter isn't specified and prevent a route match. Below is how I'm thinking it might play out with a generic middleware that makes a parameter required and hit the normal 404 Not Found without using regex.

What do you think? If you think this could be included in chi's middleware package I'd be happy to add it with tests, etc.

func RequiredParam(param string) func(http.Handler) http.Handler {            
    return func(next http.Handler) http.Handler {                             
        return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
            value := chi.URLParam(r, param)                                   
            if value == "" {                                                  
                notFoundHandler := http.NotFound                              
                rctx, _ := r.Context().Value(chi.RouteCtxKey).(*chi.Context)  
                if rctx != nil {                                              
                    notFoundHandler = mux.Routes.NotFoundHandler()            
                }                                                             
                notFoundHandler(w, r)                                         
                return                                                        
            }
            next.ServeHTTP(w, r)                                              
        })                                                                    
    }                                                                         
}                                                                             

[pkieltyka]

looks good, and could have RequiredParams version too which accepts params ...string -- sure, if you want to add middleware/params.go with this, lets do it.

I still think someone putting in an invalid value is the same as an empty one though, so it doesn't preclude having the handle the param value correctly

[leighmcculloch]

Thanks! I'll slot sometime in to contribute this back soon.

[rof20004]

@leighmcculloch @pkieltyka

What about response messages?

Can be configured or will be a generic message?

Example: id is required

What message will go to the client?

Thanks :)

[pkieltyka]

@rof20004 totally, that is the beauty of how simple the middlewares hook into the request-response flow. You can handle responses in middlewares too, and they can be contextual by looking at r.Context() or other data in r