ring icon indicating copy to clipboard operation
ring copied to clipboard

nested-params supports multiple keys

Open kochb opened this issue 9 years ago • 5 comments

nested-params currently processes only the :params, but wrap-params also assoc's :form-params and :query-params onto the query map. In cases where you need to differentiate between query and body params, wrap-nested-params will be of no use to you.

This change lets you specify which keys you want to process:

(-> app
   (wrap-nested-params {:params-keys [:params :form-params :query-params]})
   (wrap-params))

It may be more proper to process all three by default, because the current behavior of wrap-nested-params might be unexpected. I left it at the more conservative default of just :params for now for consistency across versions.

kochb avatar May 13 '15 15:05 kochb

The *-params keys are left alone by design, because they're designed to accurately reflect the parameters parsed from various sources. The :params key is there for convenience only, and therefore can be changed by middleware if it aids convenience.

What's your use-case for wanting to restrict nested parameters to a particular origin?

weavejester avatar May 13 '15 16:05 weavejester

It's not restricting nested params to a particular origin, nested parameter parsing should be applied to data from both. The problem occurs when I want to only retrieve parameters from a particular origin.

In our restful API, content can be submitted for write via the body on write requests, while on read requests query parameters can be used to filter results. They're two different sets of parameters that serve different purposes.

There is a small risk that a misbehaving client includes data in both the body and query params. The query params would contaminate or override the data supplied in the body, since :params is a merge of the two. You can't undo that merge if a key conflict occurred, you have to go back to the source in :form-params, which hasn't been processed by the nested-params middleware.

This all is more of a problem with wrap-params disregarding the differences between the two, but nested-params does not currently provide a way to cope with that.

Wanting to preserve the original data is a fair argument for not touching them by default, but the change introduced is opt-in behavior, by using the middleware you are consenting to have the query map manipulated as you requested anyway.

kochb avatar May 13 '15 20:05 kochb

It sounds as if your particular use case might be better served with the introduction of wrap-query-params and wrap-form-params middleware. Then you could choose to limit GET routes to query parameters, and POST routes to form parameters.

weavejester avatar May 13 '15 21:05 weavejester

That seems a bit odd - I should branch my middleware on a per-route basis? I could do it based on the request-method, but how do I support a POST request that uses a query parameter in addition to a request body? Query params and body params aren't an either/or - http bodies are traditionally only on POST/PUT/PATCH, but any http method accepts query params. A resource is permitted to support both.

I might be missing something here, what is the issue that you're seeing? Nested parameters are merely an encoding format, what problems are introduced by asking a middleware to decode a specific parameter?

kochb avatar May 13 '15 21:05 kochb

The :*-params keys are currently guaranteed to be a particular format, and I'm somewhat against breaking that expectation. This change would also necessitate adding the same behavior to wrap-keyword-params, and the repetition of that bothers me.

It's possible I could expose more of the internal workings of the nested-params middleware, allowing one to write their own middleware more easily. But I don't like the idea of breaking expected behavior, even as an option.

weavejester avatar May 13 '15 22:05 weavejester