Skip to content

Commit 2b2d32a

Browse files
John Bolligerclaude
andcommitted
Cover the same whitespace in both ArgumentKeys character maps
REMOVE_CHARACTERS uses [[:space:]], so it strips a tab and a newline. REPLACE_MAP listed only a plain space, so it left them in the key: remove "a\tb" # => "ab" replace "a\tb" # => "a\tb" tab survives into the key A raw tab or newline in a cache key breaks the Memcached protocol. REPLACE_MAP now covers tab, newline, carriage return, form feed, and vertical tab. It is also a Hash rather than an array of pairs, so #cache_key makes one gsub pass over the string instead of 13. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent ccf3c9f commit 2b2d32a

2 files changed

Lines changed: 51 additions & 18 deletions

File tree

lib/active_remote/cached/argument_keys.rb

Lines changed: 25 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -6,21 +6,29 @@ class ArgumentKeys
66
attr_reader :arguments, :argument_string, :options
77

88
REMOVE_CHARACTERS = /[[:space:]+=><{}\[\];:\-,]/
9-
REPLACE_MAP = [
10-
[' ', 'SP'],
11-
['+', 'PL'],
12-
['=', 'EQ'],
13-
['>', 'GT'],
14-
['<', 'LT'],
15-
['{', 'LB'],
16-
['}', 'RB'],
17-
['[', 'LB2'],
18-
[']', 'RB2'],
19-
[';', 'SC'],
20-
[':', 'CO'],
21-
['-', 'DA'],
22-
[',', 'COM']
23-
].freeze
9+
# Covers the same characters as REMOVE_CHARACTERS. A tab or a newline
10+
# left in a key breaks the Memcached protocol.
11+
REPLACE_MAP = {
12+
' ' => 'SP',
13+
"\t" => 'TB',
14+
"\n" => 'NL',
15+
"\r" => 'CR',
16+
"\f" => 'FF',
17+
"\v" => 'VT',
18+
'+' => 'PL',
19+
'=' => 'EQ',
20+
'>' => 'GT',
21+
'<' => 'LT',
22+
'{' => 'LB',
23+
'}' => 'RB',
24+
'[' => 'LB2',
25+
']' => 'RB2',
26+
';' => 'SC',
27+
':' => 'CO',
28+
'-' => 'DA',
29+
',' => 'COM'
30+
}.freeze
31+
REPLACE_CHARACTERS = ::Regexp.union(REPLACE_MAP.keys)
2432

2533
# The separators below are absent from both REMOVE_CHARACTERS and
2634
# REPLACE_MAP, so they survive either option. escape_value/1 escapes them
@@ -64,9 +72,8 @@ def cache_key
6472
return @argument_string.gsub(REMOVE_CHARACTERS, '') if remove_characters?
6573
return @argument_string unless replace_characters?
6674

67-
REPLACE_MAP.inject(@argument_string) do |key, (character, replacement)|
68-
key.gsub(character, replacement)
69-
end
75+
# One pass, rather than one gsub for each entry in the map.
76+
@argument_string.gsub(REPLACE_CHARACTERS, REPLACE_MAP)
7077
end
7178

7279
def to_s

spec/active_remote/cached/argument_keys_spec.rb

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,32 @@
2525
expect(::ActiveRemote::Cached::ArgumentKeys.new('hello {}', options).cache_key).to eq('helloSPLBRB')
2626
end
2727

28+
it 'replaces a tab when :active_remote_cached_replace_characters' do
29+
options = { :active_remote_cached_replace_characters => true }
30+
expect(::ActiveRemote::Cached::ArgumentKeys.new("a\tb", options).cache_key).to eq('aTBb')
31+
end
32+
33+
it 'replaces a newline when :active_remote_cached_replace_characters' do
34+
options = { :active_remote_cached_replace_characters => true }
35+
expect(::ActiveRemote::Cached::ArgumentKeys.new("a\nb", options).cache_key).to eq('aNLb')
36+
end
37+
38+
# REMOVE_CHARACTERS uses [[:space:]], so both options must cover the same
39+
# whitespace. A raw tab or newline in a key breaks the Memcached protocol.
40+
it 'leaves no whitespace in the key under either option' do
41+
argument = "a b\tc\nd\re\ff\vg"
42+
43+
removed = ::ActiveRemote::Cached::ArgumentKeys.new(
44+
argument, :active_remote_cached_remove_characters => true
45+
).cache_key
46+
replaced = ::ActiveRemote::Cached::ArgumentKeys.new(
47+
argument, :active_remote_cached_replace_characters => true
48+
).cache_key
49+
50+
expect(removed).not_to match(/[[:space:]]/)
51+
expect(replaced).not_to match(/[[:space:]]/)
52+
end
53+
2854
it 'joins multiple arguments into one key' do
2955
expect(::ActiveRemote::Cached::ArgumentKeys.new('hello', 'world', {}).cache_key).to eq('helloworld')
3056
end

0 commit comments

Comments
 (0)