xeqi / peridot

a basic api for interacting with ring apps
144 stars 20 forks source link

Query params problem with Compojure #39

Closed LukasRychtecky closed 6 years ago

LukasRychtecky commented 7 years ago

Hi, thanks for maintaining this project!

I've noticed a potencional issue when using it with Compojure. When I do a request in tests like:

(-> (kerodon/session handler)
      (peridot/request "/profile/123?foo=bar" :params {:baz "buz"}))

Query params (foo, baz) are not in a request, neither in :params, :route-params, nor :query-params.

But when I do a request from cURL (or something like this) the params are in the request map (:params, ...).

Have you ever run into this issue? Thanks for answering.

danielcompton commented 7 years ago

Hi @LukasRychtecky, do you have a minimal test case we can look at to reproduce this?

LukasRychtecky commented 6 years ago

Hello, sorry for a long time without response.

Here is a demo project based on Duct (I hope it's not related to the issue) https://github.com/LukasRychtecky/peridot-query-params.

Test output:

foo:  nil
baz:  nil
{:remote-addr localhost, :params {}, :route-params {}, :headers {host localhost}, :server-port 80, :compojure/route [:get /], :path-info /, :context /example, :uri /example, :server-name localhost, :query-string foo=bar&baz=buz, :body nil, :scheme :http, :request-method :get}

When I do a request from cURL/browser:

foo:  bar
baz:  buz
{:ssl-client-cert nil, :protocol HTTP/1.1, :cookies {ring-session {:value 721aea16-f058-4e60-899e-e7a0ce59e3bd}}, :remote-addr 0:0:0:0:0:0:0:1, :params {:foo bar, :baz buz}, :flash nil, :route-params {}, :headers {host localhost:3000, user-agent Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/63.0.3239.84 Safari/537.36, cookie ring-session=721aea16-f058-4e60-899e-e7a0ce59e3bd, connection keep-alive, upgrade-insecure-requests 1, if-modified-since Wed, 13 Dec 2017 08:45:32 GMT, accept text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8, accept-language en-US,en;q=0.9, accept-encoding gzip, deflate, br, cache-control max-age=0}, :server-port 3000, :content-length nil, :form-params {}, :compojure/route [:get /], :session/key nil, :query-params {foo bar, baz buz}, :content-type nil, :path-info /, :character-encoding nil, :context /example, :uri /example, :server-name localhost, :query-string foo=bar&baz=buz, :body #object[org.eclipse.jetty.server.HttpInputOverHTTP 0x336d4373 HttpInputOverHTTP@336d4373], :multipart-params {}, :scheme :http, :request-method :get, :session {}}

In "real" request parameters are destructured by Compojure. They are also in request's :params and :query-params.

In tests request parameters aren't destructured and they aren't in :params and :query-params.

I hope this will help to find bug. Or find out that this bug is not related to peridot.

If you need any help, just let me know.

Thanks and Merry Christmas.

LukasRychtecky commented 6 years ago

I've digged into the issue and the issue is in Duct's template (https://github.com/duct-framework/duct/issues/73). I'd close this issue. Sorry for disturbing.