mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-08-09 16:14:40 +00:00
fix(commons): two crash bugs in ImageReencoder/CompressionException
Reported via runtime crash dialog on the user's first PNG upload:
Exception in thread "AWT-EventQueue-0":
java.lang.IllegalStateException: Can't overwrite cause with
javax.imageio.IIOException: Bogus input colorspace
at java.lang.Throwable.initCause(Throwable.java:464)
at CompressionException.<init>(CompressionException.kt:39)
Two real bugs:
1. CompressionException constructor double-set the cause.
Exception(message, cause) super already wires the Throwable's
cause slot; the init block then called initCause(cause) AGAIN
which throws IllegalStateException by spec ("Can't overwrite
cause"). The init block was added per a code-review note that
was wrong about how Kotlin's primary constructor forwards
cause. Removed the init block; relying on super does the
right thing. Regression: encodeFailedWrapsCauseWithoutCrashing.
2. JPEG writer rejected non-RGB BufferedImages with "Bogus input
colorspace". TYPE_INT_ARGB (typical PNG decode), TYPE_BYTE_GRAY,
TYPE_CUSTOM (CMYK JPEGs, indexed PNGs) all blow up the stock
JPEGImageWriter. encodeJpeg now flattens via toRgbCanvas — draws
onto a fresh TYPE_INT_RGB canvas with white background for any
transparent pixels. White matches what every major image viewer
does for transparent PNGs over a light surface.
Regression: reencodesPngWithAlphaToJpeg.
Both regressions are covered by new tests so the patterns cannot
silently come back. Reencoder test count: 13 -> 15.
This commit is contained in:
+4
-8
@@ -26,19 +26,15 @@ package com.vitorpamplona.amethyst.commons.service.upload
|
||||
* "Send Original" decision (e.g., InputTooLarge → user can still
|
||||
* bypass; UnsupportedFormat → bypass uploads raw bytes).
|
||||
*
|
||||
* Each subclass calls `initCause` (via the secondary constructor) so
|
||||
* the cause chain survives logging — Kotlin's primary constructor
|
||||
* does not auto-wire `cause` to `Throwable.cause` when the parent
|
||||
* constructor receives both message and cause.
|
||||
* `Exception(message, cause)` already wires `cause` into the
|
||||
* `Throwable.cause` slot — DO NOT call `initCause` again, doing so
|
||||
* throws `IllegalStateException("Can't overwrite cause …")` at
|
||||
* construction time.
|
||||
*/
|
||||
sealed class CompressionException(
|
||||
message: String,
|
||||
cause: Throwable? = null,
|
||||
) : Exception(message, cause) {
|
||||
init {
|
||||
if (cause != null) initCause(cause)
|
||||
}
|
||||
|
||||
/** Source format has no usable decoder in v1 (AVIF, HEIC). */
|
||||
class UnsupportedFormat(
|
||||
val format: String,
|
||||
|
||||
+28
-5
@@ -185,16 +185,20 @@ object ImageReencoder {
|
||||
|
||||
/**
|
||||
* Encode `image` as JPEG into `out` at the given `quality` factor.
|
||||
* When the source has a non-sRGB ICC profile, the JPEG writer's
|
||||
* default behavior is to embed the profile as APP2 markers so the
|
||||
* uploaded file retains color fidelity on Display P3 / Adobe RGB
|
||||
* inputs.
|
||||
*
|
||||
* JPEG can only write 3-channel RGB. Inputs decoded as
|
||||
* TYPE_INT_ARGB (most PNGs), TYPE_BYTE_GRAY, TYPE_CUSTOM (CMYK
|
||||
* JPEGs, indexed PNGs, exotic colorspaces) all blow up the stock
|
||||
* `JPEGImageWriter` with `Bogus input colorspace`. Force a
|
||||
* canonical `TYPE_INT_RGB` BufferedImage via [toRgbCanvas] before
|
||||
* handing off to the writer.
|
||||
*/
|
||||
private fun encodeJpeg(
|
||||
image: BufferedImage,
|
||||
out: File,
|
||||
quality: Float,
|
||||
) {
|
||||
val rgb = if (image.type == BufferedImage.TYPE_INT_RGB) image else toRgbCanvas(image)
|
||||
val writer =
|
||||
ImageIO.getImageWritersByMIMEType("image/jpeg").let {
|
||||
if (!it.hasNext()) throw EncodeFailed(IllegalStateException("no JPEG writer registered"))
|
||||
@@ -209,13 +213,32 @@ object ImageReencoder {
|
||||
compressionType = "JPEG"
|
||||
compressionQuality = quality
|
||||
}
|
||||
writer.write(null, IIOImage(image, null, null), param)
|
||||
writer.write(null, IIOImage(rgb, null, null), param)
|
||||
}
|
||||
} finally {
|
||||
writer.dispose()
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Draw [src] onto a fresh `TYPE_INT_RGB` canvas. Transparent
|
||||
* pixels render against a white background — JPEG has no alpha,
|
||||
* and "default to white" matches what every major image viewer
|
||||
* does when displaying transparent PNGs over a light surface.
|
||||
*/
|
||||
private fun toRgbCanvas(src: BufferedImage): BufferedImage {
|
||||
val rgb = BufferedImage(src.width, src.height, BufferedImage.TYPE_INT_RGB)
|
||||
val g = rgb.createGraphics()
|
||||
try {
|
||||
g.color = java.awt.Color.WHITE
|
||||
g.fillRect(0, 0, src.width, src.height)
|
||||
g.drawImage(src, 0, 0, null)
|
||||
} finally {
|
||||
g.dispose()
|
||||
}
|
||||
return rgb
|
||||
}
|
||||
|
||||
/**
|
||||
* Pass-through reason captured for telemetry and diagnostics. Not
|
||||
* surfaced to users — the orchestrator decides on user-facing
|
||||
|
||||
+58
@@ -197,6 +197,35 @@ class ImageReencoderTest {
|
||||
assertEquals(0xD8.toByte(), bytes[1])
|
||||
}
|
||||
|
||||
@Test
|
||||
fun reencodesPngWithAlphaToJpeg() =
|
||||
runTest {
|
||||
// Regression: TYPE_INT_ARGB inputs (typical PNG decode)
|
||||
// used to crash the JPEG writer with "Bogus input
|
||||
// colorspace". encodeJpeg now flattens onto a white RGB
|
||||
// canvas before handing off to the writer.
|
||||
val src = makePngWithAlpha(800, 600)
|
||||
val result = ImageReencoder.reencode(src, CompressionQuality.MEDIUM)
|
||||
val reencoded = assertIs<ReencodeResult.Reencoded>(result)
|
||||
track(reencoded.file)
|
||||
|
||||
val decoded = ImageIO.read(reencoded.file)
|
||||
assertTrue(decoded != null, "output must be a valid JPEG")
|
||||
assertEquals(640, decoded.width, "Medium clamps long edge to 640")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun encodeFailedWrapsCauseWithoutCrashing() {
|
||||
// Regression: CompressionException had a double-set cause
|
||||
// (super(message, cause) + initCause(cause)) that threw
|
||||
// IllegalStateException at construction time, masking the
|
||||
// real encode error.
|
||||
val original = javax.imageio.IIOException("Bogus input colorspace")
|
||||
val wrapped = CompressionException.EncodeFailed(original)
|
||||
assertEquals(original, wrapped.cause)
|
||||
assertTrue(wrapped.message!!.contains("Bogus input colorspace"))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun outputLivesInAmethystTmpDir() =
|
||||
runTest {
|
||||
@@ -222,6 +251,35 @@ class ImageReencoderTest {
|
||||
createdFiles += file
|
||||
}
|
||||
|
||||
private fun makePngWithAlpha(
|
||||
width: Int,
|
||||
height: Int,
|
||||
): File {
|
||||
val img = BufferedImage(width, height, BufferedImage.TYPE_INT_ARGB)
|
||||
val g = img.createGraphics()
|
||||
try {
|
||||
// Transparent left half, solid colored right half — the
|
||||
// "Bogus input colorspace" trap fires regardless of
|
||||
// pixel data, just on the color model.
|
||||
for (y in 0 until height step 8) {
|
||||
for (x in 0 until width step 8) {
|
||||
val alpha = if (x < width / 2) 0 else 255
|
||||
val r = (x * 255 / width).coerceIn(0, 255)
|
||||
val gg = (y * 255 / height).coerceIn(0, 255)
|
||||
val b = ((x + y) * 255 / (width + height)).coerceIn(0, 255)
|
||||
g.color = Color(r, gg, b, alpha)
|
||||
g.fillRect(x, y, 8, 8)
|
||||
}
|
||||
}
|
||||
} finally {
|
||||
g.dispose()
|
||||
}
|
||||
val out = File.createTempFile("reencoder_alpha_", ".png")
|
||||
track(out)
|
||||
ImageIO.write(img, "png", out)
|
||||
return out
|
||||
}
|
||||
|
||||
private fun makeJpeg(
|
||||
width: Int,
|
||||
height: Int,
|
||||
|
||||
Reference in New Issue
Block a user