fix: default the Jackson max-decompressed-size to unlimited - #3515
Conversation
Motivation: A bounded default could reject a payload an existing system legitimately exchanges, so a patch release carrying the 256 MiB default from apache#3491 could break running systems on upgrade. The bound should be opt-in, matching the change made to pekko.serialization.max-decompressed-size in apache#3502. Modification: Default pekko.serialization.jackson.compression.max-decompressed-size (and the jackson3 equivalent) to -1, meaning no limit and matching the behaviour of releases before apache#3491. A negative maximum skips the gzip size check and the LZ4 declared-size check; a negative declared LZ4 size is still rejected, since it is malformed regardless of the limit. Config's getBytes refuses negative numbers, so the setting is read as a plain long first and as a memory size only when that is not a negative number. Result: Jackson payload decompression is unbounded by default; configuring a size such as 256 MiB bounds it. Tests: - sbt "serialization-jackson/testOnly org.apache.pekko.serialization.jackson.*" - 122 passed - sbt "serialization-jackson3/testOnly org.apache.pekko.serialization.jackson3.*" - 120 passed - sbt "serialization-jackson/scalafmtCheckAll" "serialization-jackson3/scalafmtCheckAll" - clean - sbt "serialization-jackson/mimaReportBinaryIssues" - no issues References: Refs apache#3491, Refs apache#3502
|
I want to backport #3491 with this PR's modification as part of the 1.7.1 release. The limit becomes opt-in as a result. |
| # The default of -1 applies no limit, preserving the behaviour of earlier | ||
| # releases; set a size such as `256 MiB` to bound decompression, choosing a | ||
| # value larger than any payload the system legitimately exchanges. | ||
| max-decompressed-size = -1 |
There was a problem hiding this comment.
Pekko overall is a bit inconsistent in this, some settings use -1 for unlimited, some settings use
max-received-message-size = unlimited
In general I find such keys more clear than using magic numbers like -1 or 0 that change the meaning of the setting.
Maybe something we can address in 2.0
There was a problem hiding this comment.
Good point. Changed so that both spellings work: the default is now written as unlimited, and a negative number such as -1 is also accepted (matching the neighbouring read.max-document-length / read.max-token-count convention). The same dual handling is applied to pekko.serialization.max-decompressed-size in #3502 so the two sibling settings stay consistent.
Motivation: Review on apache#3515 noted that Pekko is inconsistent about unlimited spellings and an explicit keyword is clearer than a magic number, while the neighbouring read.max-document-length and read.max-token-count settings use -1. Accept both. Modification: The setting reads "unlimited" or any negative number as no limit; the reference.conf default is written as `unlimited`. Applied to both serialization-jackson and serialization-jackson3, with a test each for the keyword. Result: `max-decompressed-size = unlimited` and `= -1` both disable the bound. Tests: - sbt "serialization-jackson/testOnly org.apache.pekko.serialization.jackson.*" - 123 passed - sbt "serialization-jackson3/testOnly org.apache.pekko.serialization.jackson3.*" - 121 passed - sbt "serialization-jackson/scalafmtCheckAll" "serialization-jackson3/scalafmtCheckAll" - clean References: Refs apache#3515
Motivation: Review on apache#3515 noted that an explicit keyword is clearer than a magic number. Keep the two sibling settings consistent: accept both spellings here as well. Modification: pekko.serialization.max-decompressed-size reads "unlimited" or any negative number as no limit; the reference.conf default is written as `unlimited`. New tests cover the keyword default and an explicit -1. Result: `max-decompressed-size = unlimited` and `= -1` both disable the bound. Tests: - sbt "actor-tests/testOnly org.apache.pekko.serialization.DecompressionSpec" - 9 passed - sbt "actor/scalafmtCheckAll" "actor-tests/scalafmtCheckAll" - clean References: Refs apache#3515, Refs apache#3502
Motivation
#3491 bounded Jackson payload decompression with
pekko.serialization.jackson.compression.max-decompressed-size, defaulting to 256 MiB. Abounded default could reject a payload an existing system legitimately exchanges, so a
patch release carrying it could break running systems on upgrade, and there is no default
that is provably above every deployment's largest payload. The bound should be opt-in,
matching the change made to
pekko.serialization.max-decompressed-sizein #3502.Modification
Default the setting to
unlimited, meaning no limit and matching the behaviour ofreleases before #3491, in both
serialization-jacksonandserialization-jackson3. Anegative number such as
-1also means unlimited — the convention of the neighbouringread.max-document-lengthandread.max-token-countsettings — per review, so bothspellings are accepted. Operators who want the protection set a size such as
256 MiB,larger than anything their system legitimately sends.
An unlimited maximum skips the gzip size check and the LZ4 declared-size check. A negative
declared LZ4 size is still rejected — it is malformed regardless of the limit — and now
with a message saying that, rather than one claiming it exceeds the maximum.
Config's
getBytesrefuses both spellings, so the setting is read as a string first andas a memory size only when it is neither
unlimitednor a negative number.Result
Jackson payload decompression is unbounded by default; configuring a size bounds it. A
configured limit behaves exactly as before.
Tests
Three new tests per module (so each runs under both the JSON and CBOR serializers):
apply no gzip decompression limit when max-decompressed-size is -1apply no lz4 decompression limit when max-decompressed-size is -1apply no decompression limit when max-decompressed-size is unlimitedThe
-1tests were checked to discriminate by reverting the production change andre-running: each then fails with
ConfigException$BadValue: Attempt to construct memory size with negative number: -1, which is also what an operator configuring-1on thecurrent code would get at serializer construction;
unlimitedfails the same way as aBadValueon the keyword.sbt "serialization-jackson/testOnly org.apache.pekko.serialization.jackson.*"— passedsbt "serialization-jackson3/testOnly org.apache.pekko.serialization.jackson3.*"— passedsbt "serialization-jackson/scalafmtCheckAll" "serialization-jackson3/scalafmtCheckAll"— cleansbt "serialization-jackson/mimaReportBinaryIssues"— no issues (serialization-jackson3disables MimaPlugin)
References
Refs #3491, Refs #3502