Skip to content

Commit

Permalink
🔧 ResponseParser config is mutable and non-global
Browse files Browse the repository at this point in the history
When ResponseParser was initialized without any explicit config, it used
Config.global directly.  This meant that changes to `parser.config` were
also changes to the _global_ config!

When ResponseParser is initialized with a config hash, that creates
a frozen config object.  This meant that changes to `parser.config` were
impossible.

So, when the given config is global or frozen, we create a new config
and inherit from the given config.  We want to continue using the given
config as-is in other cases, so the client and its parser can share the
same exact config.
  • Loading branch information
nevans committed Jan 23, 2025
1 parent adacca9 commit 340d3e7
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 1 deletion.
6 changes: 5 additions & 1 deletion lib/net/imap/response_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,17 @@ class ResponseParser

attr_reader :config

# :call-seq: Net::IMAP::ResponseParser.new -> Net::IMAP::ResponseParser
# Creates a new ResponseParser.
#
# When +config+ is frozen or global, the parser #config inherits from it.
# Otherwise, +config+ will be used directly.
def initialize(config: Config.global)
@str = nil
@pos = nil
@lex_state = nil
@token = nil
@config = Config[config]
@config = @config.new if @config == Config.global || @config.frozen?
end

# :call-seq:
Expand Down
35 changes: 35 additions & 0 deletions test/net/imap/test_imap_response_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,41 @@ def teardown
# response data, should still use normal tests, below
############################################################################

test "default config inherits from Config.global" do
parser = Net::IMAP::ResponseParser.new
refute parser.config.frozen?
refute_equal Net::IMAP::Config.global, parser.config
assert_same Net::IMAP::Config.global, parser.config.parent
end

test "config can be passed in to #initialize" do
config = Net::IMAP::Config.global.new
parser = Net::IMAP::ResponseParser.new config: config
assert_same config, parser.config
end

test "passing in global config inherits from Config.global" do
parser = Net::IMAP::ResponseParser.new config: Net::IMAP::Config.global
refute parser.config.frozen?
refute_equal Net::IMAP::Config.global, parser.config
assert_same Net::IMAP::Config.global, parser.config.parent
end

test "config will inherits from passed in frozen config" do
parser = Net::IMAP::ResponseParser.new config: {debug: true}
refute_equal Net::IMAP::Config.global, parser.config.parent
refute parser.config.frozen?

assert parser.config.parent.frozen?
assert parser.config.debug?
assert parser.config.inherited?(:debug)

config = Net::IMAP::Config[debug: true]
parser = Net::IMAP::ResponseParser.new(config:)
refute_equal Net::IMAP::Config.global, parser.config.parent
assert_same config, parser.config.parent
end

# Strangly, there are no example responses for BINARY[section] in either
# RFC3516 or RFC9051! The closest I found was RFC5259, and those examples
# aren't FETCH responses.
Expand Down

0 comments on commit 340d3e7

Please sign in to comment.