From 5a2c406897d65bb40afe287afe9cac9cd2baaa23 Mon Sep 17 00:00:00 2001 From: Justin Steven Date: Mon, 13 Jun 2022 21:50:26 +1000 Subject: [PATCH] Rework #146. Fixes various errors relating to input length. (#150) * Revert "Fix notify silently fails (#146)" This reverts commit 173f91472fd73c410969d063776ff1a7224fd6d1. * Emit logged lines to stdout, not stderr Fixes #134 (this was previously fixed but got backed out in landing #130) * Split lines as per CharLimit in non-bulk mode Resolves #137 * Bubble up any Scanner errors This surfaces the error that's causing #138: 'bufio.Scanner: token too long' * Fix infinite loop with -char-limit <= len('...') We're using '...' to indicate that a line has been truncated. If -char-limit was less than the length of this ellipsis string, the scanner would never terminate. Raise an error if the charLimit given to a splitter is <= len('...') * Ensure we never allow the Scanner buffer to fill We know we never need more than CharLimit chars from the Scanner in one go First of all, we ensure that the Scanner has a buffer which can hold at least CharLimit chars. Then we handle cases where the Scanner wants more data but we don't need it to get more data. Thus it should never end up in a place where its internal buffer is filled, and it should never return bufio.ErrToLong Fixes #138 * fmt.print => gologger Co-authored-by: mzack --- internal/runner/runner.go | 42 ++++++++++---------- internal/runner/util.go | 82 +++++++++++++++++++++++++++++---------- 2 files changed, 83 insertions(+), 41 deletions(-) diff --git a/internal/runner/runner.go b/internal/runner/runner.go index c51bb29..9f87c9f 100644 --- a/internal/runner/runner.go +++ b/internal/runner/runner.go @@ -84,6 +84,7 @@ func (r *Runner) Run() error { var inFile *os.File var err error + var splitter bufio.SplitFunc switch { case r.options.Data != "": @@ -97,38 +98,37 @@ func (r *Runner) Run() error { return errors.New("notify works with stdin or file using -data flag") } - info, err := inFile.Stat() - if err != nil { - return errors.Wrap(err, "could not get file info") - } br := bufio.NewScanner(inFile) - maxSize := int(info.Size()) - buffer := make([]byte, 0, maxSize) - br.Buffer(buffer, maxSize) + + if r.options.CharLimit > bufio.MaxScanTokenSize { + // Satisfy the condition of our splitters, which is that charLimit is <= the size of the bufio.Scanner buffer + buffer := make([]byte, 0, r.options.CharLimit) + br.Buffer(buffer, r.options.CharLimit) + } if r.options.Bulk { - br.Split(bulkSplitter(r.options.CharLimit)) + splitter, err = bulkSplitter(r.options.CharLimit) + } else { + splitter, err = lineLengthSplitter(r.options.CharLimit) + } + + if err != nil { + return err } + + br.Split(splitter) + for br.Scan() { msg := br.Text() - if len(msg) > r.options.CharLimit { - // send the msg in chunks of length charLimit - for _, chunk := range SplitInChunks(msg, r.options.CharLimit) { - //nolint:errcheck - r.sendMessage(chunk) - } - } else { - //nolint:errcheck - r.sendMessage(msg) - } - + //nolint:errcheck + r.sendMessage(msg) } - return nil + return br.Err() } func (r *Runner) sendMessage(msg string) error { if len(msg) > 0 { - gologger.Print().Msgf("%s\n", msg) + gologger.Silent().Msgf("%s\n", msg) err := r.providers.Send(msg) if err != nil { return err diff --git a/internal/runner/util.go b/internal/runner/util.go index 284f920..60bf889 100644 --- a/internal/runner/util.go +++ b/internal/runner/util.go @@ -2,13 +2,18 @@ package runner import ( "bufio" - "math" + "fmt" ) var ellipsis = []byte("...") -// Return a bufio.SplitFunc that tries to split on newlines while giving as many bytes that are <= charLimit each time -func bulkSplitter(charLimit int) bufio.SplitFunc { +// Return a bufio.SplitFunc that splits on as few newlines as possible +// while giving as many bytes that are <= charLimit each time. +// Note: charLimit must be <= the buffer underlying the bufio.Scanner +func bulkSplitter(charLimit int) (bufio.SplitFunc, error) { + if charLimit <= len(ellipsis) { + return nil, fmt.Errorf("charLimit must be > %d", len(ellipsis)) + } return func(data []byte, atEOF bool) (advance int, token []byte, err error) { var lineAdvance int var line []byte @@ -24,8 +29,24 @@ func bulkSplitter(charLimit int) bufio.SplitFunc { break } - // We need more data - return 0, nil, nil + if len(data) < charLimit { + // We need more data + return 0, nil, nil + } else { + // We have enough data, but bufio.ScanLines couldn't see a newline in what's left + // If we handle this then we can assure the bufio.Scanner will never give bufio.ErrTooLong + if len(token) == 0 { + // Even just the first line is too much + // Truncate and give it + token = append(token, data[:charLimit-len(ellipsis)]...) + token = append(token, ellipsis...) + advance = charLimit - len(ellipsis) + return advance, token, nil + } else { + // Give what we had + break + } + } } if len(token)+len(line) > charLimit { @@ -54,23 +75,44 @@ func bulkSplitter(charLimit int) bufio.SplitFunc { token = token[:len(token)-1] } return - } + }, nil } -// SplitInChunks splits a string into chunks of size charLimit -func SplitInChunks(data string, charLimit int) []string { - length := len(data) - noOfChunks := int(math.Ceil(float64(length) / float64(charLimit))) - chunks := make([]string, noOfChunks) - var start, stop int +// Return a bufio.SplitFunc that splits on all newlines +// while giving as many bytes that are <= charLimit each time. +// Note: charLimit must be <= the buffer underlying the bufio.Scanner +func lineLengthSplitter(charLimit int) (bufio.SplitFunc, error) { + if charLimit <= len(ellipsis) { + return nil, fmt.Errorf("charLimit must be > %d", len(ellipsis)) + } + return func(data []byte, atEOF bool) (advance int, token []byte, err error) { + var line []byte - for i := 0; i < noOfChunks; i += 1 { - start = i * charLimit - stop = start + charLimit - if stop > length { - stop = length + // Get a line + advance, line, err = bufio.ScanLines(data[advance:], atEOF) + + if !atEOF && (err != nil || line == nil) { + if len(data) < charLimit { + // We need more data + return 0, nil, nil + } else { + // We have enough data, but bufio.ScanLines couldn't see a newline in it + // If we handle this then we can assure the bufio.Scanner will never give bufio.ErrTooLong + token = append(token, data[:charLimit-len(ellipsis)]...) + token = append(token, ellipsis...) + advance = charLimit - len(ellipsis) + return advance, token, nil + } } - chunks[i] = data[start:stop] - } - return chunks + + if len(line) > charLimit { + // Got too much + token = append(token, line[:charLimit-len(ellipsis)]...) + token = append(token, ellipsis...) + advance = charLimit - len(ellipsis) + return + } + + return advance, line, err + }, nil }