Skip to content

Commit 5723106

Browse files
committed
Add File#close to prevent file handle leaks
When FormData::File is created from a String path or Pathname, the initializer opens a file handle that was never closed. Add a #close method that closes the underlying IO when it was opened internally. When created from a caller-provided IO, close is a no-op. Closes #27.
1 parent 65bac9f commit 5723106

4 files changed

Lines changed: 77 additions & 0 deletions

File tree

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
99

1010
### Added
1111

12+
- `FormData::File#close` for closing file handles opened from String paths or
13+
Pathnames. When a File is created from an existing IO, `close` is a no-op.
14+
([#27](https://github.com/httprb/form_data/issues/27))
1215
- Accept any Enumerable (not just Hash or Array) as form data input for both
1316
`Multipart` and `Urlencoded` encoders. This enables lazy enumerators and
1417
custom collections as input.

lib/http/form_data/file.rb

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,29 @@ def initialize(path_or_io, opts = nil) # rubocop:disable Lint/MissingSuper
5555
end
5656

5757
@io = make_io(path_or_io)
58+
@autoclose = path_or_io.is_a?(String) || path_or_io.is_a?(Pathname)
5859
@content_type = opts.fetch(:content_type, DEFAULT_MIME).to_s
5960
@filename = opts.fetch(:filename, filename_for(@io))
6061
end
6162

63+
# Closes the underlying IO if it was opened by this instance
64+
#
65+
# When the File was created from a String path or Pathname, the
66+
# underlying file handle is closed. When created from an existing
67+
# IO object, this is a no-op (the caller is responsible for
68+
# closing it).
69+
#
70+
# @example
71+
# file = FormData::File.new("/path/to/file.txt")
72+
# file.to_s
73+
# file.close
74+
#
75+
# @api public
76+
# @return [void]
77+
def close
78+
@io.close if @autoclose
79+
end
80+
6281
private
6382

6483
# Wraps path_or_io into an IO object

sig/http/form_data/file.rbs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,17 @@ module HTTP
33
class File < Part
44
DEFAULT_MIME: String
55

6+
@autoclose: bool
7+
68
# Deprecated alias for content_type
79
alias mime_type content_type
810

911
# Creates a new File from a path or IO object
1012
def initialize: (String | Pathname | untyped path_or_io, ?Hash[Symbol, untyped]? opts) -> void
1113

14+
# Closes the underlying IO if it was opened by this instance
15+
def close: () -> void
16+
1217
private
1318

1419
# Wraps path_or_io into an IO object

test/http/form_data/file_test.rb

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -424,4 +424,54 @@ def test_read_after_eof_returns_empty_string_without_length
424424

425425
assert_equal "", form_file.read
426426
end
427+
428+
# --- File#close ---
429+
430+
def test_close_with_string_path_closes_io
431+
form_file = HTTP::FormData::File.new(fixture("the-http-gem.info").to_s)
432+
form_file.read
433+
form_file.close
434+
435+
assert_raises(IOError) { form_file.read }
436+
end
437+
438+
def test_close_with_pathname_closes_io
439+
form_file = HTTP::FormData::File.new(fixture("the-http-gem.info"))
440+
form_file.read
441+
form_file.close
442+
443+
assert_raises(IOError) { form_file.read }
444+
end
445+
446+
def test_close_with_io_does_not_close
447+
io = StringIO.new("hello")
448+
form_file = HTTP::FormData::File.new(io)
449+
form_file.close
450+
451+
assert_equal "hello", io.read
452+
end
453+
454+
def test_close_is_idempotent
455+
form_file = HTTP::FormData::File.new(fixture("the-http-gem.info").to_s)
456+
form_file.close
457+
form_file.close
458+
end
459+
460+
def test_close_with_string_subclass_closes_io
461+
str_subclass = Class.new(String)
462+
form_file = HTTP::FormData::File.new(str_subclass.new(fixture("the-http-gem.info").to_s))
463+
form_file.read
464+
form_file.close
465+
466+
assert_raises(IOError) { form_file.read }
467+
end
468+
469+
def test_close_with_pathname_subclass_closes_io
470+
pathname_subclass = Class.new(Pathname)
471+
form_file = HTTP::FormData::File.new(pathname_subclass.new(fixture("the-http-gem.info").to_s))
472+
form_file.read
473+
form_file.close
474+
475+
assert_raises(IOError) { form_file.read }
476+
end
427477
end

0 commit comments

Comments
 (0)