Skip to content
Snippets Groups Projects
Unverified Commit cce183e7 authored by John Swanson's avatar John Swanson Committed by GitHub
Browse files

Don't throw when sent non-int `limit`/`offset` (#40821)

This middleware shouldn't throw exceptions when `limit` or `offset`
query parametesr aren't integers.
parent 83865263
No related branches found
No related tags found
No related merge requests found
(ns metabase.server.middleware.offset-paging (ns metabase.server.middleware.offset-paging
(:require (:require
[medley.core :as m] [medley.core :as m]))
[metabase.server.middleware.security :as mw.security]
[metabase.util.i18n :refer [tru]]))
(set! *warn-on-reflection* true) (set! *warn-on-reflection* true)
...@@ -17,15 +15,11 @@ ...@@ -17,15 +15,11 @@
Automatically generated by a handler in offset-paging middleware." Automatically generated by a handler in offset-paging middleware."
false) false)
(defn- offset-paged? [{{:strs [page limit offset]} :query-params}]
(or page limit offset))
(defn- parse-paging-params [{{:strs [limit offset]} :query-params}] (defn- parse-paging-params [{{:strs [limit offset]} :query-params}]
(let [limit (or (some-> limit Integer/parseUnsignedInt) (let [limit (some-> limit parse-long)
default-limit) offset (some-> offset parse-long)]
offset (or (some-> offset Integer/parseUnsignedInt) (when (or limit offset)
default-offset)] {:limit (or limit default-limit), :offset (or offset default-offset)})))
{:limit limit, :offset offset}))
(defn- with-paging-params [request {:keys [limit offset]}] (defn- with-paging-params [request {:keys [limit offset]}]
(-> request (-> request
...@@ -41,22 +35,9 @@ ...@@ -41,22 +35,9 @@
(it isn't stable with respect to underlying data changing, though)" (it isn't stable with respect to underlying data changing, though)"
[handler] [handler]
(fn [request respond raise] (fn [request respond raise]
(if-not (offset-paged? request) (if-let [{:keys [limit offset] :as paging-params} (parse-paging-params request)]
(handler request respond raise) (binding [*limit* limit
(let [paging-params (try *offset* offset
(parse-paging-params request) *paged?* true]
(catch Throwable e (handler (with-paging-params request paging-params) respond raise))
e))] (handler request respond raise))))
(if (instance? Throwable paging-params)
(let [^Throwable e paging-params]
(respond {:status 400
:headers (mw.security/security-headers)
:body (merge
(Throwable->map e)
{:message (tru "Error parsing paging parameters: {0}" (ex-message e))})}))
(let [{:keys [limit offset]} paging-params
request (with-paging-params request paging-params)]
(binding [*limit* limit
*offset* offset
*paged?* true]
(handler request respond raise))))))))
...@@ -5,8 +5,6 @@ ...@@ -5,8 +5,6 @@
[clojure.test :refer :all] [clojure.test :refer :all]
[metabase.server.handler :as handler] [metabase.server.handler :as handler]
[metabase.server.middleware.offset-paging :as mw.offset-paging] [metabase.server.middleware.offset-paging :as mw.offset-paging]
[metabase.server.middleware.security :as mw.security]
[metabase.test :as mt]
[ring.mock.request :as ring.mock] [ring.mock.request :as ring.mock]
[ring.util.response :as response]) [ring.util.response :as response])
(:import (:import
...@@ -48,11 +46,10 @@ ...@@ -48,11 +46,10 @@
"paged?" true "paged?" true
"params" {"whatever" "true"}}} "params" {"whatever" "true"}}}
(read-response (handler (ring.mock/request :get "/" {:offset "200", :limit "100", :whatever "true"})))))) (read-response (handler (ring.mock/request :get "/" {:offset "200", :limit "100", :whatever "true"}))))))
;; set the system clock here so this doesn't flake if we cross the second boundary between evaluating the expected (testing "w/ non-numeric paging params, paging is disabled"
;; form and the actual form (is (=? {:status 200
(mt/with-clock #t "2023-02-20T15:01:00-08:00[US/Pacific]" :body {"limit" nil
(testing "invalid params" "offset" nil
(is (=? {:status 400 "paged?" false
:headers (mw.security/security-headers) "params" {}}}
:body {"message" #"Error parsing paging parameters.*"}} (read-response (handler (ring.mock/request :get "/" {:offset "foo" :limit "bar"})))))))
(read-response (handler (ring.mock/request :get "/" {:offset "abc", :limit "100"}))))))))
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment