ktor icon indicating copy to clipboard operation
ktor copied to clipboard

call.parameters containing query parameter instead of path segment

Open r4zzz4k opened this issue 5 years ago • 4 comments

Ktor Version

1.2.0-alpha-1

Ktor Engine Used(client or server and name)

Tried both Netty and CIO

JVM Version, Operating System and Relevant Context

openjdk version "1.8.0_191" Ubuntu 18.04

Feedback

The following routing configuration

route("/{bar}") {
	get {
		call.respond(HttpStatusCode.OK, "${call.request.uri}\n" +
			"\tcall.parameters[\"bar\"]: ${call.parameters["bar"]}\n" +
			"\tcall.request.queryParameters[\"bar\"]: ${call.request.queryParameters["bar"]}")
	}
}

provides the following responses:

/foo?bar=baz
	call.parameters["bar"]: baz
	call.request.queryParameters["bar"]: baz
/foo?baz=baz
	call.parameters["bar"]: foo
	call.request.queryParameters["bar"]: null

So when we try to get path component value by it's name from `call.parameters', and the request has query parameter of the same name, we get query parameter instead of expected path component. Neither KDoc comment nor documentation mentions this behavior. Is it expected?

Kudos to @frost13it for this interesting finding.

r4zzz4k avatar Mar 14 '19 13:03 r4zzz4k

Yes, it seems to be expected: to get all values use call.parameters.getAll("bar") white the get operator returns the first occurrence. The question is what priority it should have...

cy6erGn0m avatar Mar 14 '19 14:03 cy6erGn0m

I've looked through all samples using call.parameters, and all of them are using it to get specifically path segments, both via get and getAll. Why query parameters are present there at all? I believe that one of these should be true:

  • call.parameters is store explicitly for path segments, and query parameters shouldn't be present there
  • call.parameters is store akin to PHP's $_REQUEST which is a garbage bin for all kinds of things that can be labeled a "parameter", and there should be another store similar to call.request.queryParameters (e.g. call.request.pathSegments), where one can find specifically path segments.

I can't come up with situation when developer would expect that query parameter implicitly hides path segment value with the same name (not that I claim such situations don't exist), so I'd say this is a surprise point for library users.

r4zzz4k avatar Mar 14 '19 14:03 r4zzz4k

I agree, parameters and queryParameters are not the same thing at all.

LouisCAD avatar Mar 15 '19 10:03 LouisCAD

Please check the following ticket on YouTrack for follow-ups to this issue. GitHub issues will be closed in the coming weeks.

oleg-larshin avatar Aug 10 '20 15:08 oleg-larshin