Plumber param collision in dynamic path and query string

General
,
1

Perhaps I'm being dense, but I'm struggling to find any description of how to handle this breaking behavior with a simple Plumber endpoint defined on a dynamic path. Here's the annotation-based definition:

#* @get /foo/<x>
#* @serializer text
function(x) {
  str(x)  ## log to the console for debugging/inspection purposes
  x
}

Simple enough ... and works like a charm, a GET to http://localhost/foo/bar returns bar, as expected.

But what happens if someone makes a request to that endpoint and decides to add a query parameter like so: http://localhost/foo/bar?x=baz? Well, in that case:

  1. baz is returned -- which is incorrect from the perspective of the API designer, and
  2. an abstraction has just been leaked to the caller (which could be considered a security flaw, as the API designer cannot necessarily control how a caller will probe that API).

Adding the endpoint annotation:

#* @preempt queryString

... solves the issue, but Plumber's default behavior is (i) to have that filter enabled and (ii) it's not immediately obvious (especially to any new programmer). To know to explicitly disable that filter requires inspecting the default router (or reading Plumber's source).

I cannot find any mention of this issue in the documentation; am I missing something re: guidance on how best to handle such scenarios?

1 Like
2

Open an issue on github?

I believe just reversing the order the args are added to the endpoint args list would solve the issue.

Matching is done here :

During implementation, there is an ote to keep only the first matched same name argument to pass to the endpoint.

And here is the order the parameters are added to the args list

All args are available, but only one is matched to the endpoint.

#* @get /foo/<x>
#* @serializer text
function(x, req) {
  str(req$args)  ## log to the console for debugging/inspection purposes
  x
}
3

And the relevant issue since this has come up in the past :

4

And here is how to handle it from the comment in NEWS.md

#* @get /foo/<x>
#* @serializer text
function(req) {
  x <- req$argsPath$x
  str(x)  ## log to the console for debugging/inspection purposes
  x
}

Hopefully you have enough information to make an informed decision. Have a good day.

5

@meztez It looks like some of the behavior is defined only for POST requests? If so, was that intentional? While it's not necessarily normal for other HTTP methods to include bodies, there's nothing stopping the API caller from adding a payload to the request.

I think I'll open a GitHub issue anyhow, as minimally there's probably some additional documentation that can be added to help prevent subtle errors here. In particular, it seems somewhat cumbersome at present to safely protect against this, as it requires disabling the queryStringFilter from the defaultPlumberFilters, which itself then causes some more headaches when one does want to parse the query string ... worth a discussion at least, IMO (though perhaps y'all might feel differently :slight_smile: )

Thanks for the response!

6

You can protect against this by using the req$argsPath directly instead of relying on the internal matching. I fail to see where this would be defined only for POST request. Beside routes matching, I do not think the method has an impact on args.

7

This topic was automatically closed 42 days after the last reply. New replies are no longer allowed.

If you have a query related to it or one of the replies, start a new topic and refer back with a link.