Fix core dump issue (important), simplify API

PROBLEM: :legacy-mode :auto/true thawing was resulting in JVM core
dumps when attempting to use Snappy to decompress encrypted data.

CAUSE: The org.iq80.snappy implementation seems to choke on the
random IV byte data being generated by the AES128 encrypter. This
may or may not be a bug (still awaiting feedback from lib's authors).

SOLUTION: We're only susceptible to this issue when trying to
decompress data that is: a) encrypted, b) being thawed in legacy mode.
In particular, we're _not_ susceptible to this issue when thawing
in non-legacy mode because in that case we have a header explicitly
warning us that the data is encrypted.

An obvious work-around, therefore, is just to disable decryption when
attempting to thaw legacy-mode data. In practice this isn't a problem
because older versions of Nippy didn't support encryption anyway.

README.md

@@ -2,7 +2,7 @@ Current [semantic](http://semver.org/) version:
 
 ```clojure
 [com.taoensso/nippy "1.2.1"]        ; Stable
-[com.taoensso/nippy "2.0.0-alpha6"] ; Development (notes below)
+[com.taoensso/nippy "2.0.0-alpha7"] ; Development (notes below)
 ```
 
 2.x adds pluggable compression, crypto support (also pluggable), an improved API (including much better error messages), and hugely improved performance. It **is backwards compatible**, but please note that the old `freeze-to-bytes`/`thaw-from-bytes` API has been **deprecated** in favor of `freeze`/`thaw`. **PLEASE REPORT ANY PROBLEMS!**

project.clj

@@ -1,4 +1,4 @@
-(defproject com.taoensso/nippy "2.0.0-alpha6"
+(defproject com.taoensso/nippy "2.0.0-alpha7"
   :description "Clojure serialization library"
   :url "https://github.com/ptaoussanis/nippy"
   :license {:name "Eclipse Public License"

src/taoensso/nippy.clj

@@ -13,7 +13,6 @@
              IPersistentSet IPersistentCollection]))
 
 ;;;; Nippy 2.x+ header spec (4 bytes)
-
 (def ^:private ^:const head-version 1)
 (def ^:private head-sig (.getBytes "NPY" "UTF-8"))
 (def ^:private head-meta "Final byte stores version-dependent metadata."
@@ -162,6 +161,8 @@
 (comment (wrap-header (.getBytes "foo") {:compressed? true
                                          :encrypted?  false}))
 
+(declare assert-legacy-args)
+
 (defn freeze
   "Serializes arg (any Clojure data type) to a byte array. Set :legacy-mode to
   true to produce bytes readble by Nippy < 2.x."
@@ -169,7 +170,7 @@
                 :or   {print-dup? true
                        compressor snappy-compressor
                        encryptor  aes128-encryptor}}]]
-
+  (when legacy-mode (assert-legacy-args compressor password))
   (let [ba     (ByteArrayOutputStream.)
         stream (DataOutputStream. ba)]
     (binding [*print-dup* print-dup?] (freeze-to-stream x stream))
@@ -259,88 +260,56 @@
       (when (utils/ba= head-sig* head-sig)
         [data-ba (head-meta meta-id {:unrecognized-header? true})]))))
 
-
-(defn throw-thaw-ex [msg & [e]] (throw (Exception. (str "Thaw failed: " msg) e)))
-
 (defn thaw
-  "Deserializes frozen bytes to their original Clojure data type.
+  "Deserializes frozen bytes to their original Clojure data type. Supports data
-
+  frozen with current and all previous versions of Nippy.
-  :legacy-mode options:
-    false - Nippy >= 2.x data only (best).
-    true  - Nippy  < 2.x data only (deprecated).
-    :auto - Mixed data (default, migrating).
-
-  In most cases you'll want :auto if you're using a preexisting data set, and
-  `false` otherwise.
 
   WARNING: Enabling `:read-eval?` can lead to security vulnerabilities unless
   you are sure you know what you're doing."
-  [^bytes ba & [{:keys [read-eval? password compressor encryptor legacy-mode
+  [^bytes ba & [{:keys [read-eval? password compressor encryptor legacy-opts]
-                        strict?]
+                 :or   {legacy-opts {:compressed? true}
-                 :or   {legacy-mode :auto
                         compressor  snappy-compressor
                         encryptor   aes128-encryptor}}]]
 
-  (let [try-thaw-data
+  (let [ex (fn [msg & [e]] (throw (Exception. (str "Thaw failed: " msg) e)))
-        (fn [data-ba {decompress? :compressed? decrypt? :encrypted?
+        try-thaw-data
-                     :or {decompress? compressor
+        (fn [data-ba {:keys [compressed? encrypted?] :as head-meta}]
-                          decrypt?    password}
+          (let [password   (when encrypted? password) ; => also head-meta
-                     :as head-meta}]
+                compressor (if head-meta
-          (let [apparent-header? (not (empty? head-meta))]
+                             (when compressed? compressor)
+                             (when (:compressed? legacy-opts) snappy-compressor))]
             (try
               (let [ba data-ba
-                    ba (if decrypt? (encryption/decrypt encryptor password ba) ba)
+                    ba (if password (encryption/decrypt encryptor password ba) ba)
-                    ba (if decompress? (compression/decompress compressor ba) ba)
+                    ba (if compressor (compression/decompress compressor ba) ba)
                     stream (DataInputStream. (ByteArrayInputStream. ba))]
                 (binding [*read-eval* read-eval?] (thaw-from-stream stream)))
               (catch Exception e
                 (cond
-                 decrypt?    (throw-thaw-ex "Wrong password/encryptor?" e)
+                 password   (ex "Wrong password/encryptor?" e)
-                 decompress? (throw-thaw-ex "Encrypted data or wrong compressor?" e)
+                 compressor (if head-meta (ex "Encrypted data or wrong compressor?" e)
-                 :else
+                                          (ex "Uncompressed data?" e))
-                 (if apparent-header?
+                 :else      (if head-meta (ex "Corrupt data?" e)
-                   (throw-thaw-ex "Corrupt data?" e)
+                                          (ex "Compressed data?" e)))))))]
-                   (throw-thaw-ex "Encrypted and/or compressed data?" e)))))))]
 
-    (if (= legacy-mode true)
-      (try-thaw-data ba nil)
     (if-let [[data-ba {:keys [unrecognized-header? compressed? encrypted?]
                        :as   head-meta}] (try-parse-header ba)]
-        (if (= legacy-mode :auto)
-          (try
-            ;; Header seems okay, but we won't trust its metadata for
-            ;; error-reporting purposes
-            (try-thaw-data data-ba head-meta)
-            (catch Exception _ (try-thaw-data ba nil)))
 
-          (cond ; Trust metadata, give fancy error messages
+      (cond ; Header appears okay
-           unrecognized-header?
+       (and (not legacy-opts) unrecognized-header?) ; Conservative
-           (throw-thaw-ex
+       (ex "Unrecognized header. Data frozen with newer Nippy version?")
-            "Unrecognized header. Data frozen with newer Nippy version?")
-           (and strict? (not encrypted?) password)
-           (throw-thaw-ex (str "Unencrypted data. Try again w/o password.\n"
-                               "Disable `:strict?` option to ignore this error. "))
-           (and strict? (not compressed?) compressor)
-           (throw-thaw-ex (str "Uncompressed data. Try again w/o compressor.\n"
-                               "Disable `:strict?` option to ignore this error."))
        (and compressed? (not compressor))
-           (throw-thaw-ex "Compressed data. Try again with compressor.")
+       (ex "Compressed data. Try again with compressor.")
        (and encrypted? (not password))
-           (throw-thaw-ex "Encrypted data. Try again with password.")
+       (ex "Encrypted data. Try again with password.")
-           :else (try-thaw-data data-ba head-meta)))
+       :else (try (try-thaw-data data-ba head-meta)
+                  (catch Exception _ (try-thaw-data ba nil))))
 
       ;; Header definitely not okay
-        (if (= legacy-mode :auto)
+      (try-thaw-data ba nil))))
-          (try-thaw-data ba nil) ; Legacy thaw
-          (throw-thaw-ex
-           (str "Not Nippy data, data frozen with Nippy < 2.x, "
-                "or corrupt data?\n"
-                "See `:legacy-mode` option for data frozen with Nippy < 2.x.")))))))
 
 (comment (thaw (freeze "hello"))
          (thaw (freeze "hello" {:compressor nil}))
-         (thaw (freeze "hello" {:compressor nil}) {:legacy-mode false
-                                                   :strict?     true}) ; ex
          (thaw (freeze "hello" {:password [:salted "p"]})) ; ex
          (thaw (freeze "hello") {:password [:salted "p"]}))
 
@@ -394,21 +363,24 @@
 
 ;;;; Deprecated API
 
+(defn- assert-legacy-args [compressor password]
+  (when password
+    (throw (AssertionError. "Encryption not supported in legacy mode.")))
+  (when (and compressor (not= compressor snappy-compressor))
+    (throw (AssertionError. "Only Snappy compressor supported in legacy mode."))))
+
 (defn freeze-to-bytes "DEPRECATED: Use `freeze` instead."
-  ^bytes [x & {:keys [print-dup? compress? password]
+  ^bytes [x & {:keys [print-dup? compress?]
                :or   {print-dup? true
                       compress?  true}}]
-  (freeze x {:print-dup?   print-dup?
+  (freeze x {:legacy-mode  true
+             :print-dup?   print-dup?
              :compressor   (when compress? snappy-compressor)
-             :encryptor    nil
+             :password     nil}))
-             :password     password
-             :legacy-mode  true}))
 
 (defn thaw-from-bytes "DEPRECATED: Use `thaw` instead."
-  [ba & {:keys [read-eval? compressed? password]
+  [ba & {:keys [read-eval? compressed?]
          :or   {compressed? true}}]
-  (thaw ba {:read-eval?   read-eval?
+  (thaw ba {:legacy-opts  {:compressed? compressed?}
-            :compressor   (when compressed? snappy-compressor)
+            :read-eval?   read-eval?
-            :encryptor    nil
+            :password     nil}))
-            :password     password
-            :legacy-mode  true}))

src/taoensso/nippy/encryption.clj

@@ -51,7 +51,7 @@
 (defn- destructure-typed-pwd
   [typed-password]
   (letfn [(throw-ex []
-            (throw (Exception.
+            (throw (AssertionError.
               (str "Expected password form: "
                    "[<#{:salted :cached}> <password-string>].\n "
                    "See `default-aes128-encryptor` docstring for details!"))))]

test/taoensso/nippy/tests/main.clj

@@ -6,13 +6,17 @@
 ;; Remove stuff from stress-data that breaks roundtrip equality
 (def test-data (dissoc nippy/stress-data :bytes))
 
-;;;; Basic data integrity
 (expect test-data ((comp thaw freeze) test-data))
 (expect test-data ((comp thaw #(freeze % {:legacy-mode true})) test-data))
 (expect test-data ((comp #(thaw   % {:password [:salted "p"]})
                          #(freeze % {:password [:salted "p"]}))
                    test-data))
 
+(expect AssertionError (thaw (freeze test-data {:password "malformed"})))
+(expect Exception (thaw (freeze test-data {:password [:salted "p"]})))
+(expect Exception (thaw (freeze test-data {:password [:salted "p"]})
+                        {:compressor nil}))
+
 (expect ; Snappy lib compatibility (for legacy versions of Nippy)
  (let [^bytes raw-ba    (freeze test-data {:compressor nil})
        ^bytes xerial-ba (org.xerial.snappy.Snappy/compress raw-ba)
@@ -23,24 +27,4 @@
       (thaw (org.iq80.snappy.Snappy/uncompress   iq80-ba    0 (alength iq80-ba)))
       (thaw (org.iq80.snappy.Snappy/uncompress   xerial-ba  0 (alength xerial-ba))))))
 
-;;;; API stuff
-
-;;; Strict/auto mode - compression
-(expect test-data (thaw (freeze test-data {:compressor nil})))
-(expect Exception (thaw (freeze test-data {:compressor nil})
-                        {:legacy-mode false
-                         :strict?     true}))
-
-;;; Strict/auto mode - encryption
-(expect test-data (thaw (freeze test-data) {:password [:salted "p"]}))
-(expect Exception (thaw (freeze test-data) {:password [:salted "p"]
-                                            :legacy-mode false
-                                            :strict?     true}))
-
-;;; Encryption - passwords
-(expect Exception (thaw (freeze test-data {:password "malformed"})))
-(expect Exception (thaw (freeze test-data {:password [:salted "p"]})))
-(expect test-data (thaw (freeze test-data {:password [:salted "p"]})
-                        {:password [:salted "p"]}))
-
 (expect (benchmarks/autobench)) ; Also tests :cached passwords