diff --git a/src/puppetlabs/puppetdb/http/command.clj b/src/puppetlabs/puppetdb/http/command.clj index 6f023d9edc..0d903ecb91 100644 --- a/src/puppetlabs/puppetdb/http/command.clj +++ b/src/puppetlabs/puppetdb/http/command.clj @@ -12,7 +12,8 @@ [clojure.core.async :as async] [puppetlabs.kitchensink.core :as kitchensink] [puppetlabs.comidi :as cmdi] - [puppetlabs.i18n.core :refer [trs tru]]) + [puppetlabs.i18n.core :refer [trs tru]] + [puppetlabs.ring-middleware.core :as rmc]) (:import (clojure.lang ExceptionInfo) (java.net HttpURLConnection) @@ -317,8 +318,8 @@ add-received-param ;; must be (temporally) after wrap-with-request-params-validation wrap-with-request-params-validation wrap-with-request-normalization - mid/verify-accepts-json - (mid/verify-content-type ["application/json"]) + rmc/wrap-accepts-json + (rmc/wrap-content-type ["application/json"]) (mid/verify-content-encoding utils/supported-content-encodings) (mid/fail-when-payload-too-large reject-large-commands? max-command-size) (mid/wrap-with-metrics (atom {}) http/leading-uris) diff --git a/src/puppetlabs/puppetdb/http/server.clj b/src/puppetlabs/puppetdb/http/server.clj index 48bc13512b..9dbf5f8f0d 100644 --- a/src/puppetlabs/puppetdb/http/server.clj +++ b/src/puppetlabs/puppetdb/http/server.clj @@ -8,13 +8,12 @@ wrap-with-metrics wrap-with-illegal-argument-catch wrap-with-exception-handling - verify-accepts-json - verify-content-type make-pdb-handler verify-sync-version]] [puppetlabs.comidi :as cmdi] [puppetlabs.puppetdb.http.handlers :as handlers] - [puppetlabs.i18n.core :refer [tru]])) + [puppetlabs.i18n.core :refer [tru]] + [puppetlabs.ring-middleware.core :as rmc])) (defn- refuse-retired-api [version] @@ -70,8 +69,8 @@ (fn [req] (let [handler (-> (make-pdb-handler routes identity) wrap-with-illegal-argument-catch - verify-accepts-json - (verify-content-type ["application/json"]) + rmc/wrap-accepts-json + (rmc/wrap-content-type ["application/json"]) verify-sync-version (wrap-with-metrics (atom {}) http/leading-uris) (wrap-with-globals get-shared-globals) diff --git a/src/puppetlabs/puppetdb/meta.clj b/src/puppetlabs/puppetdb/meta.clj index a050c28461..edb719d6a0 100644 --- a/src/puppetlabs/puppetdb/meta.clj +++ b/src/puppetlabs/puppetdb/meta.clj @@ -5,6 +5,7 @@ [puppetlabs.puppetdb.meta.version :as v] [puppetlabs.puppetdb.time :refer [now]] [puppetlabs.puppetdb.config :as conf] + [puppetlabs.ring-middleware.core :as rmc] [puppetlabs.comidi :as cmdi] [bidi.schema :as bidi-schema] [puppetlabs.puppetdb.schema :as pls] @@ -49,7 +50,7 @@ (http/error-response (tru "Could not find version") 404)))) (catch java.io.IOException e - (log/debug e (trs "Error when checking for latest version") ) + (log/debug e (trs "Error when checking for latest version")) (http/error-response (tru "Error when checking for latest version: {0}" (.getMessage e)))))))) @@ -69,5 +70,5 @@ [get-shared-globals config] (-> (meta-routes get-shared-globals config) mid/make-pdb-handler - mid/verify-accepts-json + rmc/wrap-accepts-json mid/validate-no-query-params)) diff --git a/src/puppetlabs/puppetdb/middleware.clj b/src/puppetlabs/puppetdb/middleware.clj index 7f6f10a081..256844d3eb 100644 --- a/src/puppetlabs/puppetdb/middleware.clj +++ b/src/puppetlabs/puppetdb/middleware.clj @@ -162,21 +162,6 @@ (http/error-response (tru "An unexpected error occurred") HttpURLConnection/HTTP_INTERNAL_ERROR))))) -(defn verify-accepts-content-type - "Ring middleware that requires a request for the wrapped `app` to accept the - provided `content-type`. If the content type isn't acceptable, a 406 Not - Acceptable status is returned, with a message informing the client it must - accept the content type." - [app content-type] - {:pre [(string? content-type)]} - (fn [{:keys [headers] :as req}] - (if (http/acceptable-content-type - content-type - (headers "accept")) - (app req) - (http/error-response (tru "must accept {0}" content-type) - HttpURLConnection/HTTP_NOT_ACCEPTABLE)))) - (defn verify-content-encoding "Verification for the specified list of content-encodings." [app allowed-encodings] @@ -192,22 +177,6 @@ content-encoding) HttpURLConnection/HTTP_UNSUPPORTED_TYPE))))) -(defn verify-content-type - "Verification for the specified list of content-types." - [app content-types] - {:pre [(coll? content-types) - (every? string? content-types)]} - (fn [{:keys [headers] :as req}] - (if (= (:request-method req) :post) - (let [content-type (headers "content-type") - mediatype (if (nil? content-type) nil - (kitchensink/base-type content-type))] - (if (or (nil? mediatype) (some #{mediatype} content-types)) - (app req) - (http/error-response (tru "content type {0} not supported" mediatype) - HttpURLConnection/HTTP_UNSUPPORTED_TYPE))) - (app req)))) - (def params-schema {(s/optional-key :optional) [s/Str] (s/optional-key :required) [s/Str]}) @@ -248,14 +217,6 @@ [app] (validate-query-params app {})) -(def verify-accepts-json - "Ring middleware which requires a request for `app` to accept - application/json as a content type. If the request doesn't accept - application/json, a 406 Not Acceptable status is returned with an error - message indicating the problem." - (fn [app] - (verify-accepts-content-type app "application/json"))) - (def http-metrics-registry (get-in metrics/metrics-registries [:http :registry])) (defn wrap-with-metrics diff --git a/test/puppetlabs/puppetdb/http/nodes_test.clj b/test/puppetlabs/puppetdb/http/nodes_test.clj index d4dbb88369..ab279bc986 100644 --- a/test/puppetlabs/puppetdb/http/nodes_test.clj +++ b/test/puppetlabs/puppetdb/http/nodes_test.clj @@ -18,7 +18,7 @@ query-response]] [puppetlabs.puppetdb.testutils.log :refer [notable-pdb-event?]] [puppetlabs.puppetdb.testutils.nodes :refer [store-example-nodes]] - [puppetlabs.trapperkeeper.testutils.logging :as tk-log :refer [with-log-suppressed-unless-notable]]) + [puppetlabs.trapperkeeper.testutils.logging :refer [with-log-suppressed-unless-notable]]) (:import (java.net HttpURLConnection))) @@ -675,7 +675,7 @@ (assoc-in [:headers "content-type"] "application/x-www-form-urlencoded") *app*)] (is (= 415 status)) - (is (= "content type application/x-www-form-urlencoded not supported" body)))) + (is (re-find #"content-type application/x-www-form-urlencoded is not a supported" body)))) (testing "content type application/json should be accepted" (let [{:keys [status] :as response} (-> (tu/query-request :post endpoint ["extract" "certname" ["=" "certname" "puppet.example.com"]]) diff --git a/test/puppetlabs/puppetdb/middleware_test.clj b/test/puppetlabs/puppetdb/middleware_test.clj index 25f8f4ab69..e219e4e070 100644 --- a/test/puppetlabs/puppetdb/middleware_test.clj +++ b/test/puppetlabs/puppetdb/middleware_test.clj @@ -9,10 +9,9 @@ merge-param-specs validate-query-params verify-content-encoding - verify-content-type wrap-cert-authn wrap-with-certificate-cn - wrap-with-metrics] ] + wrap-with-metrics]] [clojure.test :refer :all] [puppetlabs.puppetdb.testutils :refer [temp-file]] [puppetlabs.ssl-utils.core :refer [get-cn-from-x509-certificate]] @@ -128,39 +127,6 @@ :headers {"Content-Type" http/error-response-content-type} :body "Unsupported query parameter 'wazzup'"}))))) -(deftest verify-content-type-test - (testing "with content-type of application/json" - (let [test-req {:request-method :post - :content-type "application/json" - :headers {"content-type" "application/json"}}] - - (testing "should succeed with matching content type" - (let [wrapped-fn (verify-content-type identity ["application/json"])] - (is (= (wrapped-fn test-req) test-req)))) - - (testing "should fail with no matching content type" - (let [wrapped-fn (verify-content-type identity ["application/bson" "application/msgpack"])] - (is (= (wrapped-fn test-req) - {:status 415 - :headers {"Content-Type" http/error-response-content-type} - :body "content type application/json not supported"})))))) - - (testing "with content-type of APPLICATION/JSON" - (let [test-req {:content-type "APPLICATION/JSON" - :headers {"content-type" "APPLICATION/JSON"}}] - - (testing "should succeed with matching content type" - (let [wrapped-fn (verify-content-type identity ["application/json"])] - (is (= (wrapped-fn test-req) test-req)))))) - - (testing "with content-type of application/json;parameter=foo" - (let [test-req {:content-type "application/json;parameter=foo" - :headers {"content-type" "application/json;parameter=foo"}}] - - (testing "should succeed with matching content type" - (let [wrapped-fn (verify-content-type identity ["application/json"])] - (is (= (wrapped-fn test-req) test-req))))))) - (deftest verify-content-encoding-test (testing "with content-encoding of gzip" (let [test-req {:request-method :post