Skip to content

Commit d40e25c

Browse files
Luap99sirupsen
authored andcommitted
fix panic in Writer
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 #1383 Signed-off-by: Paul Holzinger <[email protected]>
1 parent f9291a5 commit d40e25c

File tree

2 files changed

+68
-4
lines changed

2 files changed

+68
-4
lines changed

writer.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -70,16 +70,16 @@ func (entry *Entry) writerScanner(reader *io.PipeReader, printFunc func(args ...
7070
scanner.Buffer(make([]byte, bufio.MaxScanTokenSize), bufio.MaxScanTokenSize)
7171

7272
// Define a split function to split the input into chunks of up to 64KB
73-
chunkSize := 64 * 1024 // 64KB
73+
chunkSize := bufio.MaxScanTokenSize // 64KB
7474
splitFunc := func(data []byte, atEOF bool) (int, []byte, error) {
75-
if len(data) > chunkSize {
75+
if len(data) >= chunkSize {
7676
return chunkSize, data[:chunkSize], nil
7777
}
7878

79-
return len(data), data, nil
79+
return bufio.ScanLines(data, atEOF)
8080
}
8181

82-
//Use the custom split function to split the input
82+
// Use the custom split function to split the input
8383
scanner.Split(splitFunc)
8484

8585
// Scan the input and write it to the logger using the specified print function

writer_test.go

+64
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,16 @@
11
package logrus_test
22

33
import (
4+
"bufio"
5+
"bytes"
46
"log"
57
"net/http"
8+
"strings"
9+
"testing"
10+
"time"
611

712
"github.com/sirupsen/logrus"
13+
"github.com/stretchr/testify/assert"
814
)
915

1016
func ExampleLogger_Writer_httpServer() {
@@ -32,3 +38,61 @@ func ExampleLogger_Writer_stdlib() {
3238
// Not logrus imported under the name `log`.
3339
log.SetOutput(logger.Writer())
3440
}
41+
42+
func TestWriterSplitNewlines(t *testing.T) {
43+
buf := bytes.NewBuffer(nil)
44+
logger := logrus.New()
45+
logger.Formatter = &logrus.TextFormatter{
46+
DisableColors: true,
47+
DisableTimestamp: true,
48+
}
49+
logger.SetOutput(buf)
50+
writer := logger.Writer()
51+
52+
const logNum = 10
53+
54+
for i := 0; i < logNum; i++ {
55+
_, err := writer.Write([]byte("bar\nfoo\n"))
56+
assert.NoError(t, err, "writer.Write failed")
57+
}
58+
writer.Close()
59+
// Test is flaky because it writes in another goroutine,
60+
// we need to make sure to wait a bit so all write are done.
61+
time.Sleep(500 * time.Millisecond)
62+
63+
lines := strings.Split(strings.TrimRight(buf.String(), "\n"), "\n")
64+
assert.Len(t, lines, logNum*2, "logger printed incorrect number of lines")
65+
}
66+
67+
func TestWriterSplitsMax64KB(t *testing.T) {
68+
buf := bytes.NewBuffer(nil)
69+
logger := logrus.New()
70+
logger.Formatter = &logrus.TextFormatter{
71+
DisableColors: true,
72+
DisableTimestamp: true,
73+
}
74+
logger.SetOutput(buf)
75+
writer := logger.Writer()
76+
77+
// write more than 64KB
78+
const bigWriteLen = bufio.MaxScanTokenSize + 100
79+
output := make([]byte, bigWriteLen)
80+
// lets not write zero bytes
81+
for i := 0; i < bigWriteLen; i++ {
82+
output[i] = 'A'
83+
}
84+
85+
for i := 0; i < 3; i++ {
86+
len, err := writer.Write(output)
87+
assert.NoError(t, err, "writer.Write failed")
88+
assert.Equal(t, bigWriteLen, len, "bytes written")
89+
}
90+
writer.Close()
91+
// Test is flaky because it writes in another goroutine,
92+
// we need to make sure to wait a bit so all write are done.
93+
time.Sleep(500 * time.Millisecond)
94+
95+
lines := strings.Split(strings.TrimRight(buf.String(), "\n"), "\n")
96+
// we should have 4 lines because we wrote more than 64 KB each time
97+
assert.Len(t, lines, 4, "logger printed incorrect number of lines")
98+
}

0 commit comments

Comments
 (0)