From 5c59b20350fa4a872d8ab9ef1020b95e72e568a7 Mon Sep 17 00:00:00 2001 From: jonathannewman Date: Thu, 11 Jul 2024 07:42:04 -0700 Subject: [PATCH] (maint) replace wrappers with shared implementation This removes the use of `verify-accepts-json` and `verify-content-type` to leverage the ring-middleware additions in 2.0.3 of ring-middleware. Corresponding tests were removed as well. --- src/puppetlabs/puppetdb/http/command.clj | 7 ++-- src/puppetlabs/puppetdb/http/server.clj | 9 ++--- src/puppetlabs/puppetdb/meta.clj | 5 ++- src/puppetlabs/puppetdb/middleware.clj | 39 -------------------- test/puppetlabs/puppetdb/middleware_test.clj | 36 +----------------- 5 files changed, 12 insertions(+), 84 deletions(-) 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/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