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

This middleware shouldn't throw exceptions when `limit` or `offset`
query parametesr aren't integers.

Co-authored-by: John Swanson <john.swanson@metabase.com>

src/metabase/server/middleware/offset_paging.clj

@@ -1,8 +1,6 @@
 (ns metabase.server.middleware.offset-paging
   (:require
-   [medley.core :as m]
-   [metabase.server.middleware.security :as mw.security]
-   [metabase.util.i18n :refer [tru]]))
+   [medley.core :as m]))
 
 (set! *warn-on-reflection* true)
 
@@ -17,15 +15,11 @@
   Automatically generated by a handler in offset-paging middleware."
   false)
 
-(defn- offset-paged? [{{:strs [page limit offset]} :query-params}]
-  (or page limit offset))
-
 (defn- parse-paging-params [{{:strs [limit offset]} :query-params}]
-  (let [limit  (or (some-> limit Integer/parseUnsignedInt)
-                   default-limit)
-        offset (or (some-> offset Integer/parseUnsignedInt)
-                   default-offset)]
-    {:limit limit, :offset offset}))
+  (let [limit  (some-> limit parse-long)
+        offset (some-> offset parse-long)]
+    (when (or limit offset)
+      {:limit (or limit default-limit), :offset (or offset default-offset)})))
 
 (defn- with-paging-params [request {:keys [limit offset]}]
   (-> request
@@ -41,22 +35,9 @@
   (it isn't stable with respect to underlying data changing, though)"
   [handler]
   (fn [request respond raise]
-    (if-not (offset-paged? request)
-      (handler request respond raise)
-      (let [paging-params (try
-                            (parse-paging-params request)
-                            (catch Throwable e
-                              e))]
-        (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))))))))
+    (if-let [{:keys [limit offset] :as paging-params} (parse-paging-params request)]
+      (binding [*limit* limit
+                *offset* offset
+                *paged?* true]
+        (handler (with-paging-params request paging-params) respond raise))
+      (handler request respond raise))))

test/metabase/server/middleware/offset_paging_test.clj

@@ -5,8 +5,6 @@
    [clojure.test :refer :all]
    [metabase.server.handler :as handler]
    [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.util.response :as response])
   (:import
@@ -48,11 +46,10 @@
                        "paged?" true
                        "params" {"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
-  ;; form and the actual form
-  (mt/with-clock #t "2023-02-20T15:01:00-08:00[US/Pacific]"
-    (testing "invalid params"
-      (is (=? {:status  400
-               :headers (mw.security/security-headers)
-               :body    {"message" #"Error parsing paging parameters.*"}}
-              (read-response (handler (ring.mock/request :get "/" {:offset "abc", :limit "100"}))))))))
+  (testing "w/ non-numeric paging params, paging is disabled"
+    (is (=? {:status 200
+             :body {"limit" nil
+                    "offset" nil
+                    "paged?" false
+                    "params" {}}}
+            (read-response (handler (ring.mock/request :get "/" {:offset "foo" :limit "bar"})))))))