Skip to content

Commit

Permalink
fix panic in Writer
Browse files Browse the repository at this point in the history
Commit 766cfec introduced this bug by defining an incorrect split
function. First it breaks the old behavior because it never splits at
newlines now. Second, it causes a panic because it never tells the
scanner to stop. See the bufio.ScanLines function, something like:
```
if atEOF && len(data) == 0 {
	return 0, nil, nil
}
```
is needed to do that.

This commit fixes it by restoring the old behavior and calling
bufio.ScanLines but also keep the 64KB check in place to avoid buffering
for to long.

Two tests are added to ensure it is working as expected.

Fixes sirupsen#1383

Signed-off-by: Paul Holzinger <[email protected]>
  • Loading branch information
Luap99 authored and ashmckenzie committed Jun 2, 2023
1 parent 352781d commit 39be60d
Show file tree
Hide file tree
Showing 2 changed files with 89 additions and 1 deletion.
26 changes: 25 additions & 1 deletion writer.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bufio"
"io"
"runtime"
"strings"
)

// Writer at INFO level. See WriterLevel for details.
Expand Down Expand Up @@ -54,14 +55,37 @@ func (entry *Entry) WriterLevel(level Level) *io.PipeWriter {
return writer
}

// writerScanner scans the input from the reader and writes it to the logger
func (entry *Entry) writerScanner(reader *io.PipeReader, printFunc func(args ...interface{})) {
scanner := bufio.NewScanner(reader)

// Set the buffer size to the maximum token size to avoid buffer overflows
scanner.Buffer(make([]byte, bufio.MaxScanTokenSize), bufio.MaxScanTokenSize)

// Define a split function to split the input into chunks of up to 64KB
chunkSize := bufio.MaxScanTokenSize // 64KB
splitFunc := func(data []byte, atEOF bool) (int, []byte, error) {
if len(data) >= chunkSize {
return chunkSize, data[:chunkSize], nil
}

return bufio.ScanLines(data, atEOF)
}

// Use the custom split function to split the input
scanner.Split(splitFunc)

// Scan the input and write it to the logger using the specified print function
for scanner.Scan() {
printFunc(scanner.Text())
printFunc(strings.TrimRight(scanner.Text(), "\r\n"))
}

// If there was an error while scanning the input, log an error
if err := scanner.Err(); err != nil {
entry.Errorf("Error while reading from Writer: %s", err)
}

// Close the reader when we are done
reader.Close()
}

Expand Down
64 changes: 64 additions & 0 deletions writer_test.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,16 @@
package logrus_test

import (
"bufio"
"bytes"
"log"
"net/http"
"strings"
"testing"
"time"

"github.com/sirupsen/logrus"
"github.com/stretchr/testify/assert"
)

func ExampleLogger_Writer_httpServer() {
Expand Down Expand Up @@ -32,3 +38,61 @@ func ExampleLogger_Writer_stdlib() {
// Not logrus imported under the name `log`.
log.SetOutput(logger.Writer())
}

func TestWriterSplitNewlines(t *testing.T) {
buf := bytes.NewBuffer(nil)
logger := logrus.New()
logger.Formatter = &logrus.TextFormatter{
DisableColors: true,
DisableTimestamp: true,
}
logger.SetOutput(buf)
writer := logger.Writer()

const logNum = 10

for i := 0; i < logNum; i++ {
_, err := writer.Write([]byte("bar\nfoo\n"))
assert.NoError(t, err, "writer.Write failed")
}
writer.Close()
// Test is flaky because it writes in another goroutine,
// we need to make sure to wait a bit so all write are done.
time.Sleep(500 * time.Millisecond)

lines := strings.Split(strings.TrimRight(buf.String(), "\n"), "\n")
assert.Len(t, lines, logNum*2, "logger printed incorrect number of lines")
}

func TestWriterSplitsMax64KB(t *testing.T) {
buf := bytes.NewBuffer(nil)
logger := logrus.New()
logger.Formatter = &logrus.TextFormatter{
DisableColors: true,
DisableTimestamp: true,
}
logger.SetOutput(buf)
writer := logger.Writer()

// write more than 64KB
const bigWriteLen = bufio.MaxScanTokenSize + 100
output := make([]byte, bigWriteLen)
// lets not write zero bytes
for i := 0; i < bigWriteLen; i++ {
output[i] = 'A'
}

for i := 0; i < 3; i++ {
len, err := writer.Write(output)
assert.NoError(t, err, "writer.Write failed")
assert.Equal(t, bigWriteLen, len, "bytes written")
}
writer.Close()
// Test is flaky because it writes in another goroutine,
// we need to make sure to wait a bit so all write are done.
time.Sleep(500 * time.Millisecond)

lines := strings.Split(strings.TrimRight(buf.String(), "\n"), "\n")
// we should have 4 lines because we wrote more than 64 KB each time
assert.Len(t, lines, 4, "logger printed incorrect number of lines")
}

0 comments on commit 39be60d

Please sign in to comment.