Gadfly on #23, blocking, 2/2 agreement — and it is the exact defect this whole line of work has been about: a call that succeeds while handing back the wrong bytes. sniffImageMIME falls back to image/png when detection is inconclusive, and the guard only consulted Content-Type. A response with NO Content-Type therefore skipped the check entirely and was labelled a PNG. The shim answers JSON on a semantic miss (no face found in the source or target), which is precisely the body that would have sailed through as a successful image. The check now validates the BYTES — http.DetectContentType must say image/ — and the reported MIME prefers the server's own label only when that label is itself an image type. Break-checked by restoring the header-only condition, which fails the new test. Also from that review: - index is documented as ignored under all=true, so a negative one is no longer rejected there; it is still rejected when it would actually be sent, and both halves are tested. - initImageFilename (video.go) was imageFilename with the base fixed to "frame" and now delegates to it — two copies of one extension table is how they drift. - DetectedFace carried Width/Height alongside Box, two sources of truth for one fact that can disagree after any transform. Now a Size() method derived from Box. - a dead `apiErr` in the test (declared, then `_ = apiErr`) was an abandoned errors.As check; it is wired up and now asserts callers can classify the error. - swapImg duplicated editInit verbatim; removed. Not taken: adding a FaceSwapProvider/ModelOption surface to match the other optional imagegen capabilities (single-model finding). There are no options to carry yet, and inventing an empty option type to look symmetrical would be API surface with nothing behind it. Worth revisiting when a real knob exists.
262 lines
9.2 KiB
Go
262 lines
9.2 KiB
Go
package llamaswap
|
|
|
|
import (
|
|
"context"
|
|
"encoding/base64"
|
|
"errors"
|
|
"io"
|
|
"mime"
|
|
"mime/multipart"
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"strings"
|
|
"testing"
|
|
|
|
"gitea.stevedudenhoeffer.com/steve/majordomo/imagegen"
|
|
"gitea.stevedudenhoeffer.com/steve/majordomo/llm"
|
|
)
|
|
|
|
// parseParts pulls the multipart form a handler received.
|
|
func parseParts(t *testing.T, r *http.Request) (files map[string][]byte, fields map[string]string) {
|
|
t.Helper()
|
|
_, params, err := mime.ParseMediaType(r.Header.Get("Content-Type"))
|
|
if err != nil {
|
|
t.Fatalf("content type: %v", err)
|
|
}
|
|
mr := multipart.NewReader(r.Body, params["boundary"])
|
|
files, fields = map[string][]byte{}, map[string]string{}
|
|
for {
|
|
p, err := mr.NextPart()
|
|
if err == io.EOF {
|
|
break
|
|
}
|
|
if err != nil {
|
|
t.Fatalf("next part: %v", err)
|
|
}
|
|
body, _ := io.ReadAll(p)
|
|
if p.FileName() != "" {
|
|
files[p.FormName()] = body
|
|
} else {
|
|
fields[p.FormName()] = string(body)
|
|
}
|
|
}
|
|
return files, fields
|
|
}
|
|
|
|
// TestFaceSwapSendsBothFiles pins the two-file wire shape. A face swap is the
|
|
// first endpoint in this provider taking more than one file, so buildMultipart
|
|
// grew a sibling; getting the field NAMES wrong would reach the shim as a
|
|
// missing-argument 422 rather than anything self-explanatory.
|
|
func TestFaceSwapSendsBothFiles(t *testing.T) {
|
|
var gotPath string
|
|
var files map[string][]byte
|
|
var fields map[string]string
|
|
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
|
gotPath = r.URL.Path
|
|
files, fields = parseParts(t, r)
|
|
w.Header().Set("Content-Type", "image/png")
|
|
raw, _ := base64.StdEncoding.DecodeString(onePixelPNG)
|
|
_, _ = w.Write(raw)
|
|
}))
|
|
defer srv.Close()
|
|
|
|
p := New(WithBaseURL(srv.URL), WithHTTPClient(srv.Client()))
|
|
m, err := p.FaceSwapModel("faceswap")
|
|
if err != nil {
|
|
t.Fatalf("model: %v", err)
|
|
}
|
|
img := editInit(t)
|
|
res, err := m.FaceSwap(context.Background(), imagegen.FaceSwapRequest{Target: img, Source: img},
|
|
imagegen.WithFaceIndex(2))
|
|
if err != nil {
|
|
t.Fatalf("faceswap: %v", err)
|
|
}
|
|
if len(res.Images) != 1 {
|
|
t.Fatalf("images = %d, want 1", len(res.Images))
|
|
}
|
|
if !strings.HasSuffix(gotPath, "/upstream/faceswap/v1/faceswap") {
|
|
t.Errorf("path = %q", gotPath)
|
|
}
|
|
for _, want := range []string{"target", "source"} {
|
|
if len(files[want]) == 0 {
|
|
t.Errorf("no %q file part — the shim requires both", want)
|
|
}
|
|
}
|
|
if fields["index"] != "2" {
|
|
t.Errorf("index = %q, want 2", fields["index"])
|
|
}
|
|
if _, ok := fields["all"]; ok {
|
|
t.Error("all sent alongside index — the shim ignores index under all=true, so sending both implies a precedence the caller cannot see")
|
|
}
|
|
}
|
|
|
|
// TestFaceSwapAllSuppressesIndex: same reasoning from the other side.
|
|
func TestFaceSwapAllSuppressesIndex(t *testing.T) {
|
|
var fields map[string]string
|
|
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
|
_, fields = parseParts(t, r)
|
|
w.Header().Set("Content-Type", "image/png")
|
|
raw, _ := base64.StdEncoding.DecodeString(onePixelPNG)
|
|
_, _ = w.Write(raw)
|
|
}))
|
|
defer srv.Close()
|
|
|
|
p := New(WithBaseURL(srv.URL), WithHTTPClient(srv.Client()))
|
|
m, _ := p.FaceSwapModel("faceswap")
|
|
img := editInit(t)
|
|
if _, err := m.FaceSwap(context.Background(),
|
|
imagegen.FaceSwapRequest{Target: img, Source: img, Index: new(int), All: true}); err != nil {
|
|
t.Fatalf("faceswap: %v", err)
|
|
}
|
|
if fields["all"] != "true" {
|
|
t.Errorf("all = %q, want true", fields["all"])
|
|
}
|
|
if _, ok := fields["index"]; ok {
|
|
t.Error("index sent under all=true")
|
|
}
|
|
}
|
|
|
|
// TestFaceSwapRejectsMissingImages: both are required, and the error should
|
|
// name which one rather than surfacing a shim 422.
|
|
func TestFaceSwapRejectsMissingImages(t *testing.T) {
|
|
p := New(WithBaseURL("http://example.invalid"))
|
|
m, _ := p.FaceSwapModel("faceswap")
|
|
img := editInit(t)
|
|
for _, tc := range []struct {
|
|
name string
|
|
req imagegen.FaceSwapRequest
|
|
want string
|
|
}{
|
|
{"no target", imagegen.FaceSwapRequest{Source: img}, "target"},
|
|
{"no source", imagegen.FaceSwapRequest{Target: img}, "source"},
|
|
} {
|
|
_, err := m.FaceSwap(context.Background(), tc.req)
|
|
if err == nil || !strings.Contains(err.Error(), tc.want) {
|
|
t.Errorf("%s: err = %v, want one naming %q", tc.name, err, tc.want)
|
|
}
|
|
}
|
|
}
|
|
|
|
// TestFaceSwapRejectsNonImageResponse: the shim answers JSON on a semantic
|
|
// miss (no face found). Returning those bytes as an "image" would hand the
|
|
// caller a file that is not a picture and call it success.
|
|
func TestFaceSwapRejectsNonImageResponse(t *testing.T) {
|
|
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
|
w.Header().Set("Content-Type", "application/json")
|
|
_, _ = w.Write([]byte(`{"detail":{"error":"no_face_in_source"}}`))
|
|
}))
|
|
defer srv.Close()
|
|
|
|
p := New(WithBaseURL(srv.URL), WithHTTPClient(srv.Client()))
|
|
m, _ := p.FaceSwapModel("faceswap")
|
|
img := editInit(t)
|
|
_, err := m.FaceSwap(context.Background(), imagegen.FaceSwapRequest{Target: img, Source: img})
|
|
if err == nil {
|
|
t.Fatal("a JSON body was accepted as an image")
|
|
}
|
|
var apiErr *llm.APIError
|
|
if !errors.As(err, &apiErr) {
|
|
t.Errorf("err = %T, want *llm.APIError so callers can classify it", err)
|
|
}
|
|
if !strings.Contains(err.Error(), "no_face_in_source") {
|
|
t.Errorf("err = %v, want it to relay the shim's reason", err)
|
|
}
|
|
}
|
|
|
|
// TestListFacesParsesOrdering: the shim's left-to-right index is the contract
|
|
// callers select against, so it must survive decoding intact.
|
|
func TestListFacesParsesOrdering(t *testing.T) {
|
|
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
|
if !strings.HasSuffix(r.URL.Path, "/upstream/faceswap/v1/faces") {
|
|
t.Errorf("path = %q", r.URL.Path)
|
|
}
|
|
w.Header().Set("Content-Type", "application/json")
|
|
_, _ = w.Write([]byte(`{"count":2,"faces":[
|
|
{"index":0,"box":[10,20,30,40],"score":0.9,"width":20,"height":20},
|
|
{"index":1,"box":[50,20,90,60],"score":0.8,"width":40,"height":40}]}`))
|
|
}))
|
|
defer srv.Close()
|
|
|
|
p := New(WithBaseURL(srv.URL), WithHTTPClient(srv.Client()))
|
|
m, _ := p.FaceSwapModel("faceswap")
|
|
faces, err := m.ListFaces(context.Background(), editInit(t))
|
|
if err != nil {
|
|
t.Fatalf("list: %v", err)
|
|
}
|
|
if len(faces) != 2 || faces[0].Index != 0 || faces[1].Index != 1 {
|
|
t.Fatalf("faces = %+v", faces)
|
|
}
|
|
if faces[1].Box != [4]int{50, 20, 90, 60} {
|
|
t.Errorf("box = %v", faces[1].Box)
|
|
}
|
|
}
|
|
|
|
// TestListFacesRejectsShortBox: a malformed box would send a caller at the
|
|
// wrong face, which is worse than an error.
|
|
func TestListFacesRejectsShortBox(t *testing.T) {
|
|
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
|
w.Header().Set("Content-Type", "application/json")
|
|
_, _ = w.Write([]byte(`{"count":1,"faces":[{"index":0,"box":[1,2],"score":0.9}]}`))
|
|
}))
|
|
defer srv.Close()
|
|
|
|
p := New(WithBaseURL(srv.URL), WithHTTPClient(srv.Client()))
|
|
m, _ := p.FaceSwapModel("faceswap")
|
|
if _, err := m.ListFaces(context.Background(), editInit(t)); err == nil {
|
|
t.Fatal("a 2-element box was accepted")
|
|
}
|
|
}
|
|
|
|
// TestFaceSwapRejectsHeaderlessNonImage is the regression for gadfly's
|
|
// blocking finding on #23, agreed by both models. sniffImageMIME falls back
|
|
// to image/png when detection is inconclusive, and the original guard only
|
|
// looked at Content-Type — so a JSON error body sent WITHOUT a Content-Type
|
|
// header was labelled a PNG and returned as a successful image. The shim
|
|
// answers JSON on a semantic miss, which is precisely the body that would
|
|
// have sailed through.
|
|
func TestFaceSwapRejectsHeaderlessNonImage(t *testing.T) {
|
|
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
|
// Explicitly no Content-Type — Go only sets one if we write before
|
|
// deleting it, so clear it to model a bare upstream response.
|
|
w.Header()["Content-Type"] = nil
|
|
_, _ = w.Write([]byte(`{"detail":{"error":"no_face_in_target"}}`))
|
|
}))
|
|
defer srv.Close()
|
|
|
|
p := New(WithBaseURL(srv.URL), WithHTTPClient(srv.Client()))
|
|
m, _ := p.FaceSwapModel("faceswap")
|
|
img := editInit(t)
|
|
_, err := m.FaceSwap(context.Background(), imagegen.FaceSwapRequest{Target: img, Source: img})
|
|
if err == nil {
|
|
t.Fatal("a headerless JSON body was accepted and would have been returned as image/png")
|
|
}
|
|
if !strings.Contains(err.Error(), "no_face_in_target") {
|
|
t.Errorf("err = %v, want it to relay what actually came back", err)
|
|
}
|
|
}
|
|
|
|
// TestFaceSwapAllowsNegativeIndexUnderAll: index is documented as ignored
|
|
// when all=true, so validating it there would reject a well-formed request.
|
|
func TestFaceSwapAllowsNegativeIndexUnderAll(t *testing.T) {
|
|
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
|
w.Header().Set("Content-Type", "image/png")
|
|
raw, _ := base64.StdEncoding.DecodeString(onePixelPNG)
|
|
_, _ = w.Write(raw)
|
|
}))
|
|
defer srv.Close()
|
|
|
|
p := New(WithBaseURL(srv.URL), WithHTTPClient(srv.Client()))
|
|
m, _ := p.FaceSwapModel("faceswap")
|
|
img := editInit(t)
|
|
neg := -1
|
|
if _, err := m.FaceSwap(context.Background(),
|
|
imagegen.FaceSwapRequest{Target: img, Source: img, Index: &neg, All: true}); err != nil {
|
|
t.Fatalf("negative index rejected under all=true, where it is ignored: %v", err)
|
|
}
|
|
// ...but still rejected when it WOULD be sent.
|
|
if _, err := m.FaceSwap(context.Background(),
|
|
imagegen.FaceSwapRequest{Target: img, Source: img, Index: &neg}); err == nil {
|
|
t.Error("negative index accepted when it would actually be sent")
|
|
}
|
|
}
|