diff options
author | Zeger-Jan van de Weg <git@zjvandeweg.nl> | 2020-07-21 10:37:00 +0300 |
---|---|---|
committer | Zeger-Jan van de Weg <git@zjvandeweg.nl> | 2020-07-21 13:52:46 +0300 |
commit | f9a937d69aa7ca6e586e0ba2efaf2cc65d382aaa (patch) | |
tree | 059696b0aeb0f82775b9ac3cbb26485872f896d6 | |
parent | 521bb978da8780aca690136e78a3ad388726c8ad (diff) |
lines.Send() only support byte delimiter
During the work on pagination for FindAllLocalBranches in
e3b1d6e5d2379977de477a2614f960d4f19e6dea I ran into buggy behaviour when multi
line delimiters were used in `lines.Send()`. One example: if a delimiter is
twice the same character, it would unsuccessfully detect the last delimiter and
thus send invalid results to a client.
Luckily there's no call site that actually uses multi byte delimiters,
which allows this change to remove support for it.
-rw-r--r-- | changelogs/unreleased/zj-line-multi-delimeter-support-removal.yml | 5 | ||||
-rw-r--r-- | internal/helper/lines/send.go | 16 | ||||
-rw-r--r-- | internal/helper/lines/send_test.go | 1 | ||||
-rw-r--r-- | internal/service/ref/refs.go | 4 | ||||
-rw-r--r-- | internal/service/repository/search_files.go | 2 |
5 files changed, 15 insertions, 13 deletions
diff --git a/changelogs/unreleased/zj-line-multi-delimeter-support-removal.yml b/changelogs/unreleased/zj-line-multi-delimeter-support-removal.yml new file mode 100644 index 000000000..e7514159c --- /dev/null +++ b/changelogs/unreleased/zj-line-multi-delimeter-support-removal.yml @@ -0,0 +1,5 @@ +--- +title: lines.Send() only support byte delimiter +merge_request: 2402 +author: +type: fixed diff --git a/internal/helper/lines/send.go b/internal/helper/lines/send.go index d0b65d067..5777d3c51 100644 --- a/internal/helper/lines/send.go +++ b/internal/helper/lines/send.go @@ -12,8 +12,8 @@ import ( // a line gets fed into the Sender. type SenderOpts struct { // Delimiter is the separator used to split the sender's output into - // lines. Defaults to "\n". - Delimiter []byte + // lines. Defaults to an empty byte (0). + Delimiter byte // Limit is the upper limit of how many lines will be sent. The zero // value will cause no lines to be sent. Limit int @@ -93,14 +93,14 @@ func (w *writer) consume(r io.Reader) error { for { // delim can be multiple bytes, so we read till the end byte of it ... - chunk, err := buf.ReadBytes(w.delimiter()[len(w.delimiter())-1]) + chunk, err := buf.ReadBytes(w.delimiter()) if err != nil && err != io.EOF { return err } line = append(line, chunk...) // ... then we check if the last bytes of line are the same as delim - if bytes.HasSuffix(line, w.delimiter()) { + if bytes.HasSuffix(line, []byte{w.delimiter()}) { break } @@ -110,7 +110,7 @@ func (w *writer) consume(r io.Reader) error { } } - line = bytes.TrimSuffix(line, w.delimiter()) + line = bytes.TrimSuffix(line, []byte{w.delimiter()}) if len(line) == 0 { break } @@ -135,16 +135,12 @@ func (w *writer) consume(r io.Reader) error { return w.flush() } -func (w *writer) delimiter() []byte { return w.options.Delimiter } +func (w *writer) delimiter() byte { return w.options.Delimiter } func (w *writer) filter() *regexp.Regexp { return w.options.Filter } // Send reads output from `r`, splits it at `opts.Delimiter`, then handles the // buffered lines using `sender`. func Send(r io.Reader, sender Sender, opts SenderOpts) error { - if len(opts.Delimiter) == 0 { - opts.Delimiter = []byte{'\n'} - } - if opts.IsPageToken == nil { opts.IsPageToken = func(_ []byte) bool { return true } } diff --git a/internal/helper/lines/send_test.go b/internal/helper/lines/send_test.go index 64b763a9b..a87f2486d 100644 --- a/internal/helper/lines/send_test.go +++ b/internal/helper/lines/send_test.go @@ -67,6 +67,7 @@ func TestLinesSend(t *testing.T) { Limit: tc.limit, IsPageToken: tc.isPageToken, Filter: tc.filter, + Delimiter: '\n', }) require.NoError(t, err) require.Equal(t, tc.output, out) diff --git a/internal/service/ref/refs.go b/internal/service/ref/refs.go index 7e127c612..6db83b80e 100644 --- a/internal/service/ref/refs.go +++ b/internal/service/ref/refs.go @@ -34,7 +34,7 @@ var ( type findRefsOpts struct { cmdArgs []git.Option - delim []byte + delim byte lines.SenderOpts } @@ -463,7 +463,7 @@ func validateFindTagRequest(in *gitalypb.FindTagRequest) error { } func paginationParamsToOpts(p *gitalypb.PaginationParameter) *findRefsOpts { - opts := &findRefsOpts{} + opts := &findRefsOpts{delim: '\n'} opts.IsPageToken = func(_ []byte) bool { return true } opts.Limit = math.MaxInt32 diff --git a/internal/service/repository/search_files.go b/internal/service/repository/search_files.go index a8bedded7..958880097 100644 --- a/internal/service/repository/search_files.go +++ b/internal/service/repository/search_files.go @@ -143,7 +143,7 @@ func (s *server) SearchFilesByName(req *gitalypb.SearchFilesByNameRequest, strea return stream.Send(&gitalypb.SearchFilesByNameResponse{Files: objs}) } - return lines.Send(cmd, lr, lines.SenderOpts{Delimiter: []byte{'\n'}, Limit: math.MaxInt32, Filter: filter}) + return lines.Send(cmd, lr, lines.SenderOpts{Delimiter: '\n', Limit: math.MaxInt32, Filter: filter}) } type searchFilesRequest interface { |