From 919501917013576b364447e50b46029b049a48a1 Mon Sep 17 00:00:00 2001 From: QiuSW <105186638@qq.com> Date: Wed, 29 Jul 2026 17:01:52 +0800 Subject: [PATCH] fix(t244): accept headerless ERP images safely --- .../internal/platform/shunyunbao/image.go | 72 +++++- .../platform/shunyunbao/image_test.go | 228 ++++++++++++++++++ docs/current-state.md | 11 +- docs/tasks/T-244.md | 31 ++- 4 files changed, 324 insertions(+), 18 deletions(-) diff --git a/backend-api/internal/platform/shunyunbao/image.go b/backend-api/internal/platform/shunyunbao/image.go index bd288fe..2e57c46 100644 --- a/backend-api/internal/platform/shunyunbao/image.go +++ b/backend-api/internal/platform/shunyunbao/image.go @@ -1,7 +1,11 @@ package shunyunbao import ( + "bufio" + "bytes" "context" + "errors" + "io" "mime" "net/http" "net/url" @@ -15,6 +19,7 @@ import ( const ( ProductImagePath = "/api/p/file" maxProductImageBytes = 20 << 20 + productImageSniffLen = 512 ) func (manager *SessionManager) FetchProductImage( @@ -86,11 +91,17 @@ func (manager *SessionManager) FetchProductImage( domain.ErrFreightSourceUnavailable } } - mediaType, _, err := mime.ParseMediaType( + if response.ContentLength > maxProductImageBytes { + response.Body.Close() + manager.logERPBinaryResponse(request, response) + return usecase.FreightSourceImage{}, + domain.ErrFreightImageInvalid + } + mediaType, content, err := productImageContent( response.Header.Get("Content-Type"), + response.Body, ) - if err != nil || !supportedProductImageType(mediaType) || - response.ContentLength > maxProductImageBytes { + if err != nil { response.Body.Close() manager.logERPBinaryResponse(request, response) return usecase.FreightSourceImage{}, @@ -98,11 +109,64 @@ func (manager *SessionManager) FetchProductImage( } manager.logERPBinaryResponse(request, response) return usecase.FreightSourceImage{ - Content: response.Body, + Content: content, MediaType: mediaType, }, nil } +func productImageContent( + contentType string, + body io.ReadCloser, +) (string, io.ReadCloser, error) { + contentType = strings.TrimSpace(contentType) + if contentType != "" { + mediaType, _, err := mime.ParseMediaType(contentType) + if err != nil { + return "", nil, err + } + mediaType = strings.ToLower(mediaType) + if supportedProductImageType(mediaType) { + return mediaType, body, nil + } + if mediaType != "application/octet-stream" { + return "", nil, domain.ErrFreightImageInvalid + } + } + + reader := bufio.NewReaderSize(body, productImageSniffLen) + prefix, err := reader.Peek(productImageSniffLen) + if err != nil && !errors.Is(err, io.EOF) { + return "", nil, err + } + if len(prefix) == 0 { + return "", nil, domain.ErrFreightImageInvalid + } + mediaType := detectProductImageType(prefix) + if !supportedProductImageType(mediaType) { + return "", nil, domain.ErrFreightImageInvalid + } + return mediaType, struct { + io.Reader + io.Closer + }{ + Reader: reader, + Closer: body, + }, nil +} + +func detectProductImageType(prefix []byte) string { + mediaType := strings.ToLower(http.DetectContentType(prefix)) + if supportedProductImageType(mediaType) { + return mediaType + } + if len(prefix) >= 12 && + bytes.Equal(prefix[:4], []byte("RIFF")) && + bytes.Equal(prefix[8:12], []byte("WEBP")) { + return "image/webp" + } + return mediaType +} + func supportedProductImageType(value string) bool { switch strings.ToLower(value) { case "image/jpeg", "image/png", "image/webp": diff --git a/backend-api/internal/platform/shunyunbao/image_test.go b/backend-api/internal/platform/shunyunbao/image_test.go index b15a820..529396f 100644 --- a/backend-api/internal/platform/shunyunbao/image_test.go +++ b/backend-api/internal/platform/shunyunbao/image_test.go @@ -1,8 +1,12 @@ package shunyunbao import ( + "bytes" "context" "errors" + "image" + "image/color" + "image/jpeg" "io" "net/http" "net/http/httptest" @@ -10,6 +14,7 @@ import ( "testing" "cmroubao/backend-api/internal/domain" + "cmroubao/backend-api/internal/platform/assetstore" ) func TestSessionManagerFetchProductImageUsesFixedAuthenticatedEndpoint( @@ -56,6 +61,219 @@ func TestSessionManagerFetchProductImageUsesFixedAuthenticatedEndpoint( } } +func TestSessionManagerFetchProductImageSniffsMissingContentType( + t *testing.T, +) { + body := []byte{0xff, 0xd8, 0xff, 0xe0, 0x00, 0x10, 'J', 'F', 'I', 'F'} + manager, closeServer := imageSessionManager( + t, + func(writer http.ResponseWriter, _ *http.Request) { + writer.WriteHeader(http.StatusOK) + _, _ = writer.Write(body) + }, + ) + defer closeServer() + + image, err := manager.FetchProductImage(context.Background(), "190") + if err != nil { + t.Fatalf("FetchProductImage() error = %v", err) + } + content, err := io.ReadAll(image.Content) + if closeErr := image.Content.Close(); closeErr != nil { + t.Fatalf("close image content: %v", closeErr) + } + if err != nil || image.MediaType != "image/jpeg" || + !bytes.Equal(content, body) { + t.Fatalf( + "image = %q / %x / %v", + image.MediaType, + content, + err, + ) + } +} + +func TestMissingContentTypeImageCanBeNormalizedAndStored(t *testing.T) { + var encoded bytes.Buffer + source := image.NewRGBA(image.Rect(0, 0, 2, 2)) + source.Set(0, 0, color.RGBA{R: 255, A: 255}) + source.Set(1, 0, color.RGBA{G: 255, A: 255}) + source.Set(0, 1, color.RGBA{B: 255, A: 255}) + source.Set(1, 1, color.RGBA{R: 255, G: 255, A: 255}) + if err := jpeg.Encode( + &encoded, + source, + &jpeg.Options{Quality: 90}, + ); err != nil { + t.Fatalf("jpeg.Encode() error = %v", err) + } + manager, closeServer := imageSessionManager( + t, + func(writer http.ResponseWriter, _ *http.Request) { + writer.WriteHeader(http.StatusOK) + _, _ = writer.Write(encoded.Bytes()) + }, + ) + defer closeServer() + + sourceImage, err := manager.FetchProductImage( + context.Background(), + "190", + ) + if err != nil { + t.Fatalf("FetchProductImage() error = %v", err) + } + defer sourceImage.Content.Close() + files, err := assetstore.New(t.TempDir()) + if err != nil { + t.Fatalf("assetstore.New() error = %v", err) + } + stored, err := files.Put( + context.Background(), + "00000000-0000-4000-8000-000000000190", + sourceImage.MediaType, + sourceImage.Content, + ) + if err != nil { + t.Fatalf("Put() error = %v", err) + } + if sourceImage.MediaType != "image/jpeg" || + stored.MediaType != domain.NormalizedImageMediaType || + stored.SizeBytes <= 0 || stored.StorageKey == "" { + t.Fatalf("stored image = %+v / %q", stored, sourceImage.MediaType) + } + content, err := files.Open(context.Background(), stored.StorageKey) + if err != nil { + t.Fatalf("Open() error = %v", err) + } + defer content.Close() + _, format, err := image.Decode(content) + if err != nil || format != "jpeg" { + t.Fatalf("stored format = %q, %v", format, err) + } +} + +func TestProductImageContentSniffsGenericBinaryAndPreservesStream( + t *testing.T, +) { + tests := []struct { + name string + contentType string + body []byte + wantType string + }{ + { + name: "missing JPEG", + body: []byte{0xff, 0xd8, 0xff, 0xe1, 'j', 'p', 'e', 'g'}, + wantType: "image/jpeg", + }, + { + name: "generic PNG", + contentType: "application/octet-stream", + body: []byte{ + 0x89, 'P', 'N', 'G', '\r', '\n', 0x1a, '\n', 'p', 'n', 'g', + }, + wantType: "image/png", + }, + { + name: "generic WebP", + contentType: "APPLICATION/OCTET-STREAM", + body: []byte{ + 'R', 'I', 'F', 'F', 0x04, 0x00, 0x00, 0x00, + 'W', 'E', 'B', 'P', + }, + wantType: "image/webp", + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + body := &trackingImageBody{ + Reader: bytes.NewReader(test.body), + } + mediaType, content, err := productImageContent( + test.contentType, + body, + ) + if err != nil { + t.Fatalf("productImageContent() error = %v", err) + } + got, err := io.ReadAll(content) + if err != nil || mediaType != test.wantType || + !bytes.Equal(got, test.body) { + t.Fatalf( + "content = %q / %x / %v", + mediaType, + got, + err, + ) + } + if err := content.Close(); err != nil { + t.Fatalf("Close() error = %v", err) + } + if !body.closed { + t.Fatal("source body was not closed") + } + }) + } +} + +func TestProductImageContentRejectsUntrustedFallbacks(t *testing.T) { + tests := []struct { + name string + contentType string + body []byte + }{ + { + name: "explicit JSON overrides image bytes", + contentType: "application/json", + body: []byte{0xff, 0xd8, 0xff}, + }, + { + name: "missing JSON", + body: []byte(`{"status":false}`), + }, + { + name: "generic HTML", + contentType: "application/octet-stream", + body: []byte("