Skip to content

Commit

Permalink
[mod] Remove nippy/snappy-compressor
Browse files Browse the repository at this point in the history
Details:

  - Nippy will continue to support thawing OLD data that was originally compressed with Snappy.
  - But Nippy will no longer support freezing NEW data with Snappy.

Motivation:

  - The current Snappy implementation can cause JVM crashes in some cases [1].

  - The only alternative JVM implementation that seems to be safe [2] uses JNI and
    so would introduce possible incompatibility issues even for folks not using Snappy.

  - Nippy already moved to the superior LZ4 as its default compression scheme in v2.7.0,
    more than 9 years ago.

[1] Ref. <airlift/aircompressor#183>
[2] Ref. <https://github.com/xerial/snappy-java>
  • Loading branch information
ptaoussanis committed Feb 6, 2024
1 parent 6768984 commit 578c585
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 35 deletions.
4 changes: 1 addition & 3 deletions project.clj
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,7 @@
[[org.clojure/tools.reader "1.3.7"]
[com.taoensso/encore "3.77.0"]
[org.tukaani/xz "1.9"]
[io.airlift/aircompressor "0.25"]
[org.iq80.snappy/snappy "0.4"]
[org.xerial.snappy/snappy-java "1.1.10.5"]]
[io.airlift/aircompressor "0.25"]]

:profiles
{;; :default [:base :system :user :provided :dev]
Expand Down
6 changes: 3 additions & 3 deletions src/taoensso/nippy.clj
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@
compression/lz4-compressor
compression/lz4hc-compressor
#_compression/lzo-compressor
compression/snappy-compressor
#_compression/snappy-compressor ; Can be unsafe
compression/lzma2-compressor

encryption/encrypt
Expand Down Expand Up @@ -1704,7 +1704,7 @@
(defn- get-auto-compressor [compressor-id]
(case compressor-id
nil nil
:snappy snappy-compressor
:snappy compression/snappy-compressor
:lzma2 lzma2-compressor
:lz4 lz4-compressor
:no-header (throw (ex-info ":auto not supported on headerless data." {}))
Expand Down Expand Up @@ -1806,7 +1806,7 @@

(catch Exception e (ex-fn e)))))

;; Hackish + can actually segfault JVM due to Snappy bug,
;; Hacky + can actually segfault JVM due to Snappy bug,
;; Ref. <http://goo.gl/mh7Rpy> - no better alternatives, unfortunately
thaw-v1-data
(fn [data-ba ex-fn]
Expand Down
33 changes: 6 additions & 27 deletions src/taoensso/nippy/compression.clj
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,6 @@
(compress [_ ba] (airlift-compress @airlift-lzo-compressor_ ba true))
(decompress [_ ba] (airlift-decompress @airlift-lzo-decompressor_ ba nil))))

;; Using `io.airlift/aircompressor`, vulnerable to https://github.com/airlift/aircompressor/issues/183
#_
(do
(enc/def* ^:private airlift-snappy-compressor_ (enc/thread-local (io.airlift.compress.snappy.SnappyCompressor.)))
(enc/def* ^:private airlift-snappy-decompressor_ (enc/thread-local (io.airlift.compress.snappy.SnappyDecompressor.)))
Expand All @@ -129,21 +127,6 @@
(when-not prepend-size?
(io.airlift.compress.snappy.SnappyDecompressor/getUncompressedLength ba 0))))))

;; Using `org.iq80.snappy/snappy`, vulnerable to https://github.com/airlift/aircompressor/issues/183
#_
(deftype SnappyCompressor [_]
ICompressor
(header-id [_] :snappy)
(compress [_ ba] (org.iq80.snappy.Snappy/compress ba))
(decompress [_ ba] (org.iq80.snappy.Snappy/uncompress ba 0 (alength ^bytes ba))))

;; Using `org.xerial.snappy/snappy-java`, some compatibility issues due to JNI
(deftype SnappyCompressor [_]
ICompressor
(header-id [_] :snappy)
(compress [_ ba] (org.xerial.snappy.Snappy/compress ba))
(decompress [_ ba] (org.xerial.snappy.Snappy/uncompress ba)))

;;;; LZMA2

(deftype LZMA2Compressor [compression-level]
Expand Down Expand Up @@ -208,16 +191,6 @@
See `taoensso.nippy-benchmarks` for detailed comparative benchmarks."
(LZOCompressor.))

(def snappy-compressor
"Default `Snappy` compressor:
- Compression ratio: `C` (0.58 on reference benchmark).
- Compression speed: `A+` (206 msecs on reference benchmark).
- Decompression speed: `B` (134 msecs on reference benchmark).
Good general-purpose compressor, favours speed.
See `taoensso.nippy-benchmarks` for detailed comparative benchmarks."
(SnappyCompressor. false))

(def lzma2-compressor
"Default `LZMA2` compressor:
- Compression ratio: `A+` (0.4 on reference benchmark).
Expand All @@ -232,3 +205,9 @@
"Different LZ4 modes no longer supported, prefer `lz4-compressor`."
{:deprecated "vX.Y.Z (YYYY-MM-DD)"}
(LZ4Compressor.))

(enc/def* ^:no-doc snappy-compressor
"Snappy compressor no longer recommended, prefer `lz4-compressor`.
Decompression can be unsafe against untrusted data!"
{:deprecated "vX.Y.Z (YYYY-MM-DD)"}
(SnappyCompressor. false))
4 changes: 2 additions & 2 deletions test/taoensso/nippy_tests.clj
Original file line number Diff line number Diff line change
Expand Up @@ -426,13 +426,13 @@
(doseq [c [compr/zstd-compressor
compr/lz4-compressor
compr/lzo-compressor
compr/snappy-compressor
#_compr/snappy-compressor ; Ref. <https://github.com/airlift/aircompressor/issues/183>
compr/lzma2-compressor]]

(dotimes [_ 2e4]
(is
(nil? (enc/catching (compr/decompress c (crypto/rand-bytes 1024))))
"Decompression never core dumps, even against invalid data"))))
"Decompression never crashes JVM, even against invalid data"))))

;;;; Benchmarks

Expand Down

0 comments on commit 578c585

Please sign in to comment.