diff --git a/mbtiles/src/errors.rs b/mbtiles/src/errors.rs index 935237909..543de62c8 100644 --- a/mbtiles/src/errors.rs +++ b/mbtiles/src/errors.rs @@ -37,6 +37,9 @@ pub enum MbtError { #[error("At least one tile has mismatching hash: stored value is `{1}` != computed value `{2}` in MBTile file {0}")] IncorrectTileHash(String, String, String), + #[error("At least one tile in the tiles table/view has an invalid value: zoom_level={1}, tile_column={2}, tile_row={3} in MBTile file {0}")] + InvalidTileIndex(String, String, String, String), + #[error("Computed aggregate tiles hash {0} does not match tile data in metadata {1} for MBTile file {2}")] AggHashMismatch(String, String, String), diff --git a/mbtiles/src/validation.rs b/mbtiles/src/validation.rs index e97cd49b6..d222c7df9 100644 --- a/mbtiles/src/validation.rs +++ b/mbtiles/src/validation.rs @@ -1,10 +1,11 @@ use std::collections::HashSet; +use std::str::from_utf8; #[cfg(feature = "cli")] use clap::ValueEnum; use enum_display::EnumDisplay; use log::{debug, info, warn}; -use martin_tile_utils::{Format, TileInfo}; +use martin_tile_utils::{Format, TileInfo, MAX_ZOOM}; use serde::Serialize; use serde_json::Value; use sqlx::sqlite::SqliteRow; @@ -18,6 +19,7 @@ use crate::queries::{ }; use crate::MbtError::{ AggHashMismatch, AggHashValueNotFound, FailedIntegrityCheck, IncorrectTileHash, + InvalidTileIndex, }; use crate::{invert_y_value, Mbtiles}; @@ -83,6 +85,7 @@ impl Mbtiles { self.open_readonly().await? }; self.check_integrity(&mut conn, check_type).await?; + self.check_tiles_type_validity(&mut conn).await?; self.check_each_tile_hash(&mut conn).await?; match agg_hash { AggHashType::Verify => self.check_agg_tiles_hashes(&mut conn).await, @@ -311,6 +314,64 @@ impl Mbtiles { Ok(()) } + /// Check that the tiles table has the expected column, row, zoom, and data values + pub async fn check_tiles_type_validity(&self, conn: &mut T) -> MbtResult<()> + where + for<'e> &'e mut T: SqliteExecutor<'e>, + { + let sql = format!( + " +SELECT zoom_level, tile_column, tile_row +FROM tiles +WHERE FALSE + OR typeof(zoom_level) != 'integer' + OR zoom_level < 0 + OR zoom_level > {MAX_ZOOM} + OR typeof(tile_column) != 'integer' + OR tile_column < 0 + OR tile_column >= (1 << zoom_level) + OR typeof(tile_row) != 'integer' + OR tile_row < 0 + OR tile_row >= (1 << zoom_level) + OR (typeof(tile_data) != 'blob' AND typeof(tile_data) != 'null') +LIMIT 1;" + ); + + if let Some(row) = query(&sql).fetch_optional(&mut *conn).await? { + let mut res: Vec = Vec::with_capacity(3); + for idx in (0..3).rev() { + use sqlx::ValueRef as _; + let raw = row.try_get_raw(idx)?; + if raw.is_null() { + res.push("NULL".to_string()); + } else if let Ok(v) = row.try_get::(idx) { + res.push(format!(r#""{v}" (TEXT)"#)); + } else if let Ok(v) = row.try_get::, _>(idx) { + res.push(format!( + r#""{}" (BLOB)"#, + from_utf8(&v).unwrap_or("") + )); + } else if let Ok(v) = row.try_get::(idx) { + res.push(format!("{v}")); + } else if let Ok(v) = row.try_get::(idx) { + res.push(format!(r#"{v} (REAL)"#)); + } else { + res.push(format!("{:?}", raw.type_info())); + } + } + + return Err(InvalidTileIndex( + self.filepath().to_string(), + res.pop().unwrap(), + res.pop().unwrap(), + res.pop().unwrap(), + )); + } + + info!("All values in the `tiles` table/view are valid for {self}"); + Ok(()) + } + pub async fn check_agg_tiles_hashes(&self, conn: &mut T) -> MbtResult where for<'e> &'e mut T: SqliteExecutor<'e>, diff --git a/mbtiles/tests/validate.rs b/mbtiles/tests/validate.rs new file mode 100644 index 000000000..7192d4538 --- /dev/null +++ b/mbtiles/tests/validate.rs @@ -0,0 +1,119 @@ +use martin_tile_utils::MAX_ZOOM; +use mbtiles::MbtError::InvalidTileIndex; +use mbtiles::{create_metadata_table, Mbtiles}; +use rstest::rstest; +use sqlx::{query, Executor as _, SqliteConnection}; + +#[ctor::ctor] +fn init() { + let _ = env_logger::builder().is_test(true).try_init(); +} + +async fn new(values: &str) -> (Mbtiles, SqliteConnection) { + let mbtiles = Mbtiles::new(":memory:").unwrap(); + let mut conn = mbtiles.open().await.unwrap(); + create_metadata_table(&mut conn).await.unwrap(); + + conn.execute( + "CREATE TABLE tiles ( + zoom_level integer, + tile_column integer, + tile_row integer, + tile_data blob, + PRIMARY KEY(zoom_level, tile_column, tile_row));", + ) + .await + .unwrap(); + + let sql = format!( + "INSERT INTO tiles (zoom_level, tile_column, tile_row, tile_data) + VALUES ({values});" + ); + query(&sql).execute(&mut conn).await.expect(&sql); + + (mbtiles, conn) +} + +macro_rules! ok { + ($($vals:tt)*) => {{ + let vals = format!($($vals)*); + let (mbt, mut conn) = new(&vals).await; + let res = mbt.check_tiles_type_validity(&mut conn).await; + assert!(res.is_ok(), "check_tiles_xyz_validity({vals}) = {res:?}, expected Ok"); + }}; +} + +macro_rules! err { + ($($vals:tt)*) => { + let vals = format!($($vals)*); + let (mbt, mut conn) = new(&vals).await; + match mbt.check_tiles_type_validity(&mut conn).await { + Ok(()) => panic!("check_tiles_xyz_validity({vals}) was expected to fail"), + Err(e) => match e { + InvalidTileIndex(..) => {} + _ => panic!("check_tiles_xyz_validity({vals}) = Err({e:?}), expected Err(InvalidTileIndex)"), + }, + } + }; +} + +#[rstest] +#[case("", ", 0, 0, NULL")] // test tile_zoom +#[case("0, ", ", 0, NULL")] // test tile_column +#[case("0, 0, ", ", NULL")] // test tile_row +#[trace] +#[actix_rt::test] +async fn integers(#[case] prefix: &str, #[case] suffix: &str) { + ok!("{prefix} 0 {suffix}"); + + err!("{prefix} NULL {suffix}"); + err!("{prefix} -1 {suffix}"); + err!("{prefix} 0.2 {suffix}"); + err!("{prefix} '' {suffix}"); + err!("{prefix} 'a' {suffix}"); + + err!("{prefix} CAST(1 AS BLOB) {suffix}"); + err!("{prefix} CAST('1' AS BLOB) {suffix}"); + + // These fail for some reason, probably due to internal SQLite casting/affinity rules? + // err!("{prefix} '1' {suffix}"); + // err!("{prefix} CAST(1 AS REAL) {suffix}"); + // err!("{prefix} CAST(1.0 AS NUMERIC) {suffix}"); + // err!("{prefix} CAST(1 AS TEXT) {suffix}"); +} + +#[rstest] +#[case("", ", 0, NULL")] // test tile_column +#[case("0, ", ", NULL")] // test tile_row +#[trace] +#[actix_rt::test] +async fn tile_coordinate(#[case] prefix: &str, #[case] suffix: &str) { + ok!("0, {prefix} 0 {suffix}"); + ok!("1, {prefix} 1 {suffix}"); + ok!("2, {prefix} 3 {suffix}"); + ok!("3, {prefix} 7 {suffix}"); + ok!("30, {prefix} 0 {suffix}"); + ok!("30, {prefix} 1073741823 {suffix}"); + + err!("0, {prefix} 1 {suffix}"); + err!("1, {prefix} 2 {suffix}"); + err!("2, {prefix} 4 {suffix}"); + err!("3, {prefix} 8 {suffix}"); + err!("30, {prefix} 1073741824 {suffix}"); + err!("{MAX_ZOOM}, {prefix} 1073741824 {suffix}"); + err!("{}, {prefix} 0 {suffix}", MAX_ZOOM + 1); // unsupported zoom +} + +#[actix_rt::test] +async fn tile_data() { + ok!("0, 0, 0, NULL"); + ok!("0, 0, 0, CAST('' AS BLOB)"); + ok!("0, 0, 0, CAST('abc' AS BLOB)"); + ok!("0, 0, 0, CAST(123 AS BLOB)"); + + err!("0, 0, 0, 0"); + err!("0, 0, 0, 0.1"); + err!("0, 0, 0, CAST('' AS TEXT)"); + err!("0, 0, 0, CAST('abc' AS TEXT)"); + err!("0, 0, 0, CAST(123 AS TEXT)"); +} diff --git a/tests/expected/martin-cp/flat-with-hash_validate.txt b/tests/expected/martin-cp/flat-with-hash_validate.txt index 48bfc7abf..0a344a967 100644 --- a/tests/expected/martin-cp/flat-with-hash_validate.txt +++ b/tests/expected/martin-cp/flat-with-hash_validate.txt @@ -1,2 +1,3 @@ [INFO ] Quick integrity check passed for tests/mbtiles_temp_files/cp_flat-with-hash.mbtiles +[INFO ] All values in the `tiles` table/view are valid for tests/mbtiles_temp_files/cp_flat-with-hash.mbtiles [INFO ] All tile hashes are valid for tests/mbtiles_temp_files/cp_flat-with-hash.mbtiles diff --git a/tests/expected/martin-cp/flat_validate.txt b/tests/expected/martin-cp/flat_validate.txt index d64ffd90d..19fea05ff 100644 --- a/tests/expected/martin-cp/flat_validate.txt +++ b/tests/expected/martin-cp/flat_validate.txt @@ -1,2 +1,3 @@ [INFO ] Quick integrity check passed for tests/mbtiles_temp_files/cp_flat.mbtiles +[INFO ] All values in the `tiles` table/view are valid for tests/mbtiles_temp_files/cp_flat.mbtiles [INFO ] Skipping per-tile hash validation because this is a flat MBTiles file diff --git a/tests/expected/martin-cp/normalized_validate.txt b/tests/expected/martin-cp/normalized_validate.txt index 3ef717bf5..ae3365050 100644 --- a/tests/expected/martin-cp/normalized_validate.txt +++ b/tests/expected/martin-cp/normalized_validate.txt @@ -1,2 +1,3 @@ [INFO ] Quick integrity check passed for tests/mbtiles_temp_files/cp_normalized.mbtiles +[INFO ] All values in the `tiles` table/view are valid for tests/mbtiles_temp_files/cp_normalized.mbtiles [INFO ] All tile hashes are valid for tests/mbtiles_temp_files/cp_normalized.mbtiles diff --git a/tests/expected/mbtiles/validate-bad.txt b/tests/expected/mbtiles/validate-bad-hash.txt similarity index 78% rename from tests/expected/mbtiles/validate-bad.txt rename to tests/expected/mbtiles/validate-bad-hash.txt index 242a6aaaa..4ba1d3e25 100644 --- a/tests/expected/mbtiles/validate-bad.txt +++ b/tests/expected/mbtiles/validate-bad-hash.txt @@ -1,3 +1,4 @@ [INFO ] Quick integrity check passed for ./tests/fixtures/files/bad_hash.mbtiles +[INFO ] All values in the `tiles` table/view are valid for ./tests/fixtures/files/bad_hash.mbtiles [INFO ] All tile hashes are valid for ./tests/fixtures/files/bad_hash.mbtiles [ERROR] Computed aggregate tiles hash D4E1030D57751A0B45A28A71267E46B8 does not match tile data in metadata CAFEC0DEDEADBEEFDEADBEEFDEADBEEF for MBTile file ./tests/fixtures/files/bad_hash.mbtiles diff --git a/tests/expected/mbtiles/validate-bad-tiles.txt b/tests/expected/mbtiles/validate-bad-tiles.txt new file mode 100644 index 000000000..372bcbc8f --- /dev/null +++ b/tests/expected/mbtiles/validate-bad-tiles.txt @@ -0,0 +1,2 @@ +[INFO ] Quick integrity check passed for ./tests/fixtures/files/invalid-tile-idx.mbtiles +[ERROR] At least one tile in the tiles table/view has an invalid value: zoom_level=6, tile_column=10, tile_row=64 in MBTile file ./tests/fixtures/files/invalid-tile-idx.mbtiles diff --git a/tests/expected/mbtiles/validate-fix.txt b/tests/expected/mbtiles/validate-fix.txt index 34ccfa33e..4e5815aae 100644 --- a/tests/expected/mbtiles/validate-fix.txt +++ b/tests/expected/mbtiles/validate-fix.txt @@ -1,3 +1,4 @@ [INFO ] Quick integrity check passed for tests/mbtiles_temp_files/fix_bad_hash.mbtiles +[INFO ] All values in the `tiles` table/view are valid for tests/mbtiles_temp_files/fix_bad_hash.mbtiles [INFO ] All tile hashes are valid for tests/mbtiles_temp_files/fix_bad_hash.mbtiles [INFO ] Updating agg_tiles_hash from CAFEC0DEDEADBEEFDEADBEEFDEADBEEF to D4E1030D57751A0B45A28A71267E46B8 in tests/mbtiles_temp_files/fix_bad_hash.mbtiles diff --git a/tests/expected/mbtiles/validate-fix2.txt b/tests/expected/mbtiles/validate-fix2.txt index 291281294..23d4067bf 100644 --- a/tests/expected/mbtiles/validate-fix2.txt +++ b/tests/expected/mbtiles/validate-fix2.txt @@ -1,3 +1,4 @@ [INFO ] Quick integrity check passed for tests/mbtiles_temp_files/fix_bad_hash.mbtiles +[INFO ] All values in the `tiles` table/view are valid for tests/mbtiles_temp_files/fix_bad_hash.mbtiles [INFO ] All tile hashes are valid for tests/mbtiles_temp_files/fix_bad_hash.mbtiles [INFO ] The agg_tiles_hashes=D4E1030D57751A0B45A28A71267E46B8 has been verified for tests/mbtiles_temp_files/fix_bad_hash.mbtiles diff --git a/tests/expected/mbtiles/validate-ok.txt b/tests/expected/mbtiles/validate-ok.txt index 87a15d314..f7ad6204b 100644 --- a/tests/expected/mbtiles/validate-ok.txt +++ b/tests/expected/mbtiles/validate-ok.txt @@ -1,3 +1,4 @@ [INFO ] Quick integrity check passed for ./tests/fixtures/mbtiles/zoomed_world_cities.mbtiles +[INFO ] All values in the `tiles` table/view are valid for ./tests/fixtures/mbtiles/zoomed_world_cities.mbtiles [INFO ] All tile hashes are valid for ./tests/fixtures/mbtiles/zoomed_world_cities.mbtiles [INFO ] The agg_tiles_hashes=D4E1030D57751A0B45A28A71267E46B8 has been verified for ./tests/fixtures/mbtiles/zoomed_world_cities.mbtiles diff --git a/tests/fixtures/files/invalid-tile-idx.mbtiles b/tests/fixtures/files/invalid-tile-idx.mbtiles new file mode 100644 index 000000000..e672c4803 Binary files /dev/null and b/tests/fixtures/files/invalid-tile-idx.mbtiles differ diff --git a/tests/test.sh b/tests/test.sh index e38ec0397..634764285 100755 --- a/tests/test.sh +++ b/tests/test.sh @@ -429,9 +429,14 @@ if [[ "$MBTILES_BIN" != "-" ]]; then $MBTILES_BIN validate ./tests/fixtures/mbtiles/zoomed_world_cities.mbtiles 2>&1 | tee "$TEST_OUT_DIR/validate-ok.txt" set +e - $MBTILES_BIN validate ./tests/fixtures/files/bad_hash.mbtiles 2>&1 | tee "$TEST_OUT_DIR/validate-bad.txt" + $MBTILES_BIN validate ./tests/fixtures/files/invalid-tile-idx.mbtiles 2>&1 | tee "$TEST_OUT_DIR/validate-bad-tiles.txt" if [[ $? -eq 0 ]]; then - echo "ERROR: validate with bad_hash should have failed" + echo "ERROR: validate with invalid-tile-idx.mbtiles should have failed" + exit 1 + fi + $MBTILES_BIN validate ./tests/fixtures/files/bad_hash.mbtiles 2>&1 | tee "$TEST_OUT_DIR/validate-bad-hash.txt" + if [[ $? -eq 0 ]]; then + echo "ERROR: validate with bad_hash.mbtiles should have failed" exit 1 fi set -e