From 22c1c1051ed2a758f69f0d45a5f50ea95983898d Mon Sep 17 00:00:00 2001 From: "Dr. Christian Betz" Date: Mon, 16 Mar 2020 17:53:24 +0100 Subject: [PATCH 1/9] Fix case insensitive header comparison In http-1.1, headers are to be compared case insensitive, and in http-2, headers are to be lower-cased before encoding. Thus, retrieving "Content-Type" in a case-sensitive fashion is likely to fail. Introduce two new abstractions muuntaja.core/header and .../header? to relieve direct dependencies on how headers are represented. --- modules/muuntaja/src/muuntaja/core.clj | 101 +++++++++++++++---------- test/muuntaja/ring_json/json_test.clj | 8 +- 2 files changed, 63 insertions(+), 46 deletions(-) diff --git a/modules/muuntaja/src/muuntaja/core.clj b/modules/muuntaja/src/muuntaja/core.clj index 3800851..c51e1ea 100644 --- a/modules/muuntaja/src/muuntaja/core.clj +++ b/modules/muuntaja/src/muuntaja/core.clj @@ -76,22 +76,39 @@ ;; default options ;; +(defn header + "Extract a specific header from a request or response map. + This has to be done in a case-insensitive manner for http-1.1. + With http-2 header names need to be lower-cased before encoding." + [^String header-name request-or-response] + (some (fn [[n v]] + (when (.equalsIgnoreCase header-name n) + v)) + (:headers request-or-response))) + +(defn header? + "Returns true iff a specific header is present in the request or + response map." + [^String header-name request-or-response] + (some #(.equalsIgnoreCase header-name %) + (keys (:headers request-or-response)))) + (defn extract-content-type-ring "Extracts content-type from ring-request." [request] (or (:content-type request) - (get (:headers request) "content-type"))) + (header "content-type" request))) (defn extract-accept-ring "Extracts accept from ring-request." [request] - (get (:headers request) "accept")) + (header "accept" request)) (defn extract-accept-charset-ring "Extracts accept-charset from ring-request." [request] - (get (:headers request) "accept-charset")) + (header "accept-charset" request)) (defn encode-collections [_ response] (-> response :body coll?)) @@ -101,23 +118,23 @@ (into #{} (map str/lower-case (.keySet (Charset/availableCharsets))))) (def default-options - {:http {:extract-content-type extract-content-type-ring - :extract-accept-charset extract-accept-charset-ring - :extract-accept extract-accept-ring - :decode-request-body? (constantly true) - :encode-response-body? encode-collections} + {:http {:extract-content-type extract-content-type-ring + :extract-accept-charset extract-accept-charset-ring + :extract-accept extract-accept-ring + :decode-request-body? (constantly true) + :encode-response-body? encode-collections} :allow-empty-input? true - :return :input-stream ;; :bytes :output-stream + :return :input-stream ;; :bytes :output-stream - :default-charset "utf-8" - :charsets available-charsets + :default-charset "utf-8" + :charsets available-charsets - :default-format "application/json" - :formats {"application/json" json-format/format - "application/edn" edn-format/format - "application/transit+json" transit-format/json-format - "application/transit+msgpack" transit-format/msgpack-format}}) + :default-format "application/json" + :formats {"application/json" json-format/format + "application/edn" edn-format/format + "application/transit+json" transit-format/json-format + "application/transit+msgpack" transit-format/msgpack-format}}) ;; ;; HTTP stuff @@ -131,11 +148,11 @@ (throw (ex-info (str "Malformed " (:format request-format-and-charset) " request.") - {:type :muuntaja/decode + {:type :muuntaja/decode :default-format (default-format m) - :format (:format request-format-and-charset) - :charset (:charset request-format-and-charset) - :request request} + :format (:format request-format-and-charset) + :charset (:charset request-format-and-charset) + :request request} e))) (defn- fail-on-response-decode-exception [m @@ -145,43 +162,43 @@ (throw (ex-info (str "Malformed " (:format response-format-and-charset) " response.") - {:type :muuntaja/decode + {:type :muuntaja/decode :default-format (default-format m) - :format (:format response-format-and-charset) - :charset (:charset response-format-and-charset) - :response response} + :format (:format response-format-and-charset) + :charset (:charset response-format-and-charset) + :response response} e))) (defn- fail-on-response-decode [m response] - (let [content-type (-> response :headers (get "Content-Type"))] + (let [content-type (header "Content-Type" response)] (throw (ex-info (if content-type (str "Unknown response Content-Type: " content-type) "No Content-Type found") - {:type :muuntaja/decode + {:type :muuntaja/decode :default-format (default-format m) - :response response})))) + :response response})))) (defn- fail-on-request-charset-negotiation [m] (throw (ex-info "Can't negotiate on request charset" - {:type :muuntaja/request-charset-negotiation + {:type :muuntaja/request-charset-negotiation :charsets (charsets m)}))) (defn- fail-on-response-charset-negotiation [m] (throw (ex-info "Can't negotiate on response charset" - {:type :muuntaja/response-charset-negotiation + {:type :muuntaja/response-charset-negotiation :charsets (charsets m)}))) (defn- fail-on-response-format-negotiation [m] (throw (ex-info "Can't negotiate on response format" - {:type :muuntaja/response-format-negotiation + {:type :muuntaja/response-format-negotiation :formats (encodes m)}))) (defn- set-content-type [response content-type] @@ -283,7 +300,7 @@ (throw (ex-info (str "Malformed " format " in " type "") - {:type type + {:type type :format format} e))))) @@ -302,10 +319,10 @@ (ex-info (str "Invalid format " (pr-str format) " for type " type ". " "It should satisfy " (pr-str (:on protocol))) - {:format format - :type type - :spec spec - :coder coder + {:format format + :type type + :spec spec + :coder coder :protocol protocol}))))] {key @@ -361,7 +378,7 @@ (throw (ex-info (str "invalid format: " format) - {:format format + {:format format :formats (keys formats)})) [format (map->Adapter (merge @@ -391,7 +408,7 @@ (throw (ex-info (str "Invalid default format " default-format) - {:formats valid-format? + {:formats valid-format? :default-format default-format}))) (let [m (reify Muuntaja (adapters [_] adapters) @@ -423,7 +440,7 @@ (catch Exception e (fail-on-request-decode-exception m e req-fc res-fc request)))))) -encode-response? (fn [request response] - (and (or (not (contains? (:headers response) "Content-Type")) + (and (or (not (header? "Content-Type" response)) (:muuntaja/encode response)) (encode-response-body? request response))) -resolve-response-charset (fn [_ request] @@ -441,7 +458,7 @@ (as-> response $ (dissoc $ :muuntaja/content-type) (update $ :body encoder charset) - (if-not (get (:headers $) "Content-Type") + (if-not (header "Content-Type" $) (set-content-type $ (content-type format charset)) $)))] ^{:type ::muuntaja} @@ -497,7 +514,7 @@ $)))) (-decode-response-body [this response] (or - (if-let [res-fc (-> response :headers (get "Content-Type") -negotiate-content-type)] + (if-let [res-fc (->> response (header "Content-Type") -negotiate-content-type)] (if-let [decode (decoder this (:format res-fc))] (try (decode (:body response) (:charset res-fc)) @@ -559,8 +576,8 @@ (throw (ex-info (str "invalid formats: " diff) - {:invalid (seq diff) - :formats (seq formats) + {:invalid (seq diff) + :formats (seq formats) :existing (seq existing-formats)}))) (-> options (update :formats select-keys formats) diff --git a/test/muuntaja/ring_json/json_test.clj b/test/muuntaja/ring_json/json_test.clj index ae10da0..818cc5d 100644 --- a/test/muuntaja/ring_json/json_test.clj +++ b/test/muuntaja/ring_json/json_test.clj @@ -253,15 +253,15 @@ (= body "{\n \"baz\" : \"quz\",\n \"foo\" : \"bar\"\n}"))))) (testing "CHANGED: don’t overwrite Content-Type if already set - format if :muuntaja/encode is truthy" - (let [handler (constantly {:status 200 :headers {"Content-Type" "application/json; some-param=some-value"} :body {:foo "bar"}, :muuntaja/encode true}) + (let [handler (constantly {:status 200 :headers {"content-type" "application/json; some-param=some-value"} :body {:foo "bar"}, :muuntaja/encode true}) response ((wrap-json-response handler) {}) body (-> response :body slurp)] - (is (= (get-in response [:headers "Content-Type"]) "application/json; some-param=some-value")) + (is (= (m/header "Content-Type" response) "application/json; some-param=some-value")) (is (= body "{\"foo\":\"bar\"}")))) (testing "CHANGED: don’t overwrite Content-Type if already set - don't format if :muuntaja/encode is not set" - (let [handler (constantly {:status 200 :headers {"Content-Type" "application/json; some-param=some-value"} :body {:foo "bar"}}) + (let [handler (constantly {:status 200 :headers {"coNTENt-Type" "application/json; some-param=some-value"} :body {:foo "bar"}}) response ((wrap-json-response handler) {}) body (-> response :body)] - (is (= (get-in response [:headers "Content-Type"]) "application/json; some-param=some-value")) + (is (= (m/header "Content-Type" response) "application/json; some-param=some-value")) (is (= body {:foo "bar"}))))) From 44f2fc3fe444abde16750ecdd2b233b921c9857e Mon Sep 17 00:00:00 2001 From: "Dr. Christian Betz" Date: Mon, 16 Mar 2020 17:54:58 +0100 Subject: [PATCH 2/9] Fix documented commands Helping others to contribute, as I stumbled over minor problems with CONTRIBUTING.md --- CONTRIBUTING.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e89d9df..d20143b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -10,15 +10,15 @@ Please file bug reports and feature requests to https://github.com/metosin/muunt * Create a topic branch from where you want to base your work (usually the master branch) * Check the formatting rules from existing code (no trailing whitepace, mostly default indentation) * Ensure any new code is well-tested, and if possible, any issue fixed is covered by one or more new tests -* Verify that all tests pass using ```lein midje``` +* Verify that all tests pass using ```lein test``` * Push your code to your fork of the repository * Make a Pull Request Installing jars and changing of version numbers can be done with the following scripts: ```sh -./script/set-version 1.0.0 -./script/lein-modules install +./scripts/set-version 1.0.0 +./scripts/lein-modules install ``` From af36b60fb3d37d5bfa8ca89026cfeac76f58b203 Mon Sep 17 00:00:00 2001 From: "Dr. Christian Betz" Date: Mon, 16 Mar 2020 17:55:27 +0100 Subject: [PATCH 3/9] Add next release to CHANGELOG --- CHANGELOG.md | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fb8b23a..20f8da8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,12 +1,17 @@ +## unreleased +**[compare](https://github.com/metosin/muuntaja/compare/0.6.6...master)** + +* Fix case insensitive header comparison: In http-1.1, headers are to be compared case insensitive, and in http-2, headers are to be lower-cased before encoding. Thus, retrieving "Content-Type" in a case-sensitive fashion is likely to fail. + ## 0.6.6 (2019-11-07) -**[compare](https://github.com/metosin/muuntaja/compare/0.6.5...master)** +**[compare](https://github.com/metosin/muuntaja/compare/0.6.5...0.6.6)** * Fix handler chaining when `nil` is returned from handler. ## 0.6.5 (2019-10-07) -**[compare](https://github.com/metosin/muuntaja/compare/0.6.4...master)** +**[compare](https://github.com/metosin/muuntaja/compare/0.6.4...0.6.5)** * Update deps: From 0b257cf5937bc385fb61a09dfb658e50a287fa97 Mon Sep 17 00:00:00 2001 From: "Dr. Christian Betz" Date: Tue, 17 Mar 2020 09:29:53 +0100 Subject: [PATCH 4/9] Update to lein v At gorillalabs we prefer managing versions by git tags and `lein v` commands. Also, lein v middleware makes it unneccessary to manually update dependency versions. Thus, the there's no need for `scripts/set-version` anymore. Also, managed-dependencies is kind of broken in combination with lein v middleware. However, it did not make sense to have the versions at a central place anyhow, since each central dependency is used in one place only. So I pushed the version of dependencies (e.g., jsonista) to the project definition where it is acutally used. --- .gitignore | 2 + modules/muuntaja-cheshire/project.clj | 12 ++-- modules/muuntaja-msgpack/project.clj | 12 ++-- modules/muuntaja-yaml/project.clj | 12 ++-- modules/muuntaja/project.clj | 14 +++-- project.clj | 91 +++++++++++++-------------- scripts/set-version | 8 --- 7 files changed, 80 insertions(+), 71 deletions(-) delete mode 100755 scripts/set-version diff --git a/.gitignore b/.gitignore index d244705..3f65348 100644 --- a/.gitignore +++ b/.gitignore @@ -8,3 +8,5 @@ pom.xml* .lein-repl-history .nrepl-port \#*\# +pom.properties +leiningen.core.classpath.extract-native-dependencies diff --git a/modules/muuntaja-cheshire/project.clj b/modules/muuntaja-cheshire/project.clj index 16a6c15..9a43259 100644 --- a/modules/muuntaja-cheshire/project.clj +++ b/modules/muuntaja-cheshire/project.clj @@ -1,4 +1,4 @@ -(defproject metosin/muuntaja-cheshire "0.6.6" +(defproject metosin/muuntaja-cheshire "0.0.0" ;; use lein v :description "Cheshire/JSON format for Muuntaja" :url "https://github.com/metosin/muuntaja" :license {:name "Eclipse Public License" @@ -6,8 +6,12 @@ :scm {:name "git" :url "https://github.com/metosin/muuntaja" :dir "../.."} - :plugins [[lein-parent "0.3.2"]] + :plugins [[lein-parent "0.3.2"] + [com.roomkey/lein-v "7.0.0"]] + :middleware [leiningen.v/version-from-scm + leiningen.v/dependency-version-from-scm + leiningen.v/add-workspace-data] :parent-project {:path "../../project.clj" - :inherit [:deploy-repositories :managed-dependencies]} + :inherit [:deploy-repositories]} :dependencies [[metosin/muuntaja] - [cheshire]]) + [cheshire "5.9.0"]]) diff --git a/modules/muuntaja-msgpack/project.clj b/modules/muuntaja-msgpack/project.clj index 0e5f2f7..0936480 100644 --- a/modules/muuntaja-msgpack/project.clj +++ b/modules/muuntaja-msgpack/project.clj @@ -1,4 +1,4 @@ -(defproject metosin/muuntaja-msgpack "0.6.6" +(defproject metosin/muuntaja-msgpack "0.0.0" ;; use lein v :description "Messagepack format for Muuntaja" :url "https://github.com/metosin/muuntaja" :license {:name "Eclipse Public License" @@ -6,8 +6,12 @@ :scm {:name "git" :url "https://github.com/metosin/muuntaja" :dir "../.."} - :plugins [[lein-parent "0.3.2"]] + :plugins [[lein-parent "0.3.2"] + [com.roomkey/lein-v "7.0.0"]] + :middleware [leiningen.v/version-from-scm + leiningen.v/dependency-version-from-scm + leiningen.v/add-workspace-data] :parent-project {:path "../../project.clj" - :inherit [:deploy-repositories :managed-dependencies]} + :inherit [:deploy-repositories]} :dependencies [[metosin/muuntaja] - [clojure-msgpack]]) + [clojure-msgpack "1.2.1" :exclusions [org.clojure/clojure]]]) diff --git a/modules/muuntaja-yaml/project.clj b/modules/muuntaja-yaml/project.clj index 1d6ffd6..99c39eb 100644 --- a/modules/muuntaja-yaml/project.clj +++ b/modules/muuntaja-yaml/project.clj @@ -1,4 +1,4 @@ -(defproject metosin/muuntaja-yaml "0.6.6" +(defproject metosin/muuntaja-yaml "0.0.0" ;; use lein v :description "YAML format for Muuntaja" :url "https://github.com/metosin/muuntaja" :license {:name "Eclipse Public License" @@ -6,8 +6,12 @@ :scm {:name "git" :url "https://github.com/metosin/muuntaja" :dir "../.."} - :plugins [[lein-parent "0.3.2"]] + :plugins [[lein-parent "0.3.2"] + [com.roomkey/lein-v "7.0.0"]] + :middleware [leiningen.v/version-from-scm + leiningen.v/dependency-version-from-scm + leiningen.v/add-workspace-data] :parent-project {:path "../../project.clj" - :inherit [:deploy-repositories :managed-dependencies]} + :inherit [:deploy-repositories]} :dependencies [[metosin/muuntaja] - [clj-commons/clj-yaml]]) + [clj-commons/clj-yaml "0.7.0"]]) diff --git a/modules/muuntaja/project.clj b/modules/muuntaja/project.clj index 14721b9..c0f8f28 100644 --- a/modules/muuntaja/project.clj +++ b/modules/muuntaja/project.clj @@ -1,4 +1,4 @@ -(defproject metosin/muuntaja "0.6.6" +(defproject metosin/muuntaja "0.0.0" ;; use lein v :description "Clojure library for format encoding, decoding and content-negotiation" :url "https://github.com/metosin/muuntaja" :license {:name "Eclipse Public License" @@ -6,8 +6,12 @@ :scm {:name "git" :url "https://github.com/metosin/muuntaja" :dir "../.."} - :plugins [[lein-parent "0.3.2"]] + :plugins [[lein-parent "0.3.2"] + [com.roomkey/lein-v "7.0.0"]] + :middleware [leiningen.v/version-from-scm + leiningen.v/dependency-version-from-scm + leiningen.v/add-workspace-data] :parent-project {:path "../../project.clj" - :inherit [:deploy-repositories :managed-dependencies]} - :dependencies [[metosin/jsonista] - [com.cognitect/transit-clj]]) + :inherit [:deploy-repositories]} + :dependencies [[metosin/jsonista "0.2.5"] + [com.cognitect/transit-clj "0.8.319"]]) diff --git a/project.clj b/project.clj index 2517cc1..e5f2e97 100644 --- a/project.clj +++ b/project.clj @@ -1,70 +1,69 @@ -(defproject metosin/muuntaja "0.6.6" +(defproject metosin/muuntaja "0.0.0" :description "Clojure library for format encoding, decoding and content-negotiation" :url "https://github.com/metosin/muuntaja" :license {:name "Eclipse Public License" :url "http://www.eclipse.org/legal/epl-v20.html"} :javac-options ["-Xlint:unchecked" "-target" "1.7" "-source" "1.7"] :java-source-paths ["src/java"] - :managed-dependencies [[metosin/muuntaja "0.6.6"] - [metosin/jsonista "0.2.5"] - [com.cognitect/transit-clj "0.8.319"] - [cheshire "5.9.0"] - [clj-commons/clj-yaml "0.7.0"] - [clojure-msgpack "1.2.1" :exclusions [org.clojure/clojure]]] :dependencies [] :source-paths ["modules/muuntaja/src"] - :plugins [[lein-codox "0.10.7"]] - :codox {:src-uri "http://github.com/metosin/muuntaja/blob/master/{filepath}#L{line}" + :plugins [[lein-codox "0.10.7"] + [com.roomkey/lein-v "7.0.0"]] + :middleware [leiningen.v/version-from-scm + leiningen.v/dependency-version-from-scm + leiningen.v/add-workspace-data] + + :codox {:src-uri "http://github.com/metosin/muuntaja/blob/master/{filepath}#L{line}" :output-path "doc" - :metadata {:doc/format :markdown}} + :metadata {:doc/format :markdown}} :scm {:name "git" - :url "https://github.com/metosin/muuntaja"} + :url "https://github.com/metosin/muuntaja"} :deploy-repositories [["releases" :clojars]] - :profiles {:dev {:jvm-opts ^:replace ["-server"] + :profiles {:dev {:jvm-opts ^:replace ["-server"] - ;; all module sources for development - :source-paths ["modules/muuntaja-cheshire/src" - "modules/muuntaja-yaml/src" - "modules/muuntaja-msgpack/src"] + ;; all module sources for development + :source-paths ["modules/muuntaja-cheshire/src" + "modules/muuntaja-yaml/src" + "modules/muuntaja-msgpack/src"] - :dependencies [[org.clojure/clojure "1.10.1"] - [ring/ring-core "1.7.1"] - [ring-middleware-format "0.7.4"] - [ring-transit "0.1.6"] - [ring/ring-json "0.5.0"] + :dependencies [[org.clojure/clojure "1.10.1"] + [ring/ring-core "1.7.1"] + [ring-middleware-format "0.7.4"] + [ring-transit "0.1.6"] + [ring/ring-json "0.5.0"] - ;; modules - [metosin/muuntaja "0.6.6"] - [metosin/muuntaja-cheshire "0.6.6"] - [metosin/muuntaja-msgpack "0.6.6"] - [metosin/muuntaja-yaml "0.6.6"] + ;; modules + [metosin/muuntaja nil] + [metosin/muuntaja-cheshire nil] + [metosin/muuntaja-msgpack nil] + [metosin/muuntaja-yaml nil] - ;; correct jackson - [com.fasterxml.jackson.core/jackson-databind "2.10.0"] + ;; correct jackson + [com.fasterxml.jackson.core/jackson-databind "2.10.0"] - ;; Sieppari - [metosin/sieppari "0.0.0-alpha5"] + ;; Sieppari + [metosin/sieppari "0.0.0-alpha5"] - ;; Pedestal - [org.clojure/core.async "0.4.500" :exclusions [org.clojure/tools.reader]] - [io.pedestal/pedestal.service "0.5.7" :exclusions [org.clojure/tools.reader - org.clojure/core.async - org.clojure/core.memoize]] - [javax.servlet/javax.servlet-api "4.0.1"] - [org.slf4j/slf4j-log4j12 "1.7.29"] + ;; Pedestal + [org.clojure/core.async "0.4.500" :exclusions [org.clojure/tools.reader]] + [io.pedestal/pedestal.service "0.5.7" :exclusions [org.clojure/tools.reader + org.clojure/core.async + org.clojure/core.memoize]] + [javax.servlet/javax.servlet-api "4.0.1"] + [org.slf4j/slf4j-log4j12 "1.7.29"] - [criterium "0.4.5"]]} - :1.7 {:dependencies [[org.clojure/clojure "1.7.0"]]} - :1.8 {:dependencies [[org.clojure/clojure "1.8.0"]]} - :1.9 {:dependencies [[org.clojure/clojure "1.9.0"]]} - :perf {:jvm-opts ^:replace ["-server" - "-Xmx4096m" - "-Dclojure.compiler.direct-linking=true"]} + [criterium "0.4.5"]]} + :1.7 {:dependencies [[org.clojure/clojure "1.7.0"]]} + :1.8 {:dependencies [[org.clojure/clojure "1.8.0"]]} + :1.9 {:dependencies [[org.clojure/clojure "1.9.0"]]} + :perf {:jvm-opts ^:replace ["-server" + "-Xmx4096m" + "-Dclojure.compiler.direct-linking=true"]} :analyze {:jvm-opts ^:replace ["-server" "-Dclojure.compiler.direct-linking=true" "-XX:+PrintCompilation" "-XX:+UnlockDiagnosticVMOptions" "-XX:+PrintInlining"]}} - :aliases {"all" ["with-profile" "dev:dev,1.7:dev,1.8:dev,1.9"] - "perf" ["with-profile" "default,dev,perf"] + :aliases {"all" ["with-profile" "dev:dev,1.7:dev,1.8:dev,1.9"] + "perf" ["with-profile" "default,dev,perf"] "analyze" ["with-profile" "default,dev,analyze"]}) diff --git a/scripts/set-version b/scripts/set-version deleted file mode 100755 index a14a089..0000000 --- a/scripts/set-version +++ /dev/null @@ -1,8 +0,0 @@ -#!/bin/zsh - -ext="sedbak$$" - -find . -name project.clj -exec sed -i.$ext "s/\[metosin\/muuntaja\(.*\) \".*\"\]/[metosin\/muuntaja\1 \"$1\"\]/g" '{}' \; -find . -name project.clj -exec sed -i.$ext "s/defproject metosin\/muuntaja\(.*\) \".*\"/defproject metosin\/muuntaja\1 \"$1\"/g" '{}' \; -sed -i.$ext "s/\[metosin\/muuntaja\(.*\) \".*\"\]/[metosin\/muuntaja\1 \"$1\"\]/g" **/*.md -find . -name "*.$ext" -exec rm '{}' \; From 9dc3e225d333b424722a7442a0ed6ab690b4c315 Mon Sep 17 00:00:00 2001 From: "Dr. Christian Betz" Date: Tue, 17 Mar 2020 11:26:05 +0100 Subject: [PATCH 5/9] Add ability to test things in Cursive IDE Having the same project coordinates (metosin/muuntaja) for the parent project and a module clashed in Cursive. Also, some tests require jsonista namespaces without stating that dependency, obviously leaving things to implicit dependencies (from module metosin/muuntaja). This makes it hard to start a REPL in Cursive with the proper settings. With this commit, I was able to start a Cursive based REPL where I was able to run both the tests and (re-)load the `muuntaja.core` ns). This increases (my) development speed. --- modules/muuntaja/project.clj | 3 ++- project.clj | 7 +++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/modules/muuntaja/project.clj b/modules/muuntaja/project.clj index c0f8f28..8e48560 100644 --- a/modules/muuntaja/project.clj +++ b/modules/muuntaja/project.clj @@ -13,5 +13,6 @@ leiningen.v/add-workspace-data] :parent-project {:path "../../project.clj" :inherit [:deploy-repositories]} - :dependencies [[metosin/jsonista "0.2.5"] + :dependencies [[org.clojure/clojure "1.10.1"] + [metosin/jsonista "0.2.5"] [com.cognitect/transit-clj "0.8.319"]]) diff --git a/project.clj b/project.clj index e5f2e97..70c1ad7 100644 --- a/project.clj +++ b/project.clj @@ -1,4 +1,4 @@ -(defproject metosin/muuntaja "0.0.0" +(defproject metosin/muuntaja-parent "0.0.0" :description "Clojure library for format encoding, decoding and content-negotiation" :url "https://github.com/metosin/muuntaja" :license {:name "Eclipse Public License" @@ -52,7 +52,10 @@ [javax.servlet/javax.servlet-api "4.0.1"] [org.slf4j/slf4j-log4j12 "1.7.29"] - [criterium "0.4.5"]]} + [criterium "0.4.5"] + + [metosin/jsonista "0.2.5"] + ]} :1.7 {:dependencies [[org.clojure/clojure "1.7.0"]]} :1.8 {:dependencies [[org.clojure/clojure "1.8.0"]]} :1.9 {:dependencies [[org.clojure/clojure "1.9.0"]]} From 48d18da3f437784f937f6538b7eec99ae82b6733 Mon Sep 17 00:00:00 2001 From: "Dr. Christian Betz" Date: Tue, 17 Mar 2020 15:27:45 +0100 Subject: [PATCH 6/9] Fix reading empty bodies If there is nothing to decode, it is okay to return nil. That's not a failure to decode. This problem came up when testing HEAD requests, which are body-less by default. If we still run it through middleware to decode the body, that should not fail. It's not super-elegant, as we already know we do not care for the body anyhow (or at least we shouldn't care), but it should not fail. Also, it might arise in other situations, too. --- modules/muuntaja/src/muuntaja/core.clj | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/modules/muuntaja/src/muuntaja/core.clj b/modules/muuntaja/src/muuntaja/core.clj index c51e1ea..39c714a 100644 --- a/modules/muuntaja/src/muuntaja/core.clj +++ b/modules/muuntaja/src/muuntaja/core.clj @@ -519,8 +519,10 @@ (try (decode (:body response) (:charset res-fc)) (catch Exception e - (fail-on-response-decode-exception m e res-fc response))))) - (fail-on-response-decode m response))))))))) + (fail-on-response-decode-exception m e res-fc response))) + (fail-on-response-decode m response)) + (fail-on-response-decode m response)) + )))))))) (def instance "the default instance" (create)) From 058daec13058fc983408501c84ffe9f43dd10213 Mon Sep 17 00:00:00 2001 From: "Dr. Christian Betz" Date: Thu, 19 Mar 2020 13:36:51 +0100 Subject: [PATCH 7/9] Add request body encoding To use muuntaja on the client side, it is necessary to encode the request body. The functionality was there already, however the code was "asymmetrical" wrt to decoding/encoding. This "duplicates" code with minor changes for encoding/decoding willingly. --- modules/muuntaja/src/muuntaja/core.clj | 73 +++++++++++++++++++++++++- 1 file changed, 72 insertions(+), 1 deletion(-) diff --git a/modules/muuntaja/src/muuntaja/core.clj b/modules/muuntaja/src/muuntaja/core.clj index 39c714a..fa6ddea 100644 --- a/modules/muuntaja/src/muuntaja/core.clj +++ b/modules/muuntaja/src/muuntaja/core.clj @@ -26,6 +26,8 @@ (format-request [this request]) (format-response [this request response]) (^:private -decode-response-body [this response]) + (^:private -encode-request-body [this request]) + (negotiate-and-format-request [this request])) @@ -169,6 +171,20 @@ :response response} e))) +(defn- fail-on-request-encode-exception [m + ^Exception e + ^FormatAndCharset request-format-and-charset + request] + (throw + (ex-info + (str "Malformed " (:format request-format-and-charset) " request.") + {:type :muuntaja/decode + :default-format (default-format m) + :format (:format request-format-and-charset) + :charset (:charset request-format-and-charset) + :request request} + e))) + (defn- fail-on-response-decode [m response] (let [content-type (header "Content-Type" response)] (throw @@ -180,6 +196,17 @@ :default-format (default-format m) :response response})))) +(defn- fail-on-request-encode [m request] + (let [content-type (header "Content-Type" request)] + (throw + (ex-info + (if content-type + (str "Unknown request Content-Type: " content-type) + "No Content-Type found") + {:type :muuntaja/decode + :default-format (default-format m) + :request request})))) + (defn- fail-on-request-charset-negotiation [m] (throw (ex-info @@ -251,6 +278,29 @@ (fail-on-request-charset-negotiation m)) content-type-raw))))) +(defn- -negotiate-content-type-to-encode [m s] + (let [produces (encodes m) + matchers (matchers m) + default-charset (default-charset m) + charsets (charsets m)] + (if s + (let [[content-type-raw charset-raw] (parse/parse-content-type s)] + (->FormatAndCharset + (if content-type-raw + (or (produces content-type-raw) + (some + (fn [[name r]] + (if (and r (re-find r content-type-raw)) name)) + matchers))) + (or + ;; if a provided charset was valid + (and charset-raw charsets (charsets charset-raw) charset-raw) + ;; only set default if none were set + (and (not charset-raw) default-charset) + ;; negotiation failed + (fail-on-request-charset-negotiation m)) + content-type-raw))))) + (defn- -negotiate-accept [m parse s] (let [produces (encodes m) default-format (default-format m) @@ -429,6 +479,7 @@ -parse-accept (parse/fast-memoize 1000 parse/parse-accept) -negotiate-accept (parse/fast-memoize 1000 (partial -negotiate-accept m -parse-accept)) -negotiate-content-type (parse/fast-memoize 1000 (partial -negotiate-content-type m)) + -negotiate-content-type-to-encode (parse/fast-memoize 1000 (partial -negotiate-content-type-to-encode m)) -decode-request-body? (fn [request] (and (not (:body-params request)) (decode-request-body? request))) @@ -522,7 +573,19 @@ (fail-on-response-decode-exception m e res-fc response))) (fail-on-response-decode m response)) (fail-on-response-decode m response)) - )))))))) + )) + (-encode-request-body [this request] + (or + (if-let [res-fc (->> request (header "Content-Type") -negotiate-content-type-to-encode)] + (if-let [encode (encoder this (:format res-fc))] + (try + (encode (:body request) (:charset res-fc)) + (catch Exception e + (fail-on-request-encode-exception m e res-fc request))) + (fail-on-request-encode m request)) + (fail-on-request-encode m request)) + )) + )))))) (def instance "the default instance" (create)) @@ -545,6 +608,14 @@ (encoder data charset) (util/throw! m format "encoder not found for")))) +(defn encode-request-body + "Decode response :body using the format defined by \"Content-Type\" header. + Returns Clojure Data or throws." + ([request] + (-encode-request-body instance request)) + ([m request] + (-encode-request-body m request))) + (defn decode "Decode data into the given format. Returns Clojure Data or throws." ([format data] From d2fa342b72cf393636b6eef423dda2e1f0494806 Mon Sep 17 00:00:00 2001 From: "Dr. Christian Betz" Date: Fri, 20 Mar 2020 16:20:00 +0100 Subject: [PATCH 8/9] Respect the default format --- modules/muuntaja/src/muuntaja/core.clj | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/modules/muuntaja/src/muuntaja/core.clj b/modules/muuntaja/src/muuntaja/core.clj index fa6ddea..7536b26 100644 --- a/modules/muuntaja/src/muuntaja/core.clj +++ b/modules/muuntaja/src/muuntaja/core.clj @@ -565,7 +565,8 @@ $)))) (-decode-response-body [this response] (or - (if-let [res-fc (->> response (header "Content-Type") -negotiate-content-type)] + (if-let [res-fc (-negotiate-content-type (or (header "Content-Type" response) + default-format))] (if-let [decode (decoder this (:format res-fc))] (try (decode (:body response) (:charset res-fc)) @@ -576,7 +577,8 @@ )) (-encode-request-body [this request] (or - (if-let [res-fc (->> request (header "Content-Type") -negotiate-content-type-to-encode)] + (if-let [res-fc (-negotiate-content-type-to-encode (or (header "Content-Type" request) + default-format))] (if-let [encode (encoder this (:format res-fc))] (try (encode (:body request) (:charset res-fc)) From ed234d237a8c4891d9c082c3a59a5b0e205a1cd9 Mon Sep 17 00:00:00 2001 From: "Dr. Christian Betz" Date: Wed, 25 Mar 2020 18:15:28 +0100 Subject: [PATCH 9/9] Renamed header? to header-exists? From the [PR comments by @ikitommi] (https://github.com/metosin/muuntaja/pull/107#discussion_r397647895). --- modules/muuntaja/src/muuntaja/core.clj | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/muuntaja/src/muuntaja/core.clj b/modules/muuntaja/src/muuntaja/core.clj index 7536b26..53c958f 100644 --- a/modules/muuntaja/src/muuntaja/core.clj +++ b/modules/muuntaja/src/muuntaja/core.clj @@ -88,7 +88,7 @@ v)) (:headers request-or-response))) -(defn header? +(defn header-exists? "Returns true iff a specific header is present in the request or response map." [^String header-name request-or-response] @@ -491,7 +491,7 @@ (catch Exception e (fail-on-request-decode-exception m e req-fc res-fc request)))))) -encode-response? (fn [request response] - (and (or (not (header? "Content-Type" response)) + (and (or (not (header-exists? "Content-Type" response)) (:muuntaja/encode response)) (encode-response-body? request response))) -resolve-response-charset (fn [_ request]