Skip to content

Avoid mutating global STDOUT & STDERR (#1837) #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
MitchLewis930 wants to merge 1 commit into
base: pr_051_before
Choose a base branch
from
Open

Avoid mutating global STDOUT & STDERR (#1837) #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions lib/puma/events.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ def call(str)
#
def initialize(stdout, stderr)
@formatter = DefaultFormatter.new
@stdout = stdout
@stderr = stderr
@stdout = stdout.dup
@stderr = stderr.dup

@stdout.sync = true
@stderr.sync = true
Expand Down
14 changes: 11 additions & 3 deletions test/test_events.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ def test_null

assert_instance_of Puma::NullIO, events.stdout
assert_instance_of Puma::NullIO, events.stderr
assert_equal events.stdout, events.stderr
end

def test_strings
Expand All @@ -19,8 +18,17 @@ def test_strings
def test_stdio
events = Puma::Events.stdio

assert_equal STDOUT, events.stdout
assert_equal STDERR, events.stderr
# events.stdout is a dup, so same file handle, different ruby object, but inspect should show the same file handle
assert_equal STDOUT.inspect, events.stdout.inspect
assert_equal STDERR.inspect, events.stderr.inspect
Copy link

@cursor cursor bot Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test compares inspect strings that have different formats

Medium Severity

The test_stdio test compares STDOUT.inspect with events.stdout.inspect, but events.stdout is a dup of $stdout. In Ruby, STDOUT.inspect typically returns #<IO:<STDOUT>> while STDOUT.dup.inspect returns #<IO:fd 1> - different string formats that won't be equal. The test's approach of comparing inspect strings is unreliable for verifying the same file handle; comparing fileno values would be more robust.

Fix in Cursor Fix in Web

end

def test_stdio_respects_sync
STDOUT.sync = false
events = Puma::Events.stdio

assert !STDOUT.sync
assert events.stdout.sync
Copy link

@cursor cursor bot Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test does not restore STDOUT.sync global state

Low Severity

The new test_stdio_respects_sync test sets STDOUT.sync = false but never restores it to its original value. Other tests in this file use ensure blocks to restore global state after modification (e.g., ENV["PUMA_DEBUG"]). This missing cleanup causes test pollution and could lead to flaky behavior in other tests that depend on STDOUT.sync being in its default state.

Fix in Cursor Fix in Web

end

def test_register_callback_with_block
Expand Down
1 change: 1 addition & 0 deletions test/test_tcp_logger.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ def test_events
# in lib/puma/launcher.rb:85
# Puma::Events is default tcp_logger for cluster mode
logger = Puma::Events.new(STDOUT, STDERR)
logger.instance_variable_set(:@stdout, $stdout) # ensure capture_process_io has access to the loggers output
out, err = capture_subprocess_io do
Puma::TCPLogger.new(logger, @server.app).call({}, @socket)
end
Expand Down