From 0ab90a01cb26b9191e2ba4414d1a124d3b5ec832 Mon Sep 17 00:00:00 2001 From: Steve Dudenhoeffer Date: Wed, 22 Jul 2026 01:23:08 -0400 Subject: [PATCH 1/2] EXIF orientation: bake it in when normalizing images (#103) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit imagenorm decoded and re-encoded to JPEG but ignored the EXIF Orientation tag. Phone cameras store the sensor pixels one way and set an EXIF flag to rotate on display, so a "portrait" JPEG is really a landscape bitmap tagged "rotate 90°" — and our re-encode strips EXIF, so without baking the rotation in, a packet photographed in portrait reaches the vision model sideways (bad OCR) and any future thumbnail is wrong. - exifOrientation: a small pure-Go parser that walks the JPEG APP1/Exif segment for tag 0x0112, returning 1 (normal) for non-JPEG or unparseable input — never guess a rotation onto a correct image. No cgo, no new dep. - applyOrientation: bakes in all 8 orientations (the 4 rotations + mirrors) after downscale (cheaper to rotate the small image; a 90° turn swaps the sides but not the longest edge, so the downscale bound still holds). - Non-JPEG paths (HEIC/webp/png) are untouched — their decoders own orientation and they carry no JPEG EXIF. Tests build oriented JPEGs (a corner marker + a spliced Exif APP1) and assert the marker lands where each orientation says, dims swapping for the quarter-turns; plus parser defaults for non-JPEG / no-EXIF / garbage. Stays CGO_ENABLED=0. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01H3zbym8Doka2d7D48maSgZ --- DESIGN.md | 2 +- internal/imagenorm/imagenorm.go | 147 +++++++++++++++++++++++++-- internal/imagenorm/imagenorm_test.go | 118 +++++++++++++++++++++ 3 files changed, 256 insertions(+), 11 deletions(-) diff --git a/DESIGN.md b/DESIGN.md index 27e4209..d6d43e1 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -13,7 +13,7 @@ Work is tracked in Gitea issues; the tracking epic links every piece in dependen - **Users:** multi-user with ownership. Users own gardens; a garden can be shared with other users as viewer (read) or editor (edit content). Owner additionally shares/deletes. The first registered user is `is_admin` (set race-free inside the INSERT); admin gates instance-wide Settings (`requireAdmin`), the only thing that reads that flag. - **Auth:** OIDC-first (Authentik is the primary IdP), local argon2id passwords as an optional fallback. - **Instance settings (#79):** admin-editable, instance-wide config in a single-row `instance_settings` table — pansy's first DB-stored *instance* state (everything else hangs off a garden/object). Today it holds the agent model + on/off; **secrets never move here** — `OLLAMA_CLOUD_API_KEY` stays in the env so it doesn't land in backups or the undo history. Precedence: Settings value → env → default. The live agent Runner sits behind an `atomic.Pointer` in the API layer (`agentHolder`) with its routes always registered, so a settings change swaps it with no restart and no race against in-flight requests; `/capabilities` reads that pointer, so it reports what's live rather than what was configured at boot. The model/registry knowledge lives in one leaf package (`internal/agentmodel`) that both the runner and the settings validator import — agent imports service, so it can live in neither. -- **Seed-packet capture (#81):** photograph a packet → a *vision* model (separate `vision_model` setting) reads it into structured fields via one-shot `majordomo.Generate[SeedPacket]` — NOT an agent loop, so the extraction can't touch the garden; it only reads a picture and returns data. The image is normalized to JPEG at the upload boundary (`internal/imagenorm`: decodes HEIC/webp/png/jpeg, since majordomo's stdlib media path can't do HEIC — the iPhone default). The hard part is **catalog matching, not OCR**: a wrong auto-match splits a variety's seed-lot history across duplicate rows, so the service NEVER auto-creates — it surfaces ranked candidates (`matchPlants`) and the user confirms, then `CreateFromPacket` makes the plant (new or existing) + the lot. Plants/lots aren't in the undo history (they're catalog/inventory), so there's no change set to wrap. The extractor is injectable on the service (`WithPacketExtractor`) so the whole path tests hermetically against majordomo's `fake` provider. +- **Seed-packet capture (#81):** photograph a packet → a *vision* model (separate `vision_model` setting) reads it into structured fields via one-shot `majordomo.Generate[SeedPacket]` — NOT an agent loop, so the extraction can't touch the garden; it only reads a picture and returns data. The image is normalized to JPEG at the upload boundary (`internal/imagenorm`: decodes HEIC/webp/png/jpeg, since majordomo's stdlib media path can't do HEIC — the iPhone default; it also bakes in the JPEG EXIF orientation, since the re-encode strips EXIF and a phone photo tagged "rotate 90°" would otherwise reach the model sideways). The hard part is **catalog matching, not OCR**: a wrong auto-match splits a variety's seed-lot history across duplicate rows, so the service NEVER auto-creates — it surfaces ranked candidates (`matchPlants`) and the user confirms, then `CreateFromPacket` makes the plant (new or existing) + the lot. Plants/lots aren't in the undo history (they're catalog/inventory), so there's no change set to wrap. The extractor is injectable on the service (`WithPacketExtractor`) so the whole path tests hermetically against majordomo's `fake` provider. - **Agentic future:** integration with majordomo/executus via typed Go tools (`llm.DefineTool[Args]`) wrapping the same service layer the REST API uses — not MCP/OpenAPI. ## Domain model diff --git a/internal/imagenorm/imagenorm.go b/internal/imagenorm/imagenorm.go index 4461be4..701b377 100644 --- a/internal/imagenorm/imagenorm.go +++ b/internal/imagenorm/imagenorm.go @@ -28,6 +28,7 @@ package imagenorm import ( "bytes" + "encoding/binary" "errors" "fmt" "image" @@ -104,9 +105,10 @@ var ( ) // Normalize reads an image of any supported format (JPEG, PNG, HEIC, WebP), -// downscales it to fit opts.MaxDim on its longest edge, and returns it re-encoded -// as JPEG, plus the decoded format name (e.g. "heic") — handy for logging what a -// phone actually sent. On any error the returned bytes are nil and format is "". +// downscales it to fit opts.MaxDim on its longest edge, applies the JPEG EXIF +// orientation so the pixels come out upright, and returns it re-encoded as JPEG, +// plus the decoded format name (e.g. "heic") — handy for logging what a phone +// actually sent. On any error the returned bytes are nil and format is "". // // Errors, by cause: // - input over opts.MaxBytes, or a decoded canvas over maxDecodePixels / @@ -120,13 +122,18 @@ var ( // pre-decode pixel/dimension check, and a recover around the third-party decoders // (a malformed HEIC/WebP shouldn't take the process down). // -// Two known gaps, both deferred to the upload handler that wires this in (#81): -// - EXIF ORIENTATION is not applied, so a portrait phone photo tagged -// "rotate 90°" comes out sideways. That's best fixed and tested with a real -// oriented photo end-to-end, which the library has no consumer for yet. -// - There is no context: image.Decode is CPU-bound and not cancellable -// mid-decode, so a caller that needs a hard deadline should run Normalize -// under its own timeout. The size guards keep the work finite regardless. +// EXIF orientation: phone cameras store the sensor pixels in one orientation and +// set an EXIF tag to rotate on display, so a JPEG "portrait" photo is really a +// landscape bitmap tagged "rotate 90°" — and the re-encode below strips EXIF, +// which is exactly why the rotation must be BAKED IN here. applyOrientation does +// that for the JPEG path (the format phone uploads overwhelmingly arrive in); +// other formats carry no JPEG EXIF and their decoders own orientation, so they're +// left as decoded. +// +// One known gap, deferred to the upload handler (#81): there is no context — +// image.Decode is CPU-bound and not cancellable mid-decode, so a caller that +// needs a hard deadline should run Normalize under its own timeout. The size +// guards keep the work finite regardless. func Normalize(r io.Reader, opts Options) (out []byte, format string, err error) { // Cap the read at MaxBytes+1 so we can tell "exactly at the cap" from "over". // maxBytes() is always a sane positive (default 25 MiB); guard the +1 anyway. @@ -161,7 +168,11 @@ func Normalize(r io.Reader, opts Options) (out []byte, format string, err error) return nil, "", err } + // Downscale first (cheaper to rotate the small image), then bake in the EXIF + // orientation so the JPEG we emit is upright. A 90° rotation swaps the sides + // but not the longest edge, so the downscale bound still holds after it. img = downscale(img, opts.maxDim()) + img = applyOrientation(img, exifOrientation(raw)) var buf bytes.Buffer if err := jpeg.Encode(&buf, img, &jpeg.Options{Quality: jpegQuality}); err != nil { @@ -188,6 +199,122 @@ func decodeSafely(raw []byte) (img image.Image, format string, err error) { return img, format, nil } +// applyOrientation returns img with the EXIF orientation (1..8) baked in, so the +// pixels are upright and no display-time rotation is needed. Orientation 1 (and +// anything out of range) is a no-op. Values 5..8 are 90° rotations, which swap +// the output's width and height. Reads through image.Image.At and writes an RGBA, +// so it works whatever concrete type decode/downscale produced. +func applyOrientation(img image.Image, o int) image.Image { + if o <= 1 || o > 8 { + return img + } + b := img.Bounds() + w, h := b.Dx(), b.Dy() + swap := o >= 5 // a quarter-turn: output dimensions transpose + dst := image.NewRGBA(image.Rect(0, 0, w, h)) + if swap { + dst = image.NewRGBA(image.Rect(0, 0, h, w)) + } + for y := range h { + for x := range w { + c := img.At(b.Min.X+x, b.Min.Y+y) + var dx, dy int + switch o { + case 2: // mirror horizontal + dx, dy = w-1-x, y + case 3: // rotate 180 + dx, dy = w-1-x, h-1-y + case 4: // mirror vertical + dx, dy = x, h-1-y + case 5: // transpose (mirror across the main diagonal) + dx, dy = y, x + case 6: // rotate 90° clockwise + dx, dy = h-1-y, x + case 7: // transverse (mirror across the anti-diagonal) + dx, dy = h-1-y, w-1-x + case 8: // rotate 90° counter-clockwise + dx, dy = y, w-1-x + } + dst.Set(dx, dy, c) + } + } + return dst +} + +// exifOrientation extracts the EXIF Orientation tag (1..8) from raw image bytes, +// returning 1 (normal) when it's absent or unparseable — the safe default, since +// a wrong guess rotates a correct image. Only the JPEG APP1/Exif path is parsed: +// that's the format uploaded phone photos overwhelmingly arrive in, and the other +// decoders own their own orientation. +func exifOrientation(raw []byte) int { + // A JPEG is a run of FFxx marker segments after the SOI (FFD8). Walk them + // looking for APP1 (FFE1) carrying "Exif\0\0"; stop at the scan data (SOS). + if len(raw) < 4 || raw[0] != 0xFF || raw[1] != 0xD8 { + return 1 + } + for i := 2; i+4 <= len(raw); { + if raw[i] != 0xFF { + return 1 // not aligned on a marker; give up rather than misread + } + marker := raw[i+1] + if marker == 0xD9 || marker == 0xDA { + return 1 // EOI / start-of-scan: no more headers to read + } + segLen := int(raw[i+2])<<8 | int(raw[i+3]) + if segLen < 2 || i+2+segLen > len(raw) { + return 1 + } + if marker == 0xE1 { + if o, ok := orientationFromApp1(raw[i+4 : i+2+segLen]); ok { + return o + } + } + i += 2 + segLen + } + return 1 +} + +// orientationFromApp1 reads the Orientation tag from a JPEG APP1 segment body +// (everything after the 2-byte length): "Exif\0\0" then a TIFF block holding +// IFD0. Returns (0, false) if the segment isn't Exif or the tag is missing. +func orientationFromApp1(seg []byte) (int, bool) { + const prefix = "Exif\x00\x00" + if len(seg) < len(prefix)+8 || string(seg[:len(prefix)]) != prefix { + return 0, false + } + tiff := seg[len(prefix):] + var bo binary.ByteOrder + switch string(tiff[0:2]) { + case "II": + bo = binary.LittleEndian + case "MM": + bo = binary.BigEndian + default: + return 0, false + } + ifd := int(bo.Uint32(tiff[4:8])) // offset to IFD0 from the TIFF start + if ifd < 8 || ifd+2 > len(tiff) { + return 0, false + } + n := int(bo.Uint16(tiff[ifd : ifd+2])) + for k := range n { + off := ifd + 2 + k*12 // each IFD entry is 12 bytes + if off+12 > len(tiff) { + return 0, false + } + if bo.Uint16(tiff[off:off+2]) != 0x0112 { // Orientation tag + continue + } + // SHORT value lives in the first 2 bytes of the entry's value field. + v := int(bo.Uint16(tiff[off+8 : off+10])) + if v >= 1 && v <= 8 { + return v, true + } + return 0, false + } + return 0, false +} + // downscale returns img shrunk so its longest edge is at most maxDim, preserving // aspect ratio. An image already within bounds is returned unchanged (no // re-sampling, no quality loss beyond the JPEG round-trip). Uses Catmull-Rom for diff --git a/internal/imagenorm/imagenorm_test.go b/internal/imagenorm/imagenorm_test.go index 4a67309..acf440e 100644 --- a/internal/imagenorm/imagenorm_test.go +++ b/internal/imagenorm/imagenorm_test.go @@ -5,6 +5,7 @@ import ( "encoding/binary" "hash/crc32" "image" + "image/color" "image/jpeg" "image/png" "os" @@ -200,3 +201,120 @@ func TestNormalizeRejectsPixelBomb(t *testing.T) { }) } } + +// orientedJPEG builds a JPEG whose top-left quadrant is white and the rest black +// — a marker to track through a rotation — tagged with the given EXIF orientation +// (1..8). The marker lets a test assert the pixels actually moved to where that +// orientation says they should. +func orientedJPEG(t *testing.T, orient int) []byte { + t.Helper() + const w, h = 40, 24 + m := image.NewRGBA(image.Rect(0, 0, w, h)) + for y := range h { + for x := range w { + c := color.RGBA{0, 0, 0, 255} + if x < w/2 && y < h/2 { + c = color.RGBA{255, 255, 255, 255} + } + m.Set(x, y, c) + } + } + var jb bytes.Buffer + if err := jpeg.Encode(&jb, m, &jpeg.Options{Quality: 95}); err != nil { + t.Fatalf("encode jpeg: %v", err) + } + if orient == 0 { + return jb.Bytes() // caller wants a plain JPEG with no EXIF + } + return spliceExifOrientation(t, jb.Bytes(), orient) +} + +// spliceExifOrientation inserts a minimal little-endian Exif APP1 segment +// carrying just the Orientation tag right after the JPEG SOI marker. +func spliceExifOrientation(t *testing.T, jpg []byte, orient int) []byte { + t.Helper() + var tiff bytes.Buffer + tiff.WriteString("II") // little-endian + _ = binary.Write(&tiff, binary.LittleEndian, uint16(0x2A)) + _ = binary.Write(&tiff, binary.LittleEndian, uint32(8)) // IFD0 offset + _ = binary.Write(&tiff, binary.LittleEndian, uint16(1)) // one entry + _ = binary.Write(&tiff, binary.LittleEndian, uint16(0x0112)) + _ = binary.Write(&tiff, binary.LittleEndian, uint16(3)) // SHORT + _ = binary.Write(&tiff, binary.LittleEndian, uint32(1)) // count + _ = binary.Write(&tiff, binary.LittleEndian, uint16(orient)) // value + _ = binary.Write(&tiff, binary.LittleEndian, uint16(0)) // value pad + _ = binary.Write(&tiff, binary.LittleEndian, uint32(0)) // next IFD + payload := append([]byte("Exif\x00\x00"), tiff.Bytes()...) + + segLen := len(payload) + 2 + seg := []byte{0xFF, 0xE1, byte(segLen >> 8), byte(segLen)} + seg = append(seg, payload...) + + out := make([]byte, 0, len(jpg)+len(seg)) + out = append(out, jpg[:2]...) // SOI + out = append(out, seg...) + return append(out, jpg[2:]...) +} + +func bright(c color.Color) bool { + r, g, b, _ := c.RGBA() // 16-bit + return (r+g+b)/3 > 0x8000 +} + +// TestNormalizeAppliesExifOrientation is the #103 regression: a phone photo tagged +// "rotate 90°" must come out of Normalize with the pixels upright, not sideways — +// the re-encode strips EXIF, so the rotation has to be baked into the bitmap. +func TestNormalizeAppliesExifOrientation(t *testing.T) { + // The white marker starts centred at (10,6) in the 40x24 source. For each + // orientation, wantW/H is the corrected canvas and (mx,my) is where that + // marker must land — derived from the same transform Normalize applies. + cases := []struct { + orient int + wantW, wantH int + mx, my int + }{ + {0, 40, 24, 10, 6}, // no EXIF → unchanged, marker top-left + {1, 40, 24, 10, 6}, // normal → unchanged + {3, 40, 24, 29, 17}, // 180 → bottom-right + {6, 24, 40, 17, 10}, // 90° CW → top-right (dims swap) + {8, 24, 40, 6, 29}, // 90° CCW → bottom-left (dims swap) + } + for _, tc := range cases { + out, format, err := Normalize(bytes.NewReader(orientedJPEG(t, tc.orient)), Options{}) + if err != nil { + t.Fatalf("orient %d: Normalize err = %v", tc.orient, err) + } + if format != "jpeg" { + t.Errorf("orient %d: format = %q, want jpeg", tc.orient, format) + } + m, _, err := image.Decode(bytes.NewReader(out)) + if err != nil { + t.Fatalf("orient %d: decode output: %v", tc.orient, err) + } + if m.Bounds().Dx() != tc.wantW || m.Bounds().Dy() != tc.wantH { + t.Errorf("orient %d: output %dx%d, want %dx%d", + tc.orient, m.Bounds().Dx(), m.Bounds().Dy(), tc.wantW, tc.wantH) + } + if !bright(m.At(m.Bounds().Min.X+tc.mx, m.Bounds().Min.Y+tc.my)) { + t.Errorf("orient %d: white marker not at (%d,%d) — orientation not applied", + tc.orient, tc.mx, tc.my) + } + } +} + +// TestExifOrientationParsing pins the parser against non-JPEG and no-EXIF inputs, +// which must default to 1 (never guess a rotation onto a correct image). +func TestExifOrientationParsing(t *testing.T) { + if o := exifOrientation(pngBytes(t, 8, 8)); o != 1 { + t.Errorf("PNG orientation = %d, want 1 (no JPEG EXIF path)", o) + } + if o := exifOrientation(orientedJPEG(t, 0)); o != 1 { + t.Errorf("JPEG without EXIF orientation = %d, want 1", o) + } + if o := exifOrientation(orientedJPEG(t, 6)); o != 6 { + t.Errorf("JPEG tagged 6 → %d, want 6", o) + } + if o := exifOrientation([]byte("not an image")); o != 1 { + t.Errorf("garbage bytes → %d, want 1", o) + } +} -- 2.54.0 From 8aad2278a0a93c18da7da36ee6638d34f7f044ba Mon Sep 17 00:00:00 2001 From: Steve Dudenhoeffer Date: Wed, 22 Jul 2026 01:35:19 -0400 Subject: [PATCH 2/2] Address EXIF review: single alloc, no per-pixel boxing, hardened parser MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Gadfly on #103: - (4 models) applyOrientation allocated a WxH RGBA then threw it away for a quarter-turn to allocate HxW. Compute the output dims once, allocate one buffer. - (perf) The At/Set loop boxed a color.Color per pixel — millions of heap allocs on a full-res rotation. Convert to *image.RGBA once (draw.Draw) then copy 4 bytes per pixel by offset. No boxing. - (2 findings) exifOrientation didn't skip 0xFF fill bytes before a marker, so a spec-valid padded APP1 would be misread. Skip them. - (2 findings) orientationFromApp1 read the tag's inline value without checking its type/count — a mistyped LONG/offset would be read as a bogus SHORT. Require type=SHORT, count=1; also validate the TIFF 0x2A magic agrees with the byte order. - Tests now cover all 8 orientations (added the mirror/transpose cases 2/4/5/7, the most error-prone switch arms) as t.Run subtests. All imagenorm tests green; gofmt clean; still CGO_ENABLED=0. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01H3zbym8Doka2d7D48maSgZ --- internal/imagenorm/imagenorm.go | 57 ++++++++++++++++++++++------ internal/imagenorm/imagenorm_test.go | 54 ++++++++++++++------------ 2 files changed, 75 insertions(+), 36 deletions(-) diff --git a/internal/imagenorm/imagenorm.go b/internal/imagenorm/imagenorm.go index 701b377..c19f667 100644 --- a/internal/imagenorm/imagenorm.go +++ b/internal/imagenorm/imagenorm.go @@ -202,22 +202,34 @@ func decodeSafely(raw []byte) (img image.Image, format string, err error) { // applyOrientation returns img with the EXIF orientation (1..8) baked in, so the // pixels are upright and no display-time rotation is needed. Orientation 1 (and // anything out of range) is a no-op. Values 5..8 are 90° rotations, which swap -// the output's width and height. Reads through image.Image.At and writes an RGBA, -// so it works whatever concrete type decode/downscale produced. +// the output's width and height. Copies raw RGBA pixels by byte offset (after a +// one-time conversion if the source isn't already RGBA), so a full-resolution +// rotation doesn't box a color.Color per pixel. func applyOrientation(img image.Image, o int) image.Image { if o <= 1 || o > 8 { return img } - b := img.Bounds() - w, h := b.Dx(), b.Dy() - swap := o >= 5 // a quarter-turn: output dimensions transpose - dst := image.NewRGBA(image.Rect(0, 0, w, h)) - if swap { - dst = image.NewRGBA(image.Rect(0, 0, h, w)) + // Work on a concrete RGBA so the transform is a 4-byte copy per pixel rather + // than millions of boxed color.Color values through At/Set. downscale usually + // hands us an *image.RGBA already; convert once if not (e.g. a small JPEG that + // skipped downscale decodes to YCbCr). + src, ok := img.(*image.RGBA) + if !ok { + b := img.Bounds() + conv := image.NewRGBA(image.Rect(0, 0, b.Dx(), b.Dy())) + draw.Draw(conv, conv.Bounds(), img, b.Min, draw.Src) + src = conv } + sb := src.Bounds() + w, h := sb.Dx(), sb.Dy() + // A quarter-turn (5..8) transposes the output; size the one buffer accordingly. + dw, dh := w, h + if o >= 5 { + dw, dh = h, w + } + dst := image.NewRGBA(image.Rect(0, 0, dw, dh)) for y := range h { for x := range w { - c := img.At(b.Min.X+x, b.Min.Y+y) var dx, dy int switch o { case 2: // mirror horizontal @@ -235,7 +247,9 @@ func applyOrientation(img image.Image, o int) image.Image { case 8: // rotate 90° counter-clockwise dx, dy = y, w-1-x } - dst.Set(dx, dy, c) + si := src.PixOffset(sb.Min.X+x, sb.Min.Y+y) + di := dst.PixOffset(dx, dy) + copy(dst.Pix[di:di+4], src.Pix[si:si+4]) } } return dst @@ -252,14 +266,25 @@ func exifOrientation(raw []byte) int { if len(raw) < 4 || raw[0] != 0xFF || raw[1] != 0xD8 { return 1 } - for i := 2; i+4 <= len(raw); { + for i := 2; i+1 < len(raw); { if raw[i] != 0xFF { return 1 // not aligned on a marker; give up rather than misread } + // A marker may be preceded by any number of 0xFF fill bytes (JPEG spec); + // skip them so a padded APP1 isn't misread as a marker of value 0xFF. + for i+1 < len(raw) && raw[i+1] == 0xFF { + i++ + } + if i+1 >= len(raw) { + return 1 + } marker := raw[i+1] if marker == 0xD9 || marker == 0xDA { return 1 // EOI / start-of-scan: no more headers to read } + if i+4 > len(raw) { + return 1 + } segLen := int(raw[i+2])<<8 | int(raw[i+3]) if segLen < 2 || i+2+segLen > len(raw) { return 1 @@ -292,6 +317,9 @@ func orientationFromApp1(seg []byte) (int, bool) { default: return 0, false } + if bo.Uint16(tiff[2:4]) != 0x2A { // TIFF magic (42); byte order must agree + return 0, false + } ifd := int(bo.Uint32(tiff[4:8])) // offset to IFD0 from the TIFF start if ifd < 8 || ifd+2 > len(tiff) { return 0, false @@ -305,7 +333,12 @@ func orientationFromApp1(seg []byte) (int, bool) { if bo.Uint16(tiff[off:off+2]) != 0x0112 { // Orientation tag continue } - // SHORT value lives in the first 2 bytes of the entry's value field. + // Orientation is defined as a single SHORT, whose value sits inline in the + // first 2 bytes of the value field. Reject anything else rather than read a + // mistyped entry (a LONG/offset there would be a different number entirely). + if bo.Uint16(tiff[off+2:off+4]) != 3 || bo.Uint32(tiff[off+4:off+8]) != 1 { + return 0, false + } v := int(bo.Uint16(tiff[off+8 : off+10])) if v >= 1 && v <= 8 { return v, true diff --git a/internal/imagenorm/imagenorm_test.go b/internal/imagenorm/imagenorm_test.go index acf440e..3b804bb 100644 --- a/internal/imagenorm/imagenorm_test.go +++ b/internal/imagenorm/imagenorm_test.go @@ -269,36 +269,42 @@ func TestNormalizeAppliesExifOrientation(t *testing.T) { // orientation, wantW/H is the corrected canvas and (mx,my) is where that // marker must land — derived from the same transform Normalize applies. cases := []struct { + name string orient int wantW, wantH int mx, my int }{ - {0, 40, 24, 10, 6}, // no EXIF → unchanged, marker top-left - {1, 40, 24, 10, 6}, // normal → unchanged - {3, 40, 24, 29, 17}, // 180 → bottom-right - {6, 24, 40, 17, 10}, // 90° CW → top-right (dims swap) - {8, 24, 40, 6, 29}, // 90° CCW → bottom-left (dims swap) + {"none", 0, 40, 24, 10, 6}, // no EXIF → unchanged, marker top-left + {"normal", 1, 40, 24, 10, 6}, // normal → unchanged + {"mirror-h", 2, 40, 24, 29, 6}, // flip horizontal → top-right + {"rotate-180", 3, 40, 24, 29, 17}, // → bottom-right + {"mirror-v", 4, 40, 24, 10, 17}, // flip vertical → bottom-left + {"transpose", 5, 24, 40, 6, 10}, // main diagonal (dims swap) + {"rotate-90-cw", 6, 24, 40, 17, 10}, // → top-right (dims swap) + {"transverse", 7, 24, 40, 17, 29}, // anti-diagonal (dims swap) + {"rotate-90-ccw", 8, 24, 40, 6, 29}, // → bottom-left (dims swap) } for _, tc := range cases { - out, format, err := Normalize(bytes.NewReader(orientedJPEG(t, tc.orient)), Options{}) - if err != nil { - t.Fatalf("orient %d: Normalize err = %v", tc.orient, err) - } - if format != "jpeg" { - t.Errorf("orient %d: format = %q, want jpeg", tc.orient, format) - } - m, _, err := image.Decode(bytes.NewReader(out)) - if err != nil { - t.Fatalf("orient %d: decode output: %v", tc.orient, err) - } - if m.Bounds().Dx() != tc.wantW || m.Bounds().Dy() != tc.wantH { - t.Errorf("orient %d: output %dx%d, want %dx%d", - tc.orient, m.Bounds().Dx(), m.Bounds().Dy(), tc.wantW, tc.wantH) - } - if !bright(m.At(m.Bounds().Min.X+tc.mx, m.Bounds().Min.Y+tc.my)) { - t.Errorf("orient %d: white marker not at (%d,%d) — orientation not applied", - tc.orient, tc.mx, tc.my) - } + t.Run(tc.name, func(t *testing.T) { + out, format, err := Normalize(bytes.NewReader(orientedJPEG(t, tc.orient)), Options{}) + if err != nil { + t.Fatalf("Normalize err = %v", err) + } + if format != "jpeg" { + t.Errorf("format = %q, want jpeg", format) + } + m, _, err := image.Decode(bytes.NewReader(out)) + if err != nil { + t.Fatalf("decode output: %v", err) + } + if m.Bounds().Dx() != tc.wantW || m.Bounds().Dy() != tc.wantH { + t.Errorf("output %dx%d, want %dx%d", + m.Bounds().Dx(), m.Bounds().Dy(), tc.wantW, tc.wantH) + } + if !bright(m.At(m.Bounds().Min.X+tc.mx, m.Bounds().Min.Y+tc.my)) { + t.Errorf("white marker not at (%d,%d) — orientation not applied", tc.mx, tc.my) + } + }) } } -- 2.54.0