From e44ecc0cccd4ae8ae38dd71ba8b083ee83fc67a8 Mon Sep 17 00:00:00 2001 From: yusing Date: Tue, 16 Dec 2025 17:20:59 +0800 Subject: [PATCH] fix(access_log): fix slice out-of-bound panic on log rotation --- internal/logging/accesslog/access_logger.go | 1 + internal/logging/accesslog/rotate.go | 50 ++++++++++++--------- 2 files changed, 31 insertions(+), 20 deletions(-) diff --git a/internal/logging/accesslog/access_logger.go b/internal/logging/accesslog/access_logger.go index 911e7a04..a18b9ba2 100644 --- a/internal/logging/accesslog/access_logger.go +++ b/internal/logging/accesslog/access_logger.go @@ -94,6 +94,7 @@ const ( ) var bytesPool = synk.GetUnsizedBytesPool() +var sizedPool = synk.GetSizedBytesPool() func NewAccessLogger(parent task.Parent, cfg AnyConfig) (AccessLogger, error) { writers, err := cfg.Writers() diff --git a/internal/logging/accesslog/rotate.go b/internal/logging/accesslog/rotate.go index 083443d9..054b2c85 100644 --- a/internal/logging/accesslog/rotate.go +++ b/internal/logging/accesslog/rotate.go @@ -3,12 +3,11 @@ package accesslog import ( "bytes" "errors" + "fmt" "io" - "slices" "time" "github.com/rs/zerolog" - gperr "github.com/yusing/goutils/errs" "github.com/yusing/goutils/mockable" strutils "github.com/yusing/goutils/strings" ) @@ -164,31 +163,14 @@ func rotateLogFileByPolicy(file supportRotate, config *Retention, result *Rotate // Read each line and write it to the beginning of the file writePos := int64(0) - buf := bytesPool.Get() - defer func() { - bytesPool.Put(buf) - }() - // in reverse order to keep the order of the lines (from old to new) for i := len(linesToKeep) - 1; i >= 0; i-- { line := linesToKeep[i] n := line.Size - if cap(buf) < int(n) { - buf = slices.Grow(buf, int(n)-cap(buf)) - } - buf = buf[:n] - // Read the line from its original position - if _, err := file.ReadAt(buf, line.Pos); err != nil { + if err := fileContentMove(file, line.Pos, writePos, int(n)); err != nil { return false, err } - - // Write it to the new position - if _, err := file.WriteAt(buf, writePos); err != nil { - return false, err - } else if n < line.Size { - return false, gperr.Errorf("%w, writing %d bytes, only %d written", io.ErrShortWrite, line.Size, n) - } writePos += n } @@ -199,6 +181,34 @@ func rotateLogFileByPolicy(file supportRotate, config *Retention, result *Rotate return true, nil } +// fileContentMove moves the content of the file from the source position to the destination position. +// +// this is only used for moving from the back to the front of the file. +func fileContentMove(file supportRotate, srcPos, dstPos int64, size int) error { + buf := sizedPool.GetSized(size) + defer sizedPool.Put(buf) + + // Read the line from its original position + nRead, err := file.ReadAt(buf, srcPos) + if err != nil { + return err + } + if nRead != size { + return fmt.Errorf("%w, reading %d bytes, only %d read", io.ErrShortBuffer, size, nRead) + } + + // Write it to the new position + nWritten, err := file.WriteAt(buf, dstPos) + if err != nil { + return err + } + if nWritten != size { + return fmt.Errorf("%w, writing %d bytes, only %d written", io.ErrShortWrite, size, nWritten) + } + + return nil +} + // rotateLogFileBySize rotates the log file by size. // It returns the result of the rotation and an error if any. //