From d88032ca759eb9e1074145ae43cbab53f370e96f Mon Sep 17 00:00:00 2001 From: Adam J Esslinger Date: Wed, 16 Sep 2026 15:29:34 -0400 Subject: [PATCH] fix: cap the BYTEA read-path preview at 10 KB, sniff MIME from magic bytes (#87) The BYTEA read path base64-encoded the entire byte array into the BLOB::: wire format, with no cap and a hardcoded application/octet-stream MIME. The builtin driver truncates the read/preview path to the first 10,240 bytes (MAX_BLOB_PREVIEW_SIZE) while still reporting the true total_size in the header, and sniffs MIME from the (possibly truncated) content's magic bytes. Large BYTEA columns produced much larger responses in this plugin than in the builtin, slowing the data grid and inflating transfers -- and the hardcoded MIME meant the UI couldn't preview image/PDF blobs the builtin's sniffed MIME allows. Extracted a shared src/utils/blob.rs (encode_blob for the truncated read/preview path, encode_blob_full for the untruncated file-export path) matching the builtin's drivers/common/blob.rs exactly, including sniffing MIME from the truncated preview slice specifically (not the full data) to match the builtin's encode_blob byte-for-byte. Both extract.rs's Type::BYTEA scalar-column arm and its extract_simple_kind_from_bytes (the array-element / composite-field decode path) now use the shared encode_blob; handlers/blob.rs's fetch_blob_as_data_url uses encode_blob_full, replacing its own duplicate copy of that function. Found and fixed a second, previously undetected instance of the same bug while auditing extract.rs for every BYTEA decode path: extract_simple_kind_from_bytes had its own independently-drifted, untruncated copy of the BYTEA arm, reached via array elements (bytea[]) and composite fields rather than top-level scalar columns. The builtin has exactly one BYTEA decode path shared by all three (Kind::Simple routes array/composite elements through the same simple::extract_or_null the scalar path uses); the plugin had drifted into two independent copies. Both are now fixed. TDD: added unit tests to utils/blob_tests.rs (encode_blob/encode_blob_full, including the 20 KB-input/10 KB-preview case the issue specifically requested) and a new bytea_array_element_... test in extract_tests.rs for the array-element path. Confirmed both the scalar and array-element tests fail against the pre-fix code (20480 vs expected 10240) before the fix, pass after. Verified end-to-end against a live PostgreSQL instance: a 20 KB BYTEA value's read-path response reports the true size (20480) but truncates the base64 payload to exactly the first 10,240 bytes; the underlying stored data in PostgreSQL is confirmed untouched (still 20480 bytes) -- only the read-path response is capped; fetch_blob_as_data_url (export path) still returns the complete, untruncated payload; and PNG magic bytes are correctly sniffed to image/png on the read path (previously always application/octet-stream). Filed #106 for a related, out-of-scope finding: extract.rs's binary_blob_wrapper! macro (used for internal planner-statistics types like pg_mcv_list) has its own separate untruncated encoding that should also move to the new shared encode_blob, but is low-severity (those types are essentially never queried directly) and left for a follow-up. --- src/extract.rs | 8 +-- src/extract_tests.rs | 42 ++++++++++++ src/handlers/blob.rs | 18 ++--- src/handlers/blob_tests.rs | 35 ++-------- src/utils/blob.rs | 48 +++++++++++++ src/utils/blob_tests.rs | 136 +++++++++++++++++++++++++++++++++++++ src/utils/mod.rs | 1 + 7 files changed, 239 insertions(+), 49 deletions(-) create mode 100644 src/utils/blob.rs create mode 100644 src/utils/blob_tests.rs diff --git a/src/extract.rs b/src/extract.rs index fba4b6f..ad3e36d 100644 --- a/src/extract.rs +++ b/src/extract.rs @@ -104,8 +104,7 @@ fn extract_simple_kind(col_type: &Type, row: &Row, index: usize) -> JsonValue { try_extract::(row, index, |v| v) } ref t if *t == Type::BYTEA => try_extract::>(row, index, |v| { - let b64 = base64::Engine::encode(&base64::engine::general_purpose::STANDARD, &v); - JsonValue::String(format!("BLOB:{}:application/octet-stream:{}", v.len(), b64)) + JsonValue::String(crate::utils::blob::encode_blob(&v)) }), ref t if *t == Type::INET || *t == Type::CIDR => { try_extract::(row, index, JsonValue::from) @@ -878,10 +877,7 @@ fn extract_simple_kind_from_bytes(ty: &Type, buf: &[u8]) -> JsonValue { serde_json::Value::from_sql(ty, buf).unwrap_or(JsonValue::Null) } _ if *ty == Type::BYTEA => Vec::::from_sql(ty, buf) - .map(|v| { - let b64 = base64::Engine::encode(&base64::engine::general_purpose::STANDARD, &v); - JsonValue::String(format!("BLOB:{}:application/octet-stream:{}", v.len(), b64)) - }) + .map(|v| JsonValue::String(crate::utils::blob::encode_blob(&v))) .unwrap_or(JsonValue::Null), _ if *ty == Type::INET || *ty == Type::CIDR => CidrOrInet::from_sql(ty, buf) .map(JsonValue::from) diff --git a/src/extract_tests.rs b/src/extract_tests.rs index 2aece79..5e84147 100644 --- a/src/extract_tests.rs +++ b/src/extract_tests.rs @@ -218,6 +218,48 @@ fn hstore_array_decodes_each_element_as_a_json_object() { assert_eq!(array.0, serde_json::json!([{"a": "1"}, {"b": "2"}])); } +#[test] +fn bytea_array_element_uses_the_same_truncated_preview_encoding_as_a_scalar_bytea_column() { + // #87: a bytea column reached through the array-element decode path + // (extract_kind_from_bytes -> extract_simple_kind_from_bytes) previously + // had its OWN untruncated copy of the BLOB: encoding, independently + // drifted from the scalar Type::BYTEA arm this issue fixed. Prove a + // large element in a bytea[] column is also truncated to + // MAX_BLOB_PREVIEW_SIZE, with the header still reporting the true size. + let ty = array_type(Type::BYTEA); + let large_elem = vec![0x41u8; 20 * 1024]; // 20 KB, over the 10 KB cap + let small_elem = vec![0xCAu8, 0xFE, 0xBA, 0xBE]; + let bytes = array_wire_bytes(Type::BYTEA.oid(), &[Some(&large_elem), Some(&small_elem)]); + let array = ArrayValue::from_sql(&ty, &bytes).unwrap(); + let serde_json::Value::Array(elems) = array.0 else { + panic!("expected a JSON array"); + }; + + let serde_json::Value::String(large_wire) = &elems[0] else { + panic!("expected a string"); + }; + let header_prefix = format!("BLOB:{}:", large_elem.len()); + assert!( + large_wire.starts_with(&header_prefix), + "the header must report the TRUE size (20480), not the truncated preview size: \ + {large_wire}" + ); + let mime_and_b64 = &large_wire[header_prefix.len()..]; + let (_, b64_payload) = mime_and_b64.split_once(':').unwrap(); + let decoded = + base64::Engine::decode(&base64::engine::general_purpose::STANDARD, b64_payload).unwrap(); + assert_eq!( + decoded.len(), + crate::utils::blob::MAX_BLOB_PREVIEW_SIZE, + "the base64 payload for a bytea[] element must be truncated to the preview cap too" + ); + + assert_eq!( + elems[1], + serde_json::json!("BLOB:4:application/octet-stream:yv66vg==") + ); +} + // Coverage for a gap found during a thoroughness pass on #82: the array // decoder (`extract_element_from_bytes`) is a *separate* per-element // dispatch table from the scalar dispatch (`extract_simple_kind`) — adding diff --git a/src/handlers/blob.rs b/src/handlers/blob.rs index 4583a86..c5b5c86 100644 --- a/src/handlers/blob.rs +++ b/src/handlers/blob.rs @@ -60,7 +60,10 @@ pub async fn fetch_blob_as_data_url(id: Value, params: &Value) -> Value { .unwrap_or_default(); match fetch_blob_bytes(&conn_params, table, col_name, &pk_map, schema).await { - Ok(bytes) => ok_response(id, Value::from(encode_blob_full(&bytes))), + Ok(bytes) => ok_response( + id, + Value::from(crate::utils::blob::encode_blob_full(&bytes)), + ), Err(e) => error_response(id, -32603, &e), } } @@ -126,19 +129,6 @@ fn validate_writable_file_path(file_path: &str) -> Result<(), String> { } } -/// Encode raw bytes into the canonical BLOB wire format: -/// `"BLOB:::"`. MIME type is sniffed from the -/// content's magic bytes; unrecognized content falls back to -/// `application/octet-stream`. Matches `encode_blob_full` in -/// `src-tauri/src/drivers/common/blob.rs`. -fn encode_blob_full(data: &[u8]) -> String { - let mime_type = infer::get(data) - .map(|k| k.mime_type()) - .unwrap_or("application/octet-stream"); - let b64 = base64::Engine::encode(&base64::engine::general_purpose::STANDARD, data); - format!("BLOB:{}:{}:{}", data.len(), mime_type, b64) -} - #[cfg(test)] #[path = "blob_tests.rs"] mod blob_tests; diff --git a/src/handlers/blob_tests.rs b/src/handlers/blob_tests.rs index 8fde688..d38270a 100644 --- a/src/handlers/blob_tests.rs +++ b/src/handlers/blob_tests.rs @@ -1,34 +1,11 @@ -//! Unit tests for `blob.rs`'s pure encoding helper. Sibling test file per -//! repo convention (`.rules/rust.md` #4/#5) — loaded via -//! `#[cfg(test)] #[path = "blob_tests.rs"] mod blob_tests;`. - -use super::{encode_blob_full, validate_writable_file_path}; - -#[test] -fn encodes_size_mime_and_base64() { - // 4 bytes (0xCA 0xFE 0xBA 0xBE) — not a recognized magic-byte format, so - // infer falls back to application/octet-stream. - let bytes = [0xCA, 0xFE, 0xBA, 0xBE]; - let wire = encode_blob_full(&bytes); - assert_eq!(wire, "BLOB:4:application/octet-stream:yv66vg=="); -} - -#[test] -fn empty_input_encodes_zero_size() { - let wire = encode_blob_full(&[]); - assert_eq!(wire, "BLOB:0:application/octet-stream:"); -} - -#[test] -fn sniffs_recognized_magic_bytes() { - // PNG signature. - let bytes = [0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A]; - let wire = encode_blob_full(&bytes); - assert!(wire.starts_with("BLOB:8:image/png:")); -} +//! Unit tests for `blob.rs`'s pure helper (`validate_writable_file_path`). +//! `encode_blob`/`encode_blob_full` moved to `crate::utils::blob` (#87) — +//! their tests live in `utils/blob.rs`'s own sibling test module now. +//! Sibling test file per repo convention (`.rules/rust.md` #4/#5) — loaded +//! via `#[cfg(test)] #[path = "blob_tests.rs"] mod blob_tests;`. mod validate_writable_file_path_tests { - use super::validate_writable_file_path; + use super::super::validate_writable_file_path; #[test] fn rejects_empty_path() { diff --git a/src/utils/blob.rs b/src/utils/blob.rs new file mode 100644 index 0000000..50d7ef9 --- /dev/null +++ b/src/utils/blob.rs @@ -0,0 +1,48 @@ +//! Shared BLOB (bytea) wire-format encoding, used by both the read path +//! (`extract.rs`'s `Type::BYTEA` arm) and the file-export path +//! (`handlers/blob.rs::fetch_blob_as_data_url`). +//! +//! Matches `src-tauri/src/drivers/common/blob.rs` exactly. + +/// Maximum number of bytes base64-encoded into a read/preview response. +/// Larger values are truncated to this many bytes before encoding — the +/// `BLOB:` header still reports the true, untruncated size, so the UI knows +/// the real length without paying to transfer it. +pub const MAX_BLOB_PREVIEW_SIZE: usize = 10_240; + +/// Encode raw bytes into the canonical BLOB wire format for the read/preview +/// path: `"BLOB:::"`. Truncates the +/// encoded payload to [`MAX_BLOB_PREVIEW_SIZE`] bytes (MIME is sniffed from +/// that same truncated preview, matching the builtin), while `total_size` +/// always reports the untruncated length. +pub fn encode_blob(data: &[u8]) -> String { + let total_size = data.len(); + let preview = if total_size > MAX_BLOB_PREVIEW_SIZE { + &data[..MAX_BLOB_PREVIEW_SIZE] + } else { + data + }; + + let mime_type = infer::get(preview) + .map(|k| k.mime_type()) + .unwrap_or("application/octet-stream"); + let b64 = base64::Engine::encode(&base64::engine::general_purpose::STANDARD, preview); + + format!("BLOB:{}:{}:{}", total_size, mime_type, b64) +} + +/// Encode raw bytes into the canonical BLOB wire format, preserving the +/// complete data with no truncation — used by upload/write/export paths so +/// files aren't silently truncated. +pub fn encode_blob_full(data: &[u8]) -> String { + let mime_type = infer::get(data) + .map(|k| k.mime_type()) + .unwrap_or("application/octet-stream"); + let b64 = base64::Engine::encode(&base64::engine::general_purpose::STANDARD, data); + + format!("BLOB:{}:{}:{}", data.len(), mime_type, b64) +} + +#[cfg(test)] +#[path = "blob_tests.rs"] +mod blob_tests; diff --git a/src/utils/blob_tests.rs b/src/utils/blob_tests.rs new file mode 100644 index 0000000..f2f2a69 --- /dev/null +++ b/src/utils/blob_tests.rs @@ -0,0 +1,136 @@ +//! Unit tests for `blob.rs`'s BLOB wire-format encoders. Sibling test file +//! per repo convention (`.rules/rust.md` #4/#5) — loaded via +//! `#[cfg(test)] #[path = "blob_tests.rs"] mod blob_tests;`. + +use super::{encode_blob, encode_blob_full, MAX_BLOB_PREVIEW_SIZE}; + +mod encode_blob_tests { + use super::*; + + #[test] + fn encodes_size_mime_and_base64() { + // 4 bytes (0xCA 0xFE 0xBA 0xBE) — not a recognized magic-byte format, + // so infer falls back to application/octet-stream. + let bytes = [0xCA, 0xFE, 0xBA, 0xBE]; + let wire = encode_blob(&bytes); + assert_eq!(wire, "BLOB:4:application/octet-stream:yv66vg=="); + } + + #[test] + fn empty_input_encodes_zero_size() { + let wire = encode_blob(&[]); + assert_eq!(wire, "BLOB:0:application/octet-stream:"); + } + + #[test] + fn sniffs_recognized_magic_bytes() { + // PNG signature. + let bytes = [0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A]; + let wire = encode_blob(&bytes); + assert!(wire.starts_with("BLOB:8:image/png:")); + } + + #[test] + fn small_blob_under_preview_cap_encodes_in_full() { + let bytes = vec![0x42u8; 100]; + let wire = encode_blob(&bytes); + let header = format!("BLOB:{}:application/octet-stream:", bytes.len()); + assert!(wire.starts_with(&header)); + let b64_payload = &wire[header.len()..]; + let decoded = + base64::Engine::decode(&base64::engine::general_purpose::STANDARD, b64_payload) + .unwrap(); + assert_eq!( + decoded, bytes, + "under the cap, the full payload must round-trip" + ); + } + + #[test] + fn large_blob_reports_true_size_but_truncates_payload_to_preview_cap() { + // 20 KB input -- outlasts MAX_BLOB_PREVIEW_SIZE (10 KB), per #87. + let total_size = 20 * 1024; + let bytes = vec![0x41u8; total_size]; + let wire = encode_blob(&bytes); + + let header_prefix = format!("BLOB:{}:", total_size); + assert!( + wire.starts_with(&header_prefix), + "the BLOB: header must report the TRUE total size (20480), not the truncated \ + preview size, so the UI knows the real length: {wire}" + ); + + let mime_and_b64 = &wire[header_prefix.len()..]; + let (_, b64_payload) = mime_and_b64.split_once(':').expect("mime:base64 shape"); + let decoded = + base64::Engine::decode(&base64::engine::general_purpose::STANDARD, b64_payload) + .unwrap(); + assert_eq!( + decoded.len(), + MAX_BLOB_PREVIEW_SIZE, + "the base64 payload must only cover the first {MAX_BLOB_PREVIEW_SIZE} bytes, \ + not the full 20480-byte input" + ); + assert_eq!(decoded, bytes[..MAX_BLOB_PREVIEW_SIZE]); + } + + #[test] + fn blob_exactly_at_preview_cap_is_not_truncated() { + let bytes = vec![0x99u8; MAX_BLOB_PREVIEW_SIZE]; + let wire = encode_blob(&bytes); + let header_prefix = format!("BLOB:{}:", MAX_BLOB_PREVIEW_SIZE); + assert!(wire.starts_with(&header_prefix)); + let mime_and_b64 = &wire[header_prefix.len()..]; + let (_, b64_payload) = mime_and_b64.split_once(':').unwrap(); + let decoded = + base64::Engine::decode(&base64::engine::general_purpose::STANDARD, b64_payload) + .unwrap(); + assert_eq!( + decoded.len(), + MAX_BLOB_PREVIEW_SIZE, + "the boundary itself must not truncate" + ); + } +} + +mod encode_blob_full_tests { + use super::*; + + #[test] + fn encodes_size_mime_and_base64() { + let bytes = [0xCA, 0xFE, 0xBA, 0xBE]; + let wire = encode_blob_full(&bytes); + assert_eq!(wire, "BLOB:4:application/octet-stream:yv66vg=="); + } + + #[test] + fn empty_input_encodes_zero_size() { + let wire = encode_blob_full(&[]); + assert_eq!(wire, "BLOB:0:application/octet-stream:"); + } + + #[test] + fn sniffs_recognized_magic_bytes() { + let bytes = [0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A]; + let wire = encode_blob_full(&bytes); + assert!(wire.starts_with("BLOB:8:image/png:")); + } + + #[test] + fn large_blob_is_never_truncated() { + // Unlike encode_blob, the full-fidelity encoder must preserve every + // byte regardless of size -- this is what the file-export path + // (fetch_blob_as_data_url) relies on to not silently corrupt files. + let total_size = 20 * 1024; + let bytes = vec![0x41u8; total_size]; + let wire = encode_blob_full(&bytes); + + let header_prefix = format!("BLOB:{}:", total_size); + let mime_and_b64 = &wire[header_prefix.len()..]; + let (_, b64_payload) = mime_and_b64.split_once(':').unwrap(); + let decoded = + base64::Engine::decode(&base64::engine::general_purpose::STANDARD, b64_payload) + .unwrap(); + assert_eq!(decoded, bytes, "encode_blob_full must never truncate"); + } +} diff --git a/src/utils/mod.rs b/src/utils/mod.rs index b7e27d7..67ffc50 100644 --- a/src/utils/mod.rs +++ b/src/utils/mod.rs @@ -1,5 +1,6 @@ //! Utility modules. +pub mod blob; pub mod identifiers; pub mod pagination; #[cfg(test)]