Skip to content

Commit 3ef4394

Browse files
Enforce string-only values in raw mode (#1022)
When using `raw: true`, only String values are now accepted. Previously, non-string values were silently converted via `to_s`, causing issues like `nil` becoming `""` and integers losing their type information. This matches the behavior of client-level `raw: true` mode (StringMarshaller) and aligns with the expectation that raw mode means "pass-through, no magic". Breaking change: `set(key, nil, raw: true)` and `set(key, 123, raw: true)` now raise MarshalError. Use string values for counters: `set('counter', '0', raw: true)`. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent bf2fbb3 commit 3ef4394

File tree

5 files changed

+114
-24
lines changed

5 files changed

+114
-24
lines changed

CHANGELOG.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,16 @@ Bug Fixes:
1717
- Detects correct pack format for time_t and suseconds_t on each platform
1818
- Fixes timeout issues on architectures with 64-bit time_t
1919

20+
- Fix get_multi hanging with large key counts (#776, #941)
21+
- Add interleaved read/write for pipelined gets to prevent socket buffer deadlock
22+
- For batches over 10,000 keys per server, requests are now sent in chunks
23+
24+
- **Breaking:** Enforce string-only values in raw mode (#1022)
25+
- `set(key, nil, raw: true)` now raises `MarshalError` instead of storing `""`
26+
- `set(key, 123, raw: true)` now raises `MarshalError` instead of storing `"123"`
27+
- This matches the behavior of client-level `raw: true` mode
28+
- To store counters, use string values: `set('counter', '0', raw: true)`
29+
2030
4.2.0
2131
==========
2232

lib/dalli/protocol/value_serializer.rb

Lines changed: 26 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -32,22 +32,8 @@ def initialize(protocol_options)
3232
end
3333

3434
def store(value, req_options, bitflags)
35-
if req_options
36-
return [value.to_s, bitflags] if req_options[:raw]
37-
38-
# If the value is a simple string, going through serialization is costly
39-
# for no benefit other than preserving encoding.
40-
# Assuming most strings are either UTF-8 or BINARY we can just store
41-
# that information in the bitflags.
42-
if req_options[:string_fastpath] && value.instance_of?(String)
43-
case value.encoding
44-
when Encoding::BINARY
45-
return [value, bitflags]
46-
when Encoding::UTF_8
47-
return [value, bitflags | FLAG_UTF8]
48-
end
49-
end
50-
end
35+
return store_raw(value, bitflags) if req_options&.dig(:raw)
36+
return store_string_fastpath(value, bitflags) if use_string_fastpath?(value, req_options)
5137

5238
[serialize_value(value), bitflags | FLAG_SERIALIZED]
5339
end
@@ -85,6 +71,30 @@ def serialize_value(value)
8571

8672
private
8773

74+
def store_raw(value, bitflags)
75+
unless value.is_a?(String)
76+
raise Dalli::MarshalError, "Dalli raw mode requires string values, got: #{value.class}"
77+
end
78+
79+
[value, bitflags]
80+
end
81+
82+
# If the value is a simple string, going through serialization is costly
83+
# for no benefit other than preserving encoding.
84+
# Assuming most strings are either UTF-8 or BINARY we can just store
85+
# that information in the bitflags.
86+
def store_string_fastpath(value, bitflags)
87+
case value.encoding
88+
when Encoding::BINARY then [value, bitflags]
89+
when Encoding::UTF_8 then [value, bitflags | FLAG_UTF8]
90+
else [serialize_value(value), bitflags | FLAG_SERIALIZED]
91+
end
92+
end
93+
94+
def use_string_fastpath?(value, req_options)
95+
req_options&.dig(:string_fastpath) && value.instance_of?(String)
96+
end
97+
8898
def warn_if_marshal_default(protocol_options)
8999
return if protocol_options.key?(:serializer)
90100
return if @@marshal_warning_logged

test/integration/test_operations.rb

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,39 @@
5555
end
5656
end
5757

58+
describe 'raw mode validation' do
59+
it 'raises MarshalError when setting nil with raw: true' do
60+
memcached_persistent(p) do |dc|
61+
error = assert_raises Dalli::MarshalError do
62+
dc.set('rawkey', nil, 0, raw: true)
63+
end
64+
65+
assert_match(/raw mode requires string values/, error.message)
66+
assert_match(/NilClass/, error.message)
67+
end
68+
end
69+
70+
it 'raises MarshalError when setting non-string with raw: true' do
71+
memcached_persistent(p) do |dc|
72+
error = assert_raises Dalli::MarshalError do
73+
dc.set('rawkey', 123, 0, raw: true)
74+
end
75+
76+
assert_match(/raw mode requires string values/, error.message)
77+
assert_match(/Integer/, error.message)
78+
end
79+
end
80+
81+
it 'allows setting strings with raw: true' do
82+
memcached_persistent(p) do |dc|
83+
dc.flush
84+
85+
assert op_addset_succeeds(dc.set('rawkey', 'string_value', 0, raw: true))
86+
assert_equal 'string_value', dc.get('rawkey', raw: true)
87+
end
88+
end
89+
end
90+
5891
describe 'gat' do
5992
it 'returns the value and touches on a hit' do
6093
memcached_persistent(p) do |dc|
@@ -265,7 +298,7 @@
265298
memcached_persistent(p) do |client|
266299
client.flush
267300

268-
assert op_addset_succeeds(client.set('fakecounter', 0, 0, raw: true))
301+
assert op_addset_succeeds(client.set('fakecounter', '0', 0, raw: true))
269302
assert_equal 1, client.incr('fakecounter', 1)
270303
assert_equal 2, client.incr('fakecounter', 1)
271304
assert_equal 3, client.incr('fakecounter', 1)

test/integration/test_quiet.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@
158158
dc.close
159159
dc.flush
160160
key = SecureRandom.hex(3)
161-
value = 546
161+
value = '546'
162162
incr = 134
163163
dc.set(key, value, 90, raw: true)
164164

@@ -181,7 +181,7 @@
181181
dc.close
182182
dc.flush
183183
key = SecureRandom.hex(3)
184-
value = 546
184+
value = '546'
185185
incr = 134
186186
dc.set(key, value, 90, raw: true)
187187

test/protocol/test_value_serializer.rb

Lines changed: 42 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -146,12 +146,49 @@
146146
describe 'when the request options value for the :raw key is true' do
147147
let(:req_options) { { raw: true } }
148148

149-
it 'does not call the serializer and just converts the input value to a string' do
150-
val, newbitflags = vs.store(raw_value, req_options, bitflags)
149+
describe 'with a string value' do
150+
let(:string_value) { 'test string value' }
151151

152-
assert_equal val, raw_value.to_s
153-
assert_equal newbitflags, bitflags
154-
serializer.verify
152+
it 'returns the string without serialization' do
153+
val, newbitflags = vs.store(string_value, req_options, bitflags)
154+
155+
assert_equal string_value, val
156+
assert_equal bitflags, newbitflags
157+
serializer.verify
158+
end
159+
end
160+
161+
describe 'with a nil value' do
162+
it 'raises MarshalError' do
163+
error = assert_raises Dalli::MarshalError do
164+
vs.store(nil, req_options, bitflags)
165+
end
166+
167+
assert_match(/raw mode requires string values/, error.message)
168+
assert_match(/NilClass/, error.message)
169+
end
170+
end
171+
172+
describe 'with a non-string value' do
173+
it 'raises MarshalError' do
174+
error = assert_raises Dalli::MarshalError do
175+
vs.store(raw_value, req_options, bitflags)
176+
end
177+
178+
assert_match(/raw mode requires string values/, error.message)
179+
assert_match(/Object/, error.message)
180+
end
181+
end
182+
183+
describe 'with an integer value' do
184+
it 'raises MarshalError' do
185+
error = assert_raises Dalli::MarshalError do
186+
vs.store(123, req_options, bitflags)
187+
end
188+
189+
assert_match(/raw mode requires string values/, error.message)
190+
assert_match(/Integer/, error.message)
191+
end
155192
end
156193
end
157194

0 commit comments

Comments
 (0)