From 3086cd539495af24ef26c0369995827d499033b2 Mon Sep 17 00:00:00 2001 From: Morax Date: Fri, 7 Aug 2026 09:34:28 +0200 Subject: [PATCH] Fix input file descriptor leak --- acceptance_tests/file-descriptors.sh | 32 +++++++++++++++++++++++++ pkg/yqlib/all_at_once_evaluator.go | 17 ++++++++----- pkg/yqlib/all_at_once_evaluator_test.go | 21 ++++++++++++++++ pkg/yqlib/file_utils.go | 4 ++++ pkg/yqlib/front_matter_test.go | 1 + pkg/yqlib/stream_evaluator.go | 23 +++++++++--------- pkg/yqlib/utils.go | 30 +++++++++++------------ 7 files changed, 94 insertions(+), 34 deletions(-) create mode 100755 acceptance_tests/file-descriptors.sh diff --git a/acceptance_tests/file-descriptors.sh b/acceptance_tests/file-descriptors.sh new file mode 100755 index 0000000000..78a20898b8 --- /dev/null +++ b/acceptance_tests/file-descriptors.sh @@ -0,0 +1,32 @@ +#!/bin/bash + +setUp() { + rm test-fd-*.yml 2>/dev/null || true +} + +tearDown() { + rm test-fd-*.yml 2>/dev/null || true +} + +createInputFiles() { + local index + for index in {1..80}; do + ln -s examples/sample.yaml "test-fd-${index}.yml" + done +} + +testEvalClosesInputFiles() { + createInputFiles + + (ulimit -n 32; GOGC=off ./yq eval 'select(false)' test-fd-*.yml) + assertEquals 0 "$?" +} + +testEvalAllClosesInputFiles() { + createInputFiles + + (ulimit -n 32; GOGC=off ./yq eval-all 'select(false)' test-fd-*.yml) + assertEquals 0 "$?" +} + +source ./scripts/shunit2 diff --git a/pkg/yqlib/all_at_once_evaluator.go b/pkg/yqlib/all_at_once_evaluator.go index bdeddd200c..b623f4f4e4 100644 --- a/pkg/yqlib/all_at_once_evaluator.go +++ b/pkg/yqlib/all_at_once_evaluator.go @@ -49,12 +49,7 @@ func (e *allAtOnceEvaluator) EvaluateFiles(expression string, filenames []string var allDocuments = list.New() for _, filename := range filenames { - reader, err := readStream(filename) - if err != nil { - return err - } - - fileDocuments, err := readDocuments(reader, filename, fileIndex, decoder) + fileDocuments, err := readDocumentsFromFile(filename, fileIndex, decoder) if err != nil { return err } @@ -73,3 +68,13 @@ func (e *allAtOnceEvaluator) EvaluateFiles(expression string, filenames []string } return printer.PrintResults(matches) } + +func readDocumentsFromFile(filename string, fileIndex int, decoder Decoder) (*list.List, error) { + reader, err := readStream(filename) + if err != nil { + return nil, err + } + defer SafelyCloseReader(reader) + + return readDocuments(reader, filename, fileIndex, decoder) +} diff --git a/pkg/yqlib/all_at_once_evaluator_test.go b/pkg/yqlib/all_at_once_evaluator_test.go index ce83db88cb..526f9d03d4 100644 --- a/pkg/yqlib/all_at_once_evaluator_test.go +++ b/pkg/yqlib/all_at_once_evaluator_test.go @@ -2,6 +2,7 @@ package yqlib import ( "bufio" + "os" "strings" "testing" @@ -76,3 +77,23 @@ func TestTomlDecoderCanBeReinitializedAcrossDocuments(t *testing.T) { } test.AssertResult(t, "Banana", secondDocuments.Front().Value.(*CandidateNode).Content[1].Value) } + +func TestReadDocumentsLeavesReaderOpen(t *testing.T) { + filename := t.TempDir() + "/input.yml" + if err := os.WriteFile(filename, []byte("a: apple\n"), 0600); err != nil { + t.Fatalf("failed to write input file: %v", err) + } + + file, err := os.Open(filename) + if err != nil { + t.Fatalf("failed to open input file: %v", err) + } + defer file.Close() + + if _, err := ReadDocuments(file, NewYamlDecoder(ConfiguredYamlPreferences)); err != nil { + t.Fatalf("failed to read documents: %v", err) + } + if _, err := file.Seek(0, 0); err != nil { + t.Fatalf("ReadDocuments closed the caller-owned reader: %v", err) + } +} diff --git a/pkg/yqlib/file_utils.go b/pkg/yqlib/file_utils.go index 3aca0ae117..07208cb815 100644 --- a/pkg/yqlib/file_utils.go +++ b/pkg/yqlib/file_utils.go @@ -62,6 +62,10 @@ func SafelyCloseReader(reader io.Reader) { switch reader := reader.(type) { case *os.File: safelyCloseFile(reader) + case io.Closer: + if err := reader.Close(); err != nil { + log.Errorf("Error closing reader: %v", err) + } } } diff --git a/pkg/yqlib/front_matter_test.go b/pkg/yqlib/front_matter_test.go index 2680aa2863..fe34716bde 100644 --- a/pkg/yqlib/front_matter_test.go +++ b/pkg/yqlib/front_matter_test.go @@ -142,6 +142,7 @@ Some content } decoder := NewYamlDecoder(ConfiguredYamlPreferences) docs, err := readDocuments(reader, tempFilename, 0, decoder) + SafelyCloseReader(reader) if err != nil { panic(err) } diff --git a/pkg/yqlib/stream_evaluator.go b/pkg/yqlib/stream_evaluator.go index ad1478941f..5c281731a3 100644 --- a/pkg/yqlib/stream_evaluator.go +++ b/pkg/yqlib/stream_evaluator.go @@ -5,7 +5,6 @@ import ( "errors" "fmt" "io" - "os" ) // A yaml expression evaluator that runs the expression multiple times for each given yaml document. @@ -50,21 +49,11 @@ func (s *streamEvaluator) EvaluateFiles(expression string, filenames []string, p } for _, filename := range filenames { - reader, err := readStream(filename) - - if err != nil { - return err - } - processedDocs, err := s.Evaluate(filename, reader, node, printer, decoder) + processedDocs, err := s.evaluateFile(filename, node, printer, decoder) if err != nil { return err } totalProcessDocs = totalProcessDocs + processedDocs - - switch reader := reader.(type) { - case *os.File: - safelyCloseFile(reader) - } } if totalProcessDocs == 0 { @@ -75,6 +64,16 @@ func (s *streamEvaluator) EvaluateFiles(expression string, filenames []string, p return nil } +func (s *streamEvaluator) evaluateFile(filename string, node *ExpressionNode, printer Printer, decoder Decoder) (uint, error) { + reader, err := readStream(filename) + if err != nil { + return 0, err + } + defer SafelyCloseReader(reader) + + return s.Evaluate(filename, reader, node, printer, decoder) +} + func (s *streamEvaluator) Evaluate(filename string, reader io.Reader, node *ExpressionNode, printer Printer, decoder Decoder) (uint, error) { filename = resolveFilename(filename) diff --git a/pkg/yqlib/utils.go b/pkg/yqlib/utils.go index 2843e42d28..6fc3f824b6 100644 --- a/pkg/yqlib/utils.go +++ b/pkg/yqlib/utils.go @@ -32,21 +32,23 @@ func resolveFilename(filename string) string { return filename } -func readStream(filename string) (io.Reader, error) { - var reader *bufio.Reader +type readCloser struct { + io.Reader + io.Closer +} + +func readStream(filename string) (io.ReadCloser, error) { if filename == "-" { - reader = bufio.NewReader(os.Stdin) - } else { - // ignore CWE-22 gosec issue - that's more targeted for http based apps that run in a public directory, - // and ensuring that it's not possible to give a path to a file outside that directory. - file, err := os.Open(filename) // #nosec - if err != nil { - return nil, err - } - reader = bufio.NewReader(file) + return io.NopCloser(bufio.NewReader(os.Stdin)), nil } - return reader, nil + // ignore CWE-22 gosec issue - that's more targeted for http based apps that run in a public directory, + // and ensuring that it's not possible to give a path to a file outside that directory. + file, err := os.Open(filename) // #nosec + if err != nil { + return nil, err + } + return &readCloser{Reader: bufio.NewReader(file), Closer: file}, nil } func writeString(writer io.Writer, txt string) error { @@ -71,10 +73,6 @@ func readDocuments(reader io.Reader, filename string, fileIndex int, decoder Dec candidateNode, errorReading := decoder.Decode() if errors.Is(errorReading, io.EOF) { - switch reader := reader.(type) { - case *os.File: - safelyCloseFile(reader) - } return inputList, nil } else if errorReading != nil { return nil, fmt.Errorf("bad file '%v': %w", filename, errorReading)