#492 Addressing page parameter errors.

clojars · Mar 7, 2016 · 8a88226 · 8a88226
1 parent a0d7c0a
commit 8a88226
Show file tree

Hide file tree

Showing 4 changed files with 57 additions and 16 deletions.
diff --git a/src/clojars/errors.clj b/src/clojars/errors.clj
@@ -3,7 +3,8 @@
             [yeller.clojure.client :as yeller]
             [clojars.config :refer [config]]
             [clj-stacktrace.repl :refer [pst]]
-            [clojars.web.error-page :as error-page])
+            [clojars.web.error-page :as error-page]
+            [clojars.web.error-api :as error-api])
   (:import java.util.UUID
            com.yellerapp.client.YellerHTTPClient))
 
@@ -50,5 +51,9 @@
     (try
       (app req)
       (catch Throwable t
-        (->> (report-ring-error reporter t req)
-             (error-page/error-page-response))))))
+        (let [params (:params req)
+              err-response-fn (if (= (:format params) "json")
+                                error-api/error-api-response
+                                error-page/error-page-response)]
+          (->> (report-ring-error reporter t req)
+               (err-response-fn (ex-data t))))))))
diff --git a/src/clojars/web.clj b/src/clojars/web.clj
@@ -40,6 +40,21 @@
              [resource :refer [wrap-resource]]
              [session :refer [wrap-session]]]))
 
+(defn try-parse-page
+  "Will throw a targeted error if maybe-page doesn't parse as an integer."
+  [maybe-page]
+  (try
+    (let [i (Integer/parseInt maybe-page)]
+      i)
+    (catch NumberFormatException nfe
+      (let [e (ex-info
+               "page must be an integer"
+               {:report? false
+                :title "Bad Request"
+                :error-message "The page query parameter must be an integer."
+                :status 400})]
+        (throw e)))))
+
 (defn main-routes [db reporter stats search-obj mailer]
   (routes
    (GET "/" _
@@ -50,7 +65,7 @@
    (GET "/search" {:keys [params]}
         (try-account
           #(let [validated-params (if (:page params)
-                                    (assoc params :page (Integer. (:page params)))
+                                    (assoc params :page (try-parse-page (:page params)))
                                     params)]
              (search search-obj % validated-params))))
    (GET "/projects" {:keys [params]}

diff --git a/src/clojars/web/error_api.clj b/src/clojars/web/error_api.clj
@@ -0,0 +1,17 @@
+(ns clojars.web.error-api
+  (:require [ring.util.response :refer [response status content-type]]
+            [cheshire.core :as json]))
+
+(defn error-api-response [error-options error-id]
+  (let [defaults {:title "Oops, we encountered an error"
+                  :error-message "It seems as if an internal system error has occurred. Please give it another try. If it still doesn't work, please open an issue at https://github.com/clojars/clojars-web/issues."
+                  :status 500}
+        options (merge defaults error-options)
+        body (json/generate-string
+              {:error (:error-message options)
+               :error-id error-id})
+        response {:status (:status options)
+                  :headers {"Content-Type" "application/json; charset=UTF-8"
+                            "Access-Control-Allow-Origin" "*"}
+                  :body body}]
+    response))
diff --git a/src/clojars/web/error_page.clj b/src/clojars/web/error_page.clj
@@ -3,15 +3,19 @@
             [ring.util.response :refer [response status content-type]]
             [hiccup.element :refer [link-to]]))
 
-(defn error-page-response [error-id]
-  (-> (response (html-doc "Oops, we encountered an error" {}
-                 [:div.small-section
-                  [:h1 "Oops!"]
-                  [:p
-                   "It seems as if an internal system error has occurred. Please give it another try. If it still doesn't work please "
-                   (link-to "https://github.com/clojars/clojars-web/issues" "open an issue")
-                   " and include:"]
-                  [:p
-                   [:pre.error-id (format "error-id:\"%s\"" error-id)]]]))
-      (status 500)
-      (content-type "text/html")))
+(defn error-page-response [error-options error-id]
+  (let [defaults {:title "Oops, we encountered an error"
+                  :error-message [:p
+                                  "It seems as if an internal system error has occurred. Please give it another try. If it still doesn't work please "
+                                  (link-to "https://github.com/clojars/clojars-web/issues" "open an issue")
+                                  " and include:"]
+                  :status 500}
+        options (merge defaults error-options)]
+    (-> (response (html-doc (:title options) {}
+                            [:div.small-section
+                             [:h1 "Oops!"]
+                             (:error-message options)
+                             [:p
+                              [:pre.error-id (format "error-id:\"%s\"" error-id)]]]))
+        (status (:status options))
+        (content-type "text/html"))))