Skip to content

Commit 44f2c27

Browse files
authored
Fix CSV render error (#17406)
closed #17378 Both errors from #17378 were caused by #15175. Problem 1 (error with added file): `ToUTF8WithFallbackReader` creates a `MultiReader` from a `byte[2048]` and the remaining reader. `CreateReaderAndGuessDelimiter` tries to read 10000 bytes from this reader but only gets 2048 because that's the first reader in the `MultiReader`. Then the `if size < 1e4` thinks the input is at EOF and just returns that. Problem 2 (error with changed file): The blob reader gets defer closed. That was fine because the old version reads the whole file into memory. Now with the streaming version the close needs to defer after the method.
1 parent f99d50f commit 44f2c27

File tree

2 files changed

+21
-22
lines changed

2 files changed

+21
-22
lines changed

modules/csv/csv.go

+6-14
Original file line numberDiff line numberDiff line change
@@ -27,32 +27,24 @@ func CreateReader(input io.Reader, delimiter rune) *stdcsv.Reader {
2727
}
2828

2929
// CreateReaderAndGuessDelimiter tries to guess the field delimiter from the content and creates a csv.Reader.
30+
// Reads at most 10k bytes.
3031
func CreateReaderAndGuessDelimiter(rd io.Reader) (*stdcsv.Reader, error) {
3132
var data = make([]byte, 1e4)
3233
size, err := util.ReadAtMost(rd, data)
3334
if err != nil {
3435
return nil, err
3536
}
3637

37-
delimiter := guessDelimiter(data[:size])
38-
39-
var newInput io.Reader
40-
if size < 1e4 {
41-
newInput = bytes.NewReader(data[:size])
42-
} else {
43-
newInput = io.MultiReader(bytes.NewReader(data), rd)
44-
}
45-
46-
return CreateReader(newInput, delimiter), nil
38+
return CreateReader(
39+
io.MultiReader(bytes.NewReader(data[:size]), rd),
40+
guessDelimiter(data[:size]),
41+
), nil
4742
}
4843

4944
// guessDelimiter scores the input CSV data against delimiters, and returns the best match.
50-
// Reads at most 10k bytes & 10 lines.
5145
func guessDelimiter(data []byte) rune {
5246
maxLines := 10
53-
maxBytes := util.Min(len(data), 1e4)
54-
text := string(data[:maxBytes])
55-
text = quoteRegexp.ReplaceAllLiteralString(text, "")
47+
text := quoteRegexp.ReplaceAllLiteralString(string(data), "")
5648
lines := strings.SplitN(text, "\n", maxLines+1)
5749
lines = lines[:util.Min(maxLines, len(lines))]
5850

routers/web/repo/compare.go

+15-8
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"errors"
1111
"fmt"
1212
"html"
13+
"io"
1314
"net/http"
1415
"path"
1516
"path/filepath"
@@ -105,30 +106,36 @@ func setCsvCompareContext(ctx *context.Context) {
105106

106107
errTooLarge := errors.New(ctx.Locale.Tr("repo.error.csv.too_large"))
107108

108-
csvReaderFromCommit := func(c *git.Commit) (*csv.Reader, error) {
109+
csvReaderFromCommit := func(c *git.Commit) (*csv.Reader, io.Closer, error) {
109110
blob, err := c.GetBlobByPath(diffFile.Name)
110111
if err != nil {
111-
return nil, err
112+
return nil, nil, err
112113
}
113114

114115
if setting.UI.CSV.MaxFileSize != 0 && setting.UI.CSV.MaxFileSize < blob.Size() {
115-
return nil, errTooLarge
116+
return nil, nil, errTooLarge
116117
}
117118

118119
reader, err := blob.DataAsync()
119120
if err != nil {
120-
return nil, err
121+
return nil, nil, err
121122
}
122-
defer reader.Close()
123123

124-
return csv_module.CreateReaderAndGuessDelimiter(charset.ToUTF8WithFallbackReader(reader))
124+
csvReader, err := csv_module.CreateReaderAndGuessDelimiter(charset.ToUTF8WithFallbackReader(reader))
125+
return csvReader, reader, err
125126
}
126127

127-
baseReader, err := csvReaderFromCommit(baseCommit)
128+
baseReader, baseBlobCloser, err := csvReaderFromCommit(baseCommit)
129+
if baseBlobCloser != nil {
130+
defer baseBlobCloser.Close()
131+
}
128132
if err == errTooLarge {
129133
return CsvDiffResult{nil, err.Error()}
130134
}
131-
headReader, err := csvReaderFromCommit(headCommit)
135+
headReader, headBlobCloser, err := csvReaderFromCommit(headCommit)
136+
if headBlobCloser != nil {
137+
defer headBlobCloser.Close()
138+
}
132139
if err == errTooLarge {
133140
return CsvDiffResult{nil, err.Error()}
134141
}

0 commit comments

Comments
 (0)