Skip to content

Commit 4238c0e

Browse files
committed
fix(app): address adversarial review of camera decode
1 parent 326cfc6 commit 4238c0e

1 file changed

Lines changed: 73 additions & 27 deletions

File tree

app/src/main/java/io/theficos/ereader/ui/scan/ScanScreen.kt

Lines changed: 73 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -299,20 +299,21 @@ private fun CameraPreview(
299299
-> Unit
300300
}
301301
}
302-
val reader = remember {
303-
MultiFormatReader().apply {
304-
setHints(
305-
mapOf(
306-
DecodeHintType.POSSIBLE_FORMATS to listOf(
307-
BarcodeFormat.EAN_13,
308-
BarcodeFormat.EAN_8,
309-
BarcodeFormat.UPC_A,
310-
),
311-
DecodeHintType.TRY_HARDER to true,
312-
),
313-
)
314-
}
302+
// Hints are passed to reader.decode(bitmap, hints) on every frame rather
303+
// than stashed once via setHints(): the single-arg MultiFormatReader.decode
304+
// calls setHints(null) internally and would silently discard POSSIBLE_FORMATS
305+
// and TRY_HARDER. Passing them explicitly keeps them in force every decode.
306+
val decodeHints = remember {
307+
mapOf<DecodeHintType, Any>(
308+
DecodeHintType.POSSIBLE_FORMATS to listOf(
309+
BarcodeFormat.EAN_13,
310+
BarcodeFormat.EAN_8,
311+
BarcodeFormat.UPC_A,
312+
),
313+
DecodeHintType.TRY_HARDER to true,
314+
)
315315
}
316+
val reader = remember { MultiFormatReader() }
316317

317318
DisposableEffect(Unit) {
318319
onDispose { analysisExecutor.shutdown() }
@@ -334,14 +335,19 @@ private fun CameraPreview(
334335
.also { ia ->
335336
ia.setAnalyzer(analysisExecutor) { proxy ->
336337
try {
337-
decodeBarcode(proxy, reader)?.let { isbn ->
338+
decodeBarcode(proxy, reader, decodeHints)?.let { isbn ->
338339
if (decoded.compareAndSet(false, true)) {
339340
// Hop to the main thread: onIsbnSubmitted
340341
// touches Compose/VM state and must not be
341342
// called from the analysis worker thread.
342343
mainExecutor.execute { currentOnDecoded(isbn) }
343344
}
344345
}
346+
} catch (_: Throwable) {
347+
// Never let a decode-path failure escape into
348+
// CameraX: it would be thrown once per frame on the
349+
// analysis thread (log spam, wasted CPU) and mask
350+
// genuine read failures. A no-read is just a null.
345351
} finally {
346352
// Always close so STRATEGY_KEEP_ONLY_LATEST keeps
347353
// delivering frames; an un-closed proxy stalls the
@@ -385,11 +391,18 @@ private fun CameraPreview(
385391
* landscape, so [androidx.camera.core.ImageInfo.rotationDegrees] is typically
386392
* 90/270 and a real-world-horizontal EAN-13 lands rotated in the buffer.
387393
* ZXing's EAN-13 reader is a 1D *row* scanner and a single decode() does not
388-
* try rotated variants, so a 90°-rotated barcode is a permanent no-read. We
389-
* decode the upright source and, on a miss, its 90° CCW variant — covering
390-
* both portrait orientations regardless of the actual rotation metadata.
394+
* try rotated variants, so a 90°-rotated barcode is a permanent no-read.
395+
* [PlanarYUVLuminanceSource] does NOT support rotateCounterClockwise() in
396+
* zxing 3.5.x (the base throws UnsupportedOperationException), so we rotate
397+
* the packed Y buffer ourselves: we decode the frame as delivered and, on a
398+
* miss, a 90°-transposed variant — covering an EAN-13 held horizontally in
399+
* either portrait rotation regardless of the rotation metadata.
391400
*/
392-
private fun decodeBarcode(proxy: ImageProxy, reader: MultiFormatReader): String? {
401+
private fun decodeBarcode(
402+
proxy: ImageProxy,
403+
reader: MultiFormatReader,
404+
hints: Map<DecodeHintType, Any>,
405+
): String? {
393406
val plane = proxy.planes.firstOrNull() ?: return null
394407
val width = proxy.width
395408
val height = proxy.height
@@ -399,12 +412,36 @@ private fun decodeBarcode(proxy: ImageProxy, reader: MultiFormatReader): String?
399412
// dataWidth == width because we repacked the plane tightly above.
400413
val source = PlanarYUVLuminanceSource(yData, width, height, 0, 0, width, height, false)
401414

402-
// Try the frame as delivered, then rotated 90° CCW. rotateCounterClockwise()
403-
// on PlanarYUVLuminanceSource is supported in zxing 3.5.3 and is cheap
404-
// (index remap, no extra decode of the whole image). The two orientations
405-
// cover an EAN-13 held horizontally in either portrait rotation.
406-
return decodeOrNull(source, reader)
407-
?: decodeOrNull(source.rotateCounterClockwise(), reader)
415+
// Try the frame as delivered first.
416+
decodeOrNull(source, reader, hints)?.let { return it }
417+
418+
// On a miss, try the 90°-transposed buffer. We rotate the bytes ourselves
419+
// because PlanarYUVLuminanceSource.rotateCounterClockwise() throws in zxing
420+
// 3.5.x. A single transpose makes a real-world-horizontal barcode that
421+
// landed vertical (or vice-versa) line up with ZXing's 1D row scanner; one
422+
// orientation suffices since the row scanner reads a full line either way.
423+
val rotated = rotateYPlane90(yData, width, height)
424+
val rotatedSource =
425+
PlanarYUVLuminanceSource(rotated, height, width, 0, 0, height, width, false)
426+
return decodeOrNull(rotatedSource, reader, hints)
427+
}
428+
429+
/**
430+
* Transpose a tightly-packed width×height luma buffer by 90° into a new
431+
* height×width buffer. dst[x, y] = src[y, x]; this maps a horizontal feature in
432+
* the source to a vertical one in the result, which is exactly what we need to
433+
* present a sideways barcode upright to ZXing's 1D row scanner.
434+
*/
435+
private fun rotateYPlane90(src: ByteArray, width: Int, height: Int): ByteArray {
436+
val dst = ByteArray(width * height)
437+
for (y in 0 until height) {
438+
val rowBase = y * width
439+
for (x in 0 until width) {
440+
// Destination is height(=newWidth) wide; row index is x, col is y.
441+
dst[x * height + y] = src[rowBase + x]
442+
}
443+
}
444+
return dst
408445
}
409446

410447
/**
@@ -448,11 +485,20 @@ private fun packYPlane(plane: ImageProxy.PlaneProxy, width: Int, height: Int): B
448485
return out
449486
}
450487

451-
/** Decode a single luminance source, swallowing no-reads. */
452-
private fun decodeOrNull(source: LuminanceSource, reader: MultiFormatReader): String? {
488+
/**
489+
* Decode a single luminance source, swallowing no-reads. Hints are passed
490+
* explicitly (not via the reader's stashed state) because the single-arg
491+
* MultiFormatReader.decode wipes hints with setHints(null); the two-arg form
492+
* re-applies POSSIBLE_FORMATS and TRY_HARDER on every call.
493+
*/
494+
private fun decodeOrNull(
495+
source: LuminanceSource,
496+
reader: MultiFormatReader,
497+
hints: Map<DecodeHintType, Any>,
498+
): String? {
453499
val bitmap = BinaryBitmap(HybridBinarizer(source))
454500
return try {
455-
reader.decode(bitmap).text
501+
reader.decode(bitmap, hints).text
456502
} catch (_: Exception) {
457503
null
458504
} finally {

0 commit comments

Comments
 (0)