Skip to content

Commit

Permalink
fix(imgproxy): properly handle missing last-modified
Browse files Browse the repository at this point in the history
  • Loading branch information
neurosnap committed Jan 15, 2025
1 parent b60f3d1 commit 0c80ce2
Show file tree
Hide file tree
Showing 15 changed files with 50 additions and 39 deletions.
2 changes: 1 addition & 1 deletion cmd/scripts/clean-object-store/clean.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func main() {

var st storage.StorageServe
var err error
st, err = storage.NewStorageMinio(picoCfg.MinioURL, picoCfg.MinioUser, picoCfg.MinioPass)
st, err = storage.NewStorageMinio(logger, picoCfg.MinioURL, picoCfg.MinioUser, picoCfg.MinioPass)
bail(err)

logger.Info("fetching all users")
Expand Down
4 changes: 2 additions & 2 deletions feeds/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,9 @@ func StartApiServer() {
var st storage.StorageServe
var err error
if cfg.MinioURL == "" {
st, err = storage.NewStorageFS(cfg.StorageDir)
st, err = storage.NewStorageFS(cfg.Logger, cfg.StorageDir)
} else {
st, err = storage.NewStorageMinio(cfg.MinioURL, cfg.MinioUser, cfg.MinioPass)
st, err = storage.NewStorageMinio(cfg.Logger, cfg.MinioURL, cfg.MinioUser, cfg.MinioPass)
}

if err != nil {
Expand Down
4 changes: 2 additions & 2 deletions feeds/ssh.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,9 @@ func StartSshServer() {
var st storage.StorageServe
var err error
if cfg.MinioURL == "" {
st, err = storage.NewStorageFS(cfg.StorageDir)
st, err = storage.NewStorageFS(cfg.Logger, cfg.StorageDir)
} else {
st, err = storage.NewStorageMinio(cfg.MinioURL, cfg.MinioUser, cfg.MinioPass)
st, err = storage.NewStorageMinio(cfg.Logger, cfg.MinioURL, cfg.MinioUser, cfg.MinioPass)
}

if err != nil {
Expand Down
4 changes: 2 additions & 2 deletions imgs/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -313,9 +313,9 @@ func StartApiServer() {
var st storage.StorageServe
var err error
if cfg.MinioURL == "" {
st, err = storage.NewStorageFS(cfg.StorageDir)
st, err = storage.NewStorageFS(cfg.Logger, cfg.StorageDir)
} else {
st, err = storage.NewStorageMinio(cfg.MinioURL, cfg.MinioUser, cfg.MinioPass)
st, err = storage.NewStorageMinio(cfg.Logger, cfg.MinioURL, cfg.MinioUser, cfg.MinioPass)
}

if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion imgs/ssh.go
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ func StartSshServer() {
logger := shared.CreateLogger("imgs")
logger.Info("bootup", "registry", registryUrl, "minio", minioUrl)
dbh := postgres.NewDB(dbUrl, logger)
st, err := storage.NewStorageMinio(minioUrl, minioUser, minioPass)
st, err := storage.NewStorageMinio(logger, minioUrl, minioUser, minioPass)
if err != nil {
panic(err)
}
Expand Down
4 changes: 2 additions & 2 deletions pastes/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -373,9 +373,9 @@ func StartApiServer() {
var st storage.StorageServe
var err error
if cfg.MinioURL == "" {
st, err = storage.NewStorageFS(cfg.StorageDir)
st, err = storage.NewStorageFS(cfg.Logger, cfg.StorageDir)
} else {
st, err = storage.NewStorageMinio(cfg.MinioURL, cfg.MinioUser, cfg.MinioPass)
st, err = storage.NewStorageMinio(cfg.Logger, cfg.MinioURL, cfg.MinioUser, cfg.MinioPass)
}

if err != nil {
Expand Down
4 changes: 2 additions & 2 deletions pastes/ssh.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,9 @@ func StartSshServer() {
var st storage.StorageServe
var err error
if cfg.MinioURL == "" {
st, err = storage.NewStorageFS(cfg.StorageDir)
st, err = storage.NewStorageFS(cfg.Logger, cfg.StorageDir)
} else {
st, err = storage.NewStorageMinio(cfg.MinioURL, cfg.MinioUser, cfg.MinioPass)
st, err = storage.NewStorageMinio(cfg.Logger, cfg.MinioURL, cfg.MinioUser, cfg.MinioPass)
}

if err != nil {
Expand Down
4 changes: 2 additions & 2 deletions pgs/ssh.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,9 @@ func StartSshServer() {
var st storage.StorageServe
var err error
if cfg.MinioURL == "" {
st, err = storage.NewStorageFS(cfg.StorageDir)
st, err = storage.NewStorageFS(cfg.Logger, cfg.StorageDir)
} else {
st, err = storage.NewStorageMinio(cfg.MinioURL, cfg.MinioUser, cfg.MinioPass)
st, err = storage.NewStorageMinio(cfg.Logger, cfg.MinioURL, cfg.MinioUser, cfg.MinioPass)
}

if err != nil {
Expand Down
10 changes: 5 additions & 5 deletions pgs/web.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,9 @@ func StartApiServer() {
var st storage.StorageServe
var err error
if cfg.MinioURL == "" {
st, err = storage.NewStorageFS(cfg.StorageDir)
st, err = storage.NewStorageFS(cfg.Logger, cfg.StorageDir)
} else {
st, err = storage.NewStorageMinio(cfg.MinioURL, cfg.MinioUser, cfg.MinioPass)
st, err = storage.NewStorageMinio(cfg.Logger, cfg.MinioURL, cfg.MinioUser, cfg.MinioPass)
}

if err != nil {
Expand Down Expand Up @@ -472,8 +472,8 @@ func (web *WebRouter) ServeAsset(fname string, opts *storage.ImgProcessOpts, fro
bucket, err = web.Storage.GetBucket(shared.GetImgsBucketName(user.ID))
} else {
bucket, err = web.Storage.GetBucket(shared.GetAssetBucketName(user.ID))
project, err := web.Dbpool.FindProjectByName(user.ID, props.ProjectName)
if err != nil {
project, perr := web.Dbpool.FindProjectByName(user.ID, props.ProjectName)
if perr != nil {
logger.Info("project not found")
http.Error(w, "project not found", http.StatusNotFound)
return
Expand All @@ -499,7 +499,7 @@ func (web *WebRouter) ServeAsset(fname string, opts *storage.ImgProcessOpts, fro
}

if err != nil {
logger.Info("bucket not found")
logger.Error("bucket not found", "err", err)
http.Error(w, "bucket not found", http.StatusNotFound)
return
}
Expand Down
12 changes: 8 additions & 4 deletions pgs/web_asset_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,15 +131,19 @@ func (h *ApiAssetHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
return
}

attempts = append(attempts, fp.Filepath)
logger = logger.With("filename", fp.Filepath)
var c io.ReadCloser
fpath := fp.Filepath
attempts = append(attempts, fpath)
logger = logger.With("object", fpath)
logger.Info("serving object")
c, info, err = h.Storage.ServeObject(
h.Bucket,
fp.Filepath,
fpath,
h.ImgProcessOpts,
)
if err == nil {
if err != nil {
logger.Error("serving object", "err", err)
} else {
contents = c
assetFilepath = fp.Filepath
status = fp.Status
Expand Down
4 changes: 2 additions & 2 deletions prose/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -898,9 +898,9 @@ func StartApiServer() {
var st storage.StorageServe
var err error
if cfg.MinioURL == "" {
st, err = storage.NewStorageFS(cfg.StorageDir)
st, err = storage.NewStorageFS(cfg.Logger, cfg.StorageDir)
} else {
st, err = storage.NewStorageMinio(cfg.MinioURL, cfg.MinioUser, cfg.MinioPass)
st, err = storage.NewStorageMinio(cfg.Logger, cfg.MinioURL, cfg.MinioUser, cfg.MinioPass)
}

if err != nil {
Expand Down
4 changes: 2 additions & 2 deletions prose/ssh.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,9 @@ func StartSshServer() {
var st storage.StorageServe
var err error
if cfg.MinioURL == "" {
st, err = storage.NewStorageFS(cfg.StorageDir)
st, err = storage.NewStorageFS(cfg.Logger, cfg.StorageDir)
} else {
st, err = storage.NewStorageMinio(cfg.MinioURL, cfg.MinioUser, cfg.MinioPass)
st, err = storage.NewStorageMinio(cfg.Logger, cfg.MinioURL, cfg.MinioUser, cfg.MinioPass)
}

if err != nil {
Expand Down
8 changes: 5 additions & 3 deletions shared/storage/fs.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package storage
import (
"fmt"
"io"
"log/slog"
"os"
"path/filepath"
"strings"
Expand All @@ -12,14 +13,15 @@ import (

type StorageFS struct {
*sst.StorageFS
Logger *slog.Logger
}

func NewStorageFS(dir string) (*StorageFS, error) {
func NewStorageFS(logger *slog.Logger, dir string) (*StorageFS, error) {
st, err := sst.NewStorageFS(dir)
if err != nil {
return nil, err
}
return &StorageFS{st}, nil
return &StorageFS{st, logger}, nil
}

func (s *StorageFS) ServeObject(bucket sst.Bucket, fpath string, opts *ImgProcessOpts) (io.ReadCloser, *sst.ObjectInfo, error) {
Expand All @@ -37,7 +39,7 @@ func (s *StorageFS) ServeObject(bucket sst.Bucket, fpath string, opts *ImgProces
} else {
filePath := filepath.Join(bucket.Name, fpath)
dataURL := fmt.Sprintf("s3://%s", filePath)
rc, info, err = HandleProxy(dataURL, opts)
rc, info, err = HandleProxy(s.Logger, dataURL, opts)
}
if err != nil {
return nil, nil, err
Expand Down
8 changes: 5 additions & 3 deletions shared/storage/minio.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package storage
import (
"fmt"
"io"
"log/slog"
"os"
"path/filepath"
"strings"
Expand All @@ -12,14 +13,15 @@ import (

type StorageMinio struct {
*sst.StorageMinio
Logger *slog.Logger
}

func NewStorageMinio(address, user, pass string) (*StorageMinio, error) {
func NewStorageMinio(logger *slog.Logger, address, user, pass string) (*StorageMinio, error) {
st, err := sst.NewStorageMinio(address, user, pass)
if err != nil {
return nil, err
}
return &StorageMinio{st}, nil
return &StorageMinio{st, logger}, nil
}

func (s *StorageMinio) ServeObject(bucket sst.Bucket, fpath string, opts *ImgProcessOpts) (io.ReadCloser, *sst.ObjectInfo, error) {
Expand All @@ -37,7 +39,7 @@ func (s *StorageMinio) ServeObject(bucket sst.Bucket, fpath string, opts *ImgPro
} else {
filePath := filepath.Join(bucket.Name, fpath)
dataURL := fmt.Sprintf("s3://%s", filePath)
rc, info, err = HandleProxy(dataURL, opts)
rc, info, err = HandleProxy(s.Logger, dataURL, opts)
}
if err != nil {
return nil, nil, err
Expand Down
15 changes: 9 additions & 6 deletions shared/storage/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"encoding/hex"
"fmt"
"io"
"log/slog"
"net/http"
"os"
"path/filepath"
Expand Down Expand Up @@ -202,7 +203,7 @@ func (img *ImgProcessOpts) String() string {
return processOpts
}

func HandleProxy(dataURL string, opts *ImgProcessOpts) (io.ReadCloser, *storage.ObjectInfo, error) {
func HandleProxy(logger *slog.Logger, dataURL string, opts *ImgProcessOpts) (io.ReadCloser, *storage.ObjectInfo, error) {
imgProxyURL := os.Getenv("IMGPROXY_URL")
imgProxySalt := os.Getenv("IMGPROXY_SALT")
imgProxyKey := os.Getenv("IMGPROXY_KEY")
Expand Down Expand Up @@ -242,13 +243,15 @@ func HandleProxy(dataURL string, opts *ImgProcessOpts) (io.ReadCloser, *storage.
lastModified := res.Header.Get("Last-Modified")
parsedTime, err := time.Parse(time.RFC1123, lastModified)
if err != nil {
return nil, nil, fmt.Errorf("decoding last-modified: %w", err)
logger.Error("decoding last-modified", "err", err)
}
info := &storage.ObjectInfo{
Size: res.ContentLength,
LastModified: parsedTime,
ETag: trimEtag(res.Header.Get("etag")),
Metadata: res.Header,
Size: res.ContentLength,
ETag: trimEtag(res.Header.Get("etag")),
Metadata: res.Header,
}
if !parsedTime.IsZero() {
info.LastModified = parsedTime
}

return res.Body, info, nil
Expand Down

0 comments on commit 0c80ce2

Please sign in to comment.