log/slog logging + linting

.github/workflows/pr-and-merge.yaml

@@ -0,0 +1,34 @@
+---
+name: CI
+on:
+  pull_request:
+
+jobs:
+  golangci-lint:
+    name: golangci-lint
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout code
+        uses: actions/checkout@v4
+      - name: Install Go
+        uses: actions/setup-go@v5
+        with:
+          go-version: "1.23"
+      - name: Run golangci-lint
+        uses: golangci/golangci-lint-action@v6
+        with:
+          version: v1.63.4
+
+  tests:
+    name: tests
+    needs: golangci-lint
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout code
+        uses: actions/checkout@v4
+      - name: Install Go
+        uses: actions/setup-go@v5
+        with:
+          go-version: "1.23"
+      - name: Test code
+        run: go test -race -v ./...

.golangci.yml

@@ -0,0 +1,74 @@
+---
+linters-settings:
+  stylecheck:
+    # STxxxx checks in https://staticcheck.io/docs/configuration/options/#checks
+    # ST1000 Incorrect or missing package comment.
+    checks: ["all", "-ST1000"]
+  misspell:
+    locale: US
+  gci:
+    sections:
+      - standard # Standard section: captures all standard packages.
+      - default # Default section: contains all imports that could not be matched to another section type.
+      - localmodule # Local module section: contains all local packages. This section is not present unless explicitly enabled.
+
+linters:
+  disable-all: true
+  enable:
+    - asasalint
+    - asciicheck
+    - bidichk
+    - errcheck
+    - errchkjson
+    - errorlint
+    - exhaustive
+    - fatcontext
+    - gci
+    - gocritic
+    - godox
+    - gofmt
+    - gofumpt
+    - goimports
+    - gosec
+    - govet
+    - ineffassign
+    - misspell
+    - nolintlint
+    - predeclared
+    - reassign
+    - sloglint
+    - staticcheck
+    - stylecheck
+    - unparam
+    - unused
+    - usestdlibvars
+    - wastedassign
+    - whitespace
+    - wsl
+
+issues:
+  exclude-use-default: false
+  exclude-rules:
+    - path: '_test\.go'
+      linters:
+        - errcheck
+
+    # TLS is not needed in tests.
+    - text: "G402: TLS InsecureSkipVerify set true"
+      path: '_test\.go'
+      linters:
+        - gosec
+
+    # Ignoring errors of .Close() is ok.
+    - text: "Error return value of `.*\\.Close` is not checked"
+      linters:
+        - errcheck
+
+    # Unhandled errors are checked by errcheck.
+    - text: G104
+      linters:
+        - gosec
+
+  # Ensure that we can see all issues at once.
+  max-issues-per-linter: 0
+  max-same-issues: 0

README.md

@@ -23,7 +23,6 @@ server := &Server{
     IdleTimeout:    350 * time.Second,
     WriteTimeout:   2 * time.Minute,
     ReadTimeout:    10 * time.Second,
-    ErrorLog:       log, // whichever log you prefer that fit the interface
     MaxMessageSize: 1000,
 }
 

go.mod

@@ -1,11 +1,10 @@
 module github.com/dotse/epp-lib
 
-go 1.18
+go 1.23
 
 require (
-	github.com/beevik/etree v1.1.0
-	github.com/pkg/errors v0.9.1
-	github.com/stretchr/testify v1.8.0
+	github.com/beevik/etree v1.5.0
+	github.com/stretchr/testify v1.10.0
 )
 
 require (

go.sum

@@ -1,19 +1,12 @@
-github.com/beevik/etree v1.1.0 h1:T0xke/WvNtMoCqgzPhkX2r4rjY3GDZFi+FjpRZY2Jbs=
-github.com/beevik/etree v1.1.0/go.mod h1:r8Aw8JqVegEf0w2fDnATrX9VpkMcyFeM0FhwO62wh+A=
-github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
+github.com/beevik/etree v1.5.0 h1:iaQZFSDS+3kYZiGoc9uKeOkUY3nYMXOKLl6KIJxiJWs=
+github.com/beevik/etree v1.5.0/go.mod h1:gPNJNaBGVZ9AwsidazFZyygnd+0pAU38N4D+WemwKNs=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
-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/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
-github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw=
-github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
-github.com/stretchr/testify v1.8.0 h1:pSgiaMZlXftHpm5L7V1+rVB+AZJydKsMxsQBIJw4PKk=
-github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU=
+github.com/stretchr/testify v1.10.0 h1:Xv5erBjTwe/5IxqUQTdXv5kgmIvbHo3QQyRwhJsOfJA=
+github.com/stretchr/testify v1.10.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
-gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
 gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
 gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=

mux.go

@@ -3,6 +3,7 @@ package epplib
 import (
 	"context"
 	"io"
+	"log/slog"
 
 	"github.com/beevik/etree"
 )
@@ -11,8 +12,6 @@ import (
 type CommandMux struct {
 	greetingCommand CommandFunc
 	handlers        []handler
-
-	Logger Logger
 }
 
 // GetGreeting returns a greeting.
@@ -27,7 +26,10 @@ func (c *CommandMux) Handle(ctx context.Context, rw *ResponseWriter, cmd io.Read
 
 	_, err := doc.ReadFrom(cmd)
 	if err != nil {
-		c.Logger.Infof("could not read command, err: %s", err)
+		slog.InfoContext(ctx, "could not read command",
+			slog.Any("err", err),
+		)
+
 		rw.CloseAfterWrite()
 
 		return
@@ -40,7 +42,7 @@ func (c *CommandMux) Handle(ctx context.Context, rw *ResponseWriter, cmd io.Read
 		}
 	}
 
-	c.Logger.Infof("unknown command")
+	slog.InfoContext(ctx, "unknown command")
 	rw.CloseAfterWrite()
 }
 

mux_test.go

@@ -32,9 +32,7 @@ func TestMux_Handle(t *testing.T) {
 		barCalled bool
 	)
 
-	cm := &CommandMux{
-		Logger: &DummyLogger{},
-	}
+	cm := &CommandMux{}
 
 	cm.Bind(
 		"//foo[namespace-uri()='urn:ietf:params:xml:ns:epp-1.0']",
@@ -115,12 +113,15 @@ func TestMux_Bind(t *testing.T) {
 	for _, tc := range cases {
 		t.Run(tc.name, func(t *testing.T) {
 			cm := &CommandMux{}
+
 			if tc.wantPanic {
 				assert.Panics(t, func() {
 					cm.Bind(tc.path, tc.handler)
 				})
+
 				return
 			}
+
 			cm.Bind(tc.path, tc.handler)
 			assert.Equal(t, len(cm.handlers), tc.wantLength)
 		})

read_write.go

@@ -4,10 +4,9 @@ import (
 	"bytes"
 	"encoding/binary"
 	"errors"
+	"fmt"
 	"io"
 	"math"
-
-	pkgerrors "github.com/pkg/errors"
 )
 
 // ErrMessageSize represent an error where the incoming message size is bigger than allowed.
@@ -16,29 +15,34 @@ var ErrMessageSize = errors.New("message size exceeds limit")
 // MessageReader returns an io.Reader that reads one message according to the
 // message size header. Blocks until a message size header is read.
 // The message limit is in bytes.
-func MessageReader(r io.Reader, msgLimit int64) (io.Reader, error) {
+func MessageReader(r io.Reader, msgLimit uint32) (io.Reader, error) {
 	// Get the size of the message we are going to read in the future.
 	// https://tools.ietf.org/html/rfc5734#section-4
+	//
+	// Note:The RFC doesn't explicitly state "signed" or "unsigned", but based
+	// on its description the field is clearly meant to represent a
+	// non-negative total length. Since a length can't be negative, it's best
+	// to interpret the 32 bits as an unsigned integer, which is what we have
+	// done historically.
 	var totalSize uint32
 
 	err := binary.Read(r, binary.BigEndian, &totalSize)
 	if err != nil {
 		return nil, err
 	}
 
-	// The size that the client sent is the size including the bytes that tells
-	// the size so we need to subtract that to get the actual message size.
-	messageSize := int64(totalSize - uint32(binary.Size(totalSize)))
-
-	if messageSize <= 0 || (msgLimit != 0 && messageSize > msgLimit) {
-		return nil, pkgerrors.Wrapf(ErrMessageSize, "incoming message size %d", messageSize)
+	// The length of the EPP XML instance is determined by subtracting four
+	// octets from the total length of the data unit. The four octets are the
+	// 32bit length field.
+	if totalSize <= 4 || (msgLimit > 0 && totalSize > msgLimit) {
+		return nil, fmt.Errorf("%w: incoming message size %d", ErrMessageSize, totalSize)
 	}
 
 	// Since we know the message size of the future message we can create a
 	// reader that will read exactly that size and then return an EOF. That way
 	// reading from the connection will always read the number of bytes that
 	// the client said the message is.
-	return io.LimitReader(r, messageSize), nil
+	return io.LimitReader(r, int64(totalSize-4)), nil
 }
 
 // ResponseWriter is an io.Writer that buffers response data before writing it
@@ -69,20 +73,18 @@ type MessageBuffer struct {
 
 // FlushTo flushes the buffer to dst after writing the message size header.
 func (mb *MessageBuffer) FlushTo(dst io.Writer) error {
-	if mb.Len() == 0 {
-		// Nothing to write.
-		return nil
-	}
 	// Begin by writing the len(b) as Big Endian uint32, including the
 	// size of the content length header.
 	// https://tools.ietf.org/html/rfc5734#section-4
 	contentSize := mb.Len()
-	headerSize := binary.Size(uint32(contentSize))
-	totalSize := contentSize + headerSize
+	if contentSize == 0 {
+		// Nothing to write.
+		return nil
+	}
 
-	// Bounds check.
-	if totalSize > math.MaxUint32 {
-		return errors.New("content is too large")
+	totalSize := contentSize + 4 // 4 bytes for the content size header.
+	if totalSize <= 4 || totalSize > math.MaxUint32 {
+		return errors.New("invalid message size")
 	}
 
 	err := binary.Write(dst, binary.BigEndian, uint32(totalSize))