diff options
Diffstat (limited to 'internal')
| -rw-r--r-- | internal/extract/tools.go | 18 | ||||
| -rw-r--r-- | internal/extract/tools_test.go | 8 | ||||
| -rw-r--r-- | internal/lock/lock.go | 5 | ||||
| -rw-r--r-- | internal/lock/lock_test.go | 13 |
4 files changed, 33 insertions, 11 deletions
diff --git a/internal/extract/tools.go b/internal/extract/tools.go index ca6a9b0..f815123 100644 --- a/internal/extract/tools.go +++ b/internal/extract/tools.go @@ -65,17 +65,19 @@ type boundedWriter struct { } func (w *boundedWriter) Write(p []byte) (int, error) { + if room := w.limit - w.total; room > 0 { + // The part of p that still fits is kept even when p overflows, so a + // tool that writes its whole error message in one large write still + // has that message's start to show. + w.buf.Write(p[:min(int64(len(p)), room)]) + } w.total += int64(len(p)) - if w.total > w.limit { - if !w.overflowed { - w.overflowed = true - if w.onOverflow != nil { - w.onOverflow() - } + if w.total > w.limit && !w.overflowed { + w.overflowed = true + if w.onOverflow != nil { + w.onOverflow() } - return len(p), nil } - w.buf.Write(p) return len(p), nil } diff --git a/internal/extract/tools_test.go b/internal/extract/tools_test.go index 756903d..5af80fb 100644 --- a/internal/extract/tools_test.go +++ b/internal/extract/tools_test.go @@ -212,13 +212,17 @@ func TestCallerCancelReturnsPromptly(t *testing.T) { // it at all must not cost unbounded memory. func TestStderrFloodBounded(t *testing.T) { bin := t.TempDir() - fakeTool(t, bin, "pdftotext", `head -c 10000000 /dev/zero | tr '\0' x >&2; exit 1`) + // dd, not head -c: OpenBSD's head has no -c. + fakeTool(t, bin, "pdftotext", `dd if=/dev/zero bs=10000 count=1000 2>/dev/null | tr '\0' x >&2; exit 1`) e := newWithPath(bin) _, err := e.run(context.Background(), maxToolOutput, "pdftotext") if err == nil { t.Fatal("stderr flood: want an error") } + if !strings.Contains(err.Error(), strings.Repeat("x", 100)) { + t.Fatalf("stderr flood: the message %q does not come from the flood", err) + } if max := len("pdftotext failed: ") + 200; len(err.Error()) > max { t.Errorf("stderr flood: message is %d bytes, want <=%d", len(err.Error()), max) } @@ -231,7 +235,7 @@ func TestStderrFloodBounded(t *testing.T) { // result. func TestMaxReadCapsToolOutput(t *testing.T) { bin := t.TempDir() - fakeTool(t, bin, "pdftotext", `yes aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | head -c 200000`) + fakeTool(t, bin, "pdftotext", `dd if=/dev/zero bs=1000 count=200 2>/dev/null | tr '\0' a`) e := newWithPath(bin) if _, err := text(t, e, file(t, "small.pdf", []byte("%PDF-1.4")), 1024); !errors.Is(err, ErrTooLarge) { t.Fatalf("got %v, want ErrTooLarge (max-read 1024 should have capped a 200000-byte tool output)", err) diff --git a/internal/lock/lock.go b/internal/lock/lock.go index 462b26e..2b98227 100644 --- a/internal/lock/lock.go +++ b/internal/lock/lock.go @@ -10,6 +10,7 @@ import ( "errors" "fmt" "io/fs" + "math" "os" "path/filepath" "strconv" @@ -156,8 +157,10 @@ func parsePid(s string) (int, bool) { } // running reports whether pid names a process that is currently running. +// A pid outside the kernel's 32-bit range names no process: kill(2) would +// cut it to its low bits and ask about another one. func running(pid int) bool { - if pid <= 0 { + if pid <= 0 || pid > math.MaxInt32 { return false } proc, err := os.FindProcess(pid) diff --git a/internal/lock/lock_test.go b/internal/lock/lock_test.go index fca4d76..53ce5ca 100644 --- a/internal/lock/lock_test.go +++ b/internal/lock/lock_test.go @@ -122,3 +122,16 @@ func TestStaleLockIsTakenOver(t *testing.T) { t.Error("the takeover was not reported to the caller") } } + +// TestRunningRejectsPidsBeyondTheKernelsRange: a pid wider than 32 bits in a +// damaged lock file is cut to its low bits by kill(2), so without a range +// check it names some other process - here this very one - and a stale lock +// would never be taken over. +func TestRunningRejectsPidsBeyondTheKernelsRange(t *testing.T) { + if running(os.Getpid() + 1<<32) { + t.Error("running(pid + 2^32) is true: the pid was truncated to this process") + } + if !running(os.Getpid()) { + t.Error("running(own pid) is false") + } +} |
