Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rollback pkg/errors portion of #298 #299

Merged
merged 1 commit into from
Aug 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 10 additions & 9 deletions boundary.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,19 @@ package enmime
import (
"bufio"
"bytes"
"errors"
"fmt"
stderrors "errors"
"io"
"unicode"

"github.com/pkg/errors"
)

// This constant needs to be at least 76 for this package to work correctly. This is because
// \r\n--separator_of_len_70- would fill the buffer and it wouldn't be safe to consume a single byte
// from it.
const peekBufferSize = 4096

var errNoBoundaryTerminator = errors.New("expected boundary not present")
var errNoBoundaryTerminator = stderrors.New("expected boundary not present")

type boundaryReader struct {
finished bool // No parts remain when finished
Expand Down Expand Up @@ -82,7 +83,7 @@ func (b *boundaryReader) Read(dest []byte) (n int, err error) {
var cs []byte
cs, err = b.r.Peek(1)
if err != nil && err != io.EOF {
return 0, fmt.Errorf("failed to read content: %w", err)
return 0, errors.WithStack(err)
}
// Ensure that we can switch on the first byte of 'cs' without panic.
if len(cs) > 0 {
Expand Down Expand Up @@ -131,7 +132,7 @@ func (b *boundaryReader) Read(dest []byte) (n int, err error) {
}
return n, io.EOF
default:
return 0, fmt.Errorf("failed to read content: %w", err)
return 0, errors.WithStack(err)
}
}
case io.EOF:
Expand All @@ -153,11 +154,11 @@ func (b *boundaryReader) Read(dest []byte) (n int, err error) {
break
}

return 0, fmt.Errorf("failed to read content: %w", err)
return 0, errors.WithStack(err)
}

if err = b.buffer.WriteByte(next); err != nil {
return 0, fmt.Errorf("failed to read content: %w", err)
return 0, errors.WithStack(err)
}
}

Expand Down Expand Up @@ -194,7 +195,7 @@ func (b *boundaryReader) Next() (bool, error) {
if err == nil || err == io.EOF {
break
} else if err != bufio.ErrBufferFull || len(segment) == 0 {
return false, fmt.Errorf("failed to move over the boundary: %w", err)
return false, errors.WithStack(err)
}
}

Expand Down Expand Up @@ -222,7 +223,7 @@ func (b *boundaryReader) Next() (bool, error) {
continue
}
b.finished = true
return false, fmt.Errorf("expecting boundary %q, got %q: %w", string(b.prefix), string(line), errNoBoundaryTerminator)
return false, errors.WithMessagef(errNoBoundaryTerminator, "expecting boundary %q, got %q", string(b.prefix), string(line))
}
}

Expand Down
7 changes: 4 additions & 3 deletions boundary_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@ package enmime
import (
"bufio"
"bytes"
"errors"
"io"
"strings"
"testing"

"github.com/pkg/errors"
)

func TestBoundaryReader(t *testing.T) {
Expand Down Expand Up @@ -462,15 +463,15 @@ func TestBoundaryReaderReadErrors(t *testing.T) {
if n != 0 {
t.Fatal("Read() should not have read any bytes, failed")
}
if !errors.Is(err, bufio.ErrBufferFull) {
if errors.Cause(err) != bufio.ErrBufferFull {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

errors.Is() is probably better here, ultimately.

t.Fatal("Read() should have returned bufio.ErrBufferFull error, failed")
}
// Next method to return a non io.EOF error.
next, err := br.Next()
if next {
t.Fatal("Next() should have returned false, failed")
}
if !errors.Is(err, bufio.ErrBufferFull) {
if errors.Cause(err) != bufio.ErrBufferFull {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

errors.Is() is probably better here, ultimately.

t.Fatal("Read() should have returned bufio.ErrBufferFull error, failed")
}
}
Expand Down
4 changes: 3 additions & 1 deletion envelope.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import (
"github.com/jhillyerd/enmime/internal/coding"
"github.com/jhillyerd/enmime/internal/textproto"
"github.com/jhillyerd/enmime/mediatype"

"github.com/pkg/errors"
)

// Envelope is a simplified wrapper for MIME email messages.
Expand Down Expand Up @@ -156,7 +158,7 @@ func (p Parser) ReadEnvelope(r io.Reader) (*Envelope, error) {
// Read MIME parts from reader
root, err := p.ReadParts(r)
if err != nil {
return nil, fmt.Errorf("failed to ReadParts: %w", err)
return nil, errors.WithMessage(err, "Failed to ReadParts")
}
return p.EnvelopeFromPart(root)
}
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ require (
github.com/go-test/deep v1.0.7
github.com/gogs/chardet v0.0.0-20191104214054-4b6791f73a28
github.com/jaytaylor/html2text v0.0.0-20200412013138-3577fbdbcff7
github.com/pkg/errors v0.9.1
github.com/stretchr/testify v1.7.0
golang.org/x/text v0.8.0
)
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ github.com/mattn/go-runewidth v0.0.12 h1:Y41i/hVW3Pgwr8gV+J23B9YEY0zxjptBuCWEaxm
github.com/mattn/go-runewidth v0.0.12/go.mod h1:RAqKPSqVFrSLVXbA8x7dzmKdmGzieGRCM46jaSJTDAk=
github.com/olekukonko/tablewriter v0.0.5 h1:P2Ga83D34wi1o9J6Wh1mRuqd4mF/x/lgBS7N7AbDhec=
github.com/olekukonko/tablewriter v0.0.5/go.mod h1:hPp6KlRPjbx+hW8ykQs1w3UBbZlj6HuIJcUGPhkA7kY=
github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/rivo/uniseg v0.1.0/go.mod h1:J6wj4VEh+S6ZtnVlnTBMWIodfgj8LQOQFoIToxlJtxc=
Expand Down
7 changes: 3 additions & 4 deletions header.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import (
"github.com/jhillyerd/enmime/internal/stringutil"
"github.com/jhillyerd/enmime/internal/textproto"
"github.com/jhillyerd/enmime/mediatype"

"github.com/pkg/errors"
)

const (
Expand Down Expand Up @@ -192,10 +194,7 @@ line:
buf.Write([]byte{'\r', '\n'})
tr := textproto.NewReader(bufio.NewReader(buf))
header, err := tr.ReadEmailMIMEHeader()
if err != nil {
return nil, fmt.Errorf("failed to read header: %w", err)
}
return header, nil
return header, errors.WithStack(err)
}

// decodeToUTF8Base64Header decodes a MIME header per RFC 2047, reencoding to =?utf-8b?
Expand Down
10 changes: 6 additions & 4 deletions inspect.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@ package enmime
import (
"bufio"
"bytes"
"errors"
"io"

"github.com/jhillyerd/enmime/internal/coding"
"github.com/jhillyerd/enmime/internal/textproto"

"github.com/pkg/errors"
)

var defaultHeadersList = []string{
Expand Down Expand Up @@ -39,9 +40,10 @@ func DecodeHeaders(b []byte, addtlHeaders ...string) (textproto.MIMEHeader, erro
b = ensureHeaderBoundary(b)
tr := textproto.NewReader(bufio.NewReader(bytes.NewReader(b)))
headers, err := tr.ReadMIMEHeader()

// io.EOF is expected
if err != nil && !errors.Is(err, io.EOF) {
switch errors.Cause(err) {
case nil, io.EOF:
// carry on, io.EOF is expected
default:
Comment on lines +43 to +46
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previous logic, using errors.Is(), is probably better here.

return nil, err
}
headerList := defaultHeadersList
Expand Down
3 changes: 2 additions & 1 deletion mediatype/mediatype.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

"github.com/jhillyerd/enmime/internal/coding"
"github.com/jhillyerd/enmime/internal/stringutil"
"github.com/pkg/errors"
)

const (
Expand Down Expand Up @@ -43,7 +44,7 @@ func Parse(ctype string) (mtype string, params map[string]string, invalidParams
if err.Error() == "mime: no media type" {
return "", nil, nil, nil
}
return "", nil, nil, fmt.Errorf("parsing failed: %w", err)
return "", nil, nil, errors.WithStack(err)
}

if mtype == ctPlaceholder {
Expand Down
19 changes: 10 additions & 9 deletions part.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ import (
"bufio"
"bytes"
"encoding/base64"
"errors"
"fmt"
"io"
"math/rand"
"mime/quotedprintable"
Expand All @@ -17,6 +15,8 @@ import (
"github.com/jhillyerd/enmime/internal/coding"
"github.com/jhillyerd/enmime/internal/textproto"
"github.com/jhillyerd/enmime/mediatype"

"github.com/pkg/errors"
)

const (
Expand Down Expand Up @@ -195,7 +195,7 @@ func (p *Part) convertFromDetectedCharset(r io.Reader, readPartErrorPolicy ReadP

buf, err := p.readPartContent(r, readPartErrorPolicy)
if err != nil {
return nil, err
return nil, errors.WithStack(err)
}

cs, err := cd.DetectBest(buf)
Expand Down Expand Up @@ -304,7 +304,7 @@ func (p *Part) decodeContent(r io.Reader, readPartErrorPolicy ReadPartErrorPolic
// Decode and store content.
content, err := p.readPartContent(contentReader, readPartErrorPolicy)
if err != nil {
return p.base64CorruptInputCheck(fmt.Errorf("failed to decode content: %w", err))
return p.base64CorruptInputCheck(errors.WithStack(err))
}
p.Content = content
// Collect base64 errors.
Expand All @@ -325,11 +325,12 @@ func (p *Part) decodeContent(r io.Reader, readPartErrorPolicy ReadPartErrorPolic
//
// It can be used to create ReadPartErrorPolicy functions.
func IsBase64CorruptInputError(err error) bool {
if err == nil {
switch errors.Cause(err).(type) {
case base64.CorruptInputError:
return true
default:
return false
}
_, ok := err.(base64.CorruptInputError)
return ok
}
Comment on lines 327 to 334
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

func IsBase64CorruptInputError(err error) bool {
	 var errCorrupt base64.CorruptInputError
	 return errors.As(err, &errCorrupt)
}


// base64CorruptInputCheck will avoid fatal failure on corrupt base64 input
Expand Down Expand Up @@ -406,7 +407,7 @@ func parseParts(parent *Part, reader *bufio.Reader) error {
br := newBoundaryReader(reader, parent.Boundary)
for indexPartID := 1; true; indexPartID++ {
next, err := br.Next()
if err != nil && !errors.Is(err, io.EOF) {
if err != nil && errors.Cause(err) != io.EOF {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

!errors.Is(io.EOF) is probably better here, ultimately.

return err
}
if br.unbounded {
Expand Down Expand Up @@ -464,7 +465,7 @@ func parseParts(parent *Part, reader *bufio.Reader) error {
// Store any content following the closing boundary marker into the epilogue.
epilogue, err := io.ReadAll(reader)
if err != nil {
return fmt.Errorf("failed to parse parts: %w", err)
return errors.WithStack(err)
}
parent.Epilogue = epilogue

Expand Down