Prevent dangling command calls

go.mod

@@ -18,8 +18,6 @@ require (
 	github.com/caddyserver/certmagic v0.15.4
 	github.com/chi-middleware/proxy v1.1.1
 	github.com/denisenkom/go-mssqldb v0.12.0
-	github.com/djherbis/buffer v1.2.0
-	github.com/djherbis/nio/v3 v3.0.1
 	github.com/duo-labs/webauthn v0.0.0-20220223184316-4d1cf2d34051
 	github.com/dustin/go-humanize v1.0.0
 	github.com/editorconfig/editorconfig-core-go/v2 v2.4.3

go.sum

@@ -369,11 +369,6 @@ github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f h1:lO4WD4F/r
 github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f/go.mod h1:cuUVRXasLTGF7a8hSLbxyZXjz+1KgoB3wDUb6vlszIc=
 github.com/dgryski/go-sip13 v0.0.0-20181026042036-e10d5fee7954/go.mod h1:vAd38F8PWV+bWy6jNmig1y/TA+kYO4g3RSRF0IAv0no=
 github.com/dimchansky/utfbom v1.1.0/go.mod h1:rO41eb7gLfo8SF1jd9F8HplJm1Fewwi4mQvIirEdv+8=
-github.com/djherbis/buffer v1.1.0/go.mod h1:VwN8VdFkMY0DCALdY8o00d3IZ6Amz/UNVMWcSaJT44o=
-github.com/djherbis/buffer v1.2.0 h1:PH5Dd2ss0C7CRRhQCZ2u7MssF+No9ide8Ye71nPHcrQ=
-github.com/djherbis/buffer v1.2.0/go.mod h1:fjnebbZjCUpPinBRD+TDwXSOeNQ7fPQWLfGQqiAiUyE=
-github.com/djherbis/nio/v3 v3.0.1 h1:6wxhnuppteMa6RHA4L81Dq7ThkZH8SwnDzXDYy95vB4=
-github.com/djherbis/nio/v3 v3.0.1/go.mod h1:Ng4h80pbZFMla1yKzm61cF0tqqilXZYrogmWgZxOcmg=
 github.com/dlclark/regexp2 v1.4.0 h1:F1rxgk7p4uKjwIQxBs9oAXe5CqrXlCduYEJvrF4u93E=
 github.com/dlclark/regexp2 v1.4.0/go.mod h1:2pZnwuY/m+8K6iRw6wQdMtk+rH5tNGR1i55kozfMjCc=
 github.com/dnaeon/go-vcr v1.2.0/go.mod h1:R4UdLID7HZT3taECzJs4YgbbH6PIGXB6W/sc5OLb6RQ=

modules/git/batch_reader.go

@@ -9,24 +9,14 @@ import (
 	"bytes"
 	"context"
 	"fmt"
-	"io"
 	"math"
 	"runtime"
 	"strconv"
 	"strings"
 
 	"code.gitea.io/gitea/modules/log"
-
-	"github.com/djherbis/buffer"
-	"github.com/djherbis/nio/v3"
 )
 
-// WriteCloserError wraps an io.WriteCloser with an additional CloseWithError function
-type WriteCloserError interface {
-	io.WriteCloser
-	CloseWithError(err error) error
-}
-
 // EnsureValidGitRepository runs git rev-parse in the repository path - thus ensuring that the repository is a valid repository.
 // Run before opening git cat-file.
 // This is needed otherwise the git cat-file will hang for invalid repositories.
@@ -44,10 +34,21 @@ func EnsureValidGitRepository(ctx context.Context, repoPath string) error {
 	return nil
 }
 
+func returnClosedReaderWriters(err error) (WriteCloserError, *bufio.Reader, func()) {
+	wr := &ClosedReadWriteCloserError{err}
+	return wr, bufio.NewReader(wr), func() {}
+}
+
 // CatFileBatchCheck opens git cat-file --batch-check in the provided repo and returns a stdin pipe, a stdout reader and cancel function
 func CatFileBatchCheck(ctx context.Context, repoPath string) (WriteCloserError, *bufio.Reader, func()) {
-	batchStdinReader, batchStdinWriter := io.Pipe()
-	batchStdoutReader, batchStdoutWriter := io.Pipe()
+	pipes, err := NewPipePairs(2)
+	if err != nil {
+		log.Critical("Unable to open pipe to write to: %v", err)
+		return returnClosedReaderWriters(err)
+	}
+	batchStdinReader, batchStdinWriter := pipes[0].ReaderWriter()
+	batchStdoutReader, batchStdoutWriter := pipes[1].ReaderWriter()
+
 	ctx, ctxCancel := context.WithCancel(ctx)
 	closed := make(chan struct{})
 	cancel := func() {
@@ -57,12 +58,6 @@ func CatFileBatchCheck(ctx context.Context, repoPath string) (WriteCloserError,
 		<-closed
 	}
 
-	// Ensure cancel is called as soon as the provided context is cancelled
-	go func() {
-		<-ctx.Done()
-		cancel()
-	}()
-
 	_, filename, line, _ := runtime.Caller(2)
 	filename = strings.TrimPrefix(filename, callerPrefix)
 
@@ -77,8 +72,9 @@ func CatFileBatchCheck(ctx context.Context, repoPath string) (WriteCloserError,
 				Stderr: &stderr,
 			})
 		if err != nil {
-			_ = batchStdoutWriter.CloseWithError(ConcatenateError(err, (&stderr).String()))
-			_ = batchStdinReader.CloseWithError(ConcatenateError(err, (&stderr).String()))
+			err := ConcatenateError(err, (&stderr).String())
+			_ = batchStdinReader.CloseWithError(err)
+			_ = batchStdoutWriter.CloseWithError(err)
 		} else {
 			_ = batchStdoutWriter.Close()
 			_ = batchStdinReader.Close()
@@ -88,16 +84,21 @@ func CatFileBatchCheck(ctx context.Context, repoPath string) (WriteCloserError,
 
 	// For simplicities sake we'll use a buffered reader to read from the cat-file --batch-check
 	batchReader := bufio.NewReader(batchStdoutReader)
-
 	return batchStdinWriter, batchReader, cancel
 }
 
 // CatFileBatch opens git cat-file --batch in the provided repo and returns a stdin pipe, a stdout reader and cancel function
 func CatFileBatch(ctx context.Context, repoPath string) (WriteCloserError, *bufio.Reader, func()) {
 	// We often want to feed the commits in order into cat-file --batch, followed by their trees and sub trees as necessary.
 	// so let's create a batch stdin and stdout
-	batchStdinReader, batchStdinWriter := io.Pipe()
-	batchStdoutReader, batchStdoutWriter := nio.Pipe(buffer.New(32 * 1024))
+	pipes, err := NewPipePairs(2)
+	if err != nil {
+		log.Critical("Unable to open pipe to write to: %v", err)
+		return returnClosedReaderWriters(err)
+	}
+	batchStdinReader, batchStdinWriter := pipes[0].ReaderWriter()
+	batchStdoutReader, batchStdoutWriter := pipes[1].ReaderWriter()
+
 	ctx, ctxCancel := context.WithCancel(ctx)
 	closed := make(chan struct{})
 	cancel := func() {
@@ -107,12 +108,6 @@ func CatFileBatch(ctx context.Context, repoPath string) (WriteCloserError, *bufi
 		<-closed
 	}
 
-	// Ensure cancel is called as soon as the provided context is cancelled
-	go func() {
-		<-ctx.Done()
-		cancel()
-	}()
-
 	_, filename, line, _ := runtime.Caller(2)
 	filename = strings.TrimPrefix(filename, callerPrefix)
 
@@ -127,8 +122,9 @@ func CatFileBatch(ctx context.Context, repoPath string) (WriteCloserError, *bufi
 				Stderr: &stderr,
 			})
 		if err != nil {
-			_ = batchStdoutWriter.CloseWithError(ConcatenateError(err, (&stderr).String()))
-			_ = batchStdinReader.CloseWithError(ConcatenateError(err, (&stderr).String()))
+			err := ConcatenateError(err, (&stderr).String())
+			_ = batchStdinReader.CloseWithError(err)
+			_ = batchStdoutWriter.CloseWithError(err)
 		} else {
 			_ = batchStdoutWriter.Close()
 			_ = batchStdinReader.Close()

modules/git/blob.go

@@ -9,6 +9,7 @@ import (
 	"bytes"
 	"encoding/base64"
 	"io"
+	"strings"
 
 	"code.gitea.io/gitea/modules/typesniffer"
 	"code.gitea.io/gitea/modules/util"
@@ -69,25 +70,18 @@ func (b *Blob) GetBlobContentBase64() (string, error) {
 	}
 	defer dataRc.Close()
 
-	pr, pw := io.Pipe()
-	encoder := base64.NewEncoder(base64.StdEncoding, pw)
+	sb := &strings.Builder{}
 
-	go func() {
-		_, err := io.Copy(encoder, dataRc)
-		_ = encoder.Close()
+	encoder := base64.NewEncoder(base64.StdEncoding, sb)
 
-		if err != nil {
-			_ = pw.CloseWithError(err)
-		} else {
-			_ = pw.Close()
-		}
-	}()
+	_, err = io.Copy(encoder, dataRc)
+	_ = encoder.Close()
 
-	out, err := io.ReadAll(pr)
 	if err != nil {
 		return "", err
 	}
-	return string(out), nil
+
+	return sb.String(), nil
 }
 
 // GuessContentType guesses the content type of the blob.

modules/git/command.go

@@ -158,13 +158,50 @@ func (c *Command) Run(opts *RunOpts) error {
 	)
 
 	cmd.Dir = opts.Dir
-	cmd.Stdout = opts.Stdout
-	cmd.Stderr = opts.Stderr
-	cmd.Stdin = opts.Stdin
+	if pipeWriter, ok := opts.Stdout.(*PipeWriter); ok {
+		cmd.Stdout = pipeWriter.File()
+	} else {
+		cmd.Stdout = opts.Stdout
+	}
+	if pipeWriter, ok := opts.Stderr.(*PipeWriter); ok {
+		cmd.Stderr = pipeWriter.File()
+	} else {
+		cmd.Stderr = opts.Stderr
+	}
+	if pipeReader, ok := opts.Stdin.(*PipeReader); ok {
+		cmd.Stdin = pipeReader.File()
+	} else {
+		cmd.Stdin = opts.Stdin
+	}
 	if err := cmd.Start(); err != nil {
 		return err
 	}
 
+	// Ensure that closers are closed
+	closers := make([]io.Closer, 0, 3)
+	for _, pipe := range []interface{}{cmd.Stdout, cmd.Stdin, cmd.Stderr} {
+		if pipe == nil {
+			continue
+		}
+		if _, ok := pipe.(*os.File); ok {
+			continue
+		}
+
+		if closer, ok := pipe.(io.Closer); ok {
+			closers = append(closers, closer)
+		}
+	}
+
+	if len(closers) > 0 {
+		go func() {
+			<-ctx.Done()
+			cancel()
+			for _, closer := range closers {
+				_ = closer.Close()
+			}
+		}()
+	}
+
 	if opts.PipelineFunc != nil {
 		err := opts.PipelineFunc(ctx, cancel)
 		if err != nil {

modules/git/commit.go

@@ -12,6 +12,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"os"
 	"os/exec"
 	"strconv"
 	"strings"
@@ -475,7 +476,10 @@ func parseCommitFileStatus(fileStatus *CommitFileStatus, stdout io.Reader) {
 
 // GetCommitFileStatus returns file status of commit in given repository.
 func GetCommitFileStatus(ctx context.Context, repoPath, commitID string) (*CommitFileStatus, error) {
-	stdout, w := io.Pipe()
+	stdout, w, err := os.Pipe()
+	if err != nil {
+		return nil, err
+	}
 	done := make(chan struct{})
 	fileStatus := NewCommitFileStatus()
 	go func() {
@@ -486,7 +490,7 @@ func GetCommitFileStatus(ctx context.Context, repoPath, commitID string) (*Commi
 	stderr := new(bytes.Buffer)
 	args := []string{"log", "--name-status", "-c", "--pretty=format:", "--parents", "--no-renames", "-z", "-1", commitID}
 
-	err := NewCommand(ctx, args...).Run(&RunOpts{
+	err = NewCommand(ctx, args...).Run(&RunOpts{
 		Dir:    repoPath,
 		Stdout: w,
 		Stderr: stderr,

modules/git/log_name_status.go

@@ -13,15 +13,19 @@ import (
 	"sort"
 	"strings"
 
-	"github.com/djherbis/buffer"
-	"github.com/djherbis/nio/v3"
+	"code.gitea.io/gitea/modules/log"
 )
 
 // LogNameStatusRepo opens git log --raw in the provided repo and returns a stdin pipe, a stdout reader and cancel function
 func LogNameStatusRepo(ctx context.Context, repository, head, treepath string, paths ...string) (*bufio.Reader, func()) {
 	// We often want to feed the commits in order into cat-file --batch, followed by their trees and sub trees as necessary.
 	// so let's create a batch stdin and stdout
-	stdoutReader, stdoutWriter := nio.Pipe(buffer.New(32 * 1024))
+	stdoutReader, stdoutWriter, err := Pipe()
+	if err != nil {
+		log.Critical("Unable to open pipe to write to: %v", err)
+		rd := &ClosedReadWriteCloserError{err}
+		return bufio.NewReader(rd), func() {}
+	}
 
 	// Lets also create a context so that we can absolutely ensure that the command should die when we're done
 	ctx, ctxCancel := context.WithCancel(ctx)

modules/git/pipeline/catfile.go

@@ -9,7 +9,6 @@ import (
 	"bytes"
 	"context"
 	"fmt"
-	"io"
 	"strconv"
 	"strings"
 	"sync"
@@ -19,7 +18,7 @@ import (
 )
 
 // CatFileBatchCheck runs cat-file with --batch-check
-func CatFileBatchCheck(ctx context.Context, shasToCheckReader *io.PipeReader, catFileCheckWriter *io.PipeWriter, wg *sync.WaitGroup, tmpBasePath string) {
+func CatFileBatchCheck(ctx context.Context, shasToCheckReader git.ReadCloserError, catFileCheckWriter git.WriteCloserError, wg *sync.WaitGroup, tmpBasePath string) {
 	defer wg.Done()
 	defer shasToCheckReader.Close()
 	defer catFileCheckWriter.Close()
@@ -38,7 +37,7 @@ func CatFileBatchCheck(ctx context.Context, shasToCheckReader *io.PipeReader, ca
 }
 
 // CatFileBatchCheckAllObjects runs cat-file with --batch-check --batch-all
-func CatFileBatchCheckAllObjects(ctx context.Context, catFileCheckWriter *io.PipeWriter, wg *sync.WaitGroup, tmpBasePath string, errChan chan<- error) {
+func CatFileBatchCheckAllObjects(ctx context.Context, catFileCheckWriter git.WriteCloserError, wg *sync.WaitGroup, tmpBasePath string, errChan chan<- error) {
 	defer wg.Done()
 	defer catFileCheckWriter.Close()
 
@@ -58,7 +57,7 @@ func CatFileBatchCheckAllObjects(ctx context.Context, catFileCheckWriter *io.Pip
 }
 
 // CatFileBatch runs cat-file --batch
-func CatFileBatch(ctx context.Context, shasToBatchReader *io.PipeReader, catFileBatchWriter *io.PipeWriter, wg *sync.WaitGroup, tmpBasePath string) {
+func CatFileBatch(ctx context.Context, shasToBatchReader git.ReadCloserError, catFileBatchWriter git.WriteCloserError, wg *sync.WaitGroup, tmpBasePath string) {
 	defer wg.Done()
 	defer shasToBatchReader.Close()
 	defer catFileBatchWriter.Close()
@@ -76,7 +75,7 @@ func CatFileBatch(ctx context.Context, shasToBatchReader *io.PipeReader, catFile
 }
 
 // BlobsLessThan1024FromCatFileBatchCheck reads a pipeline from cat-file --batch-check and returns the blobs <1024 in size
-func BlobsLessThan1024FromCatFileBatchCheck(catFileCheckReader *io.PipeReader, shasToBatchWriter *io.PipeWriter, wg *sync.WaitGroup) {
+func BlobsLessThan1024FromCatFileBatchCheck(catFileCheckReader git.ReadCloserError, shasToBatchWriter git.WriteCloserError, wg *sync.WaitGroup) {
 	defer wg.Done()
 	defer catFileCheckReader.Close()
 	scanner := bufio.NewScanner(catFileCheckReader)

modules/git/pipeline/lfs.go

@@ -99,8 +99,15 @@ func FindLFSFile(repo *git.Repository, hash git.SHA1) ([]*LFSResult, error) {
 	sort.Sort(lfsResultSlice(results))
 
 	// Should really use a go-git function here but name-rev is not completed and recapitulating it is not simple
-	shasToNameReader, shasToNameWriter := io.Pipe()
-	nameRevStdinReader, nameRevStdinWriter := io.Pipe()
+	pipes, err := git.NewPipePairs(2)
+	if err != nil {
+		return nil, err
+	}
+	defer pipes.Close()
+
+	shasToNameReader, shasToNameWriter := pipes[0].ReaderWriter()
+	nameRevStdinReader, nameRevStdinWriter := pipes[1].ReaderWriter()
+
 	errChan := make(chan error, 1)
 	wg := sync.WaitGroup{}
 	wg.Add(3)

modules/git/pipeline/lfs_nogogit.go

@@ -45,7 +45,10 @@ func FindLFSFile(repo *git.Repository, hash git.SHA1) ([]*LFSResult, error) {
 	basePath := repo.Path
 
 	// Use rev-list to provide us with all commits in order
-	revListReader, revListWriter := io.Pipe()
+	revListReader, revListWriter, err := git.Pipe()
+	if err != nil {
+		return nil, err
+	}
 	defer func() {
 		_ = revListWriter.Close()
 		_ = revListReader.Close()
@@ -195,8 +198,15 @@ func FindLFSFile(repo *git.Repository, hash git.SHA1) ([]*LFSResult, error) {
 	sort.Sort(lfsResultSlice(results))
 
 	// Should really use a go-git function here but name-rev is not completed and recapitulating it is not simple
-	shasToNameReader, shasToNameWriter := io.Pipe()
-	nameRevStdinReader, nameRevStdinWriter := io.Pipe()
+	pipes, err := git.NewPipePairs(2)
+	if err != nil {
+		return nil, err
+	}
+	defer pipes.Close()
+
+	shasToNameReader, shasToNameWriter := pipes[0].ReaderWriter()
+	nameRevStdinReader, nameRevStdinWriter := pipes[1].ReaderWriter()
+
 	errChan := make(chan error, 1)
 	wg := sync.WaitGroup{}
 	wg.Add(3)

modules/git/pipeline/namerev.go

@@ -8,15 +8,14 @@ import (
 	"bytes"
 	"context"
 	"fmt"
-	"io"
 	"strings"
 	"sync"
 
 	"code.gitea.io/gitea/modules/git"
 )
 
 // NameRevStdin runs name-rev --stdin
-func NameRevStdin(ctx context.Context, shasToNameReader *io.PipeReader, nameRevStdinWriter *io.PipeWriter, wg *sync.WaitGroup, tmpBasePath string) {
+func NameRevStdin(ctx context.Context, shasToNameReader git.ReadCloserError, nameRevStdinWriter git.WriteCloserError, wg *sync.WaitGroup, tmpBasePath string) {
 	defer wg.Done()
 	defer shasToNameReader.Close()
 	defer nameRevStdinWriter.Close()