From d3b0a091a921997559457b4923d9367c436e5d80 Mon Sep 17 00:00:00 2001 From: Paul Florence Date: Mon, 12 Jul 2021 16:50:10 +0200 Subject: [PATCH 1/3] Fix invalid json escaping --- src/function_source.rs | 10 ++++------ src/utils.rs | 8 +++++--- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/function_source.rs b/src/function_source.rs index e5de1ca24..1cfb55e78 100644 --- a/src/function_source.rs +++ b/src/function_source.rs @@ -1,4 +1,4 @@ -use postgres::types::{Json, Type}; +use postgres::types::Type; use postgres_protocol::escape::escape_identifier; use serde::{Deserialize, Serialize}; use std::collections::HashMap; @@ -7,7 +7,7 @@ use tilejson::{TileJSON, TileJSONBuilder}; use crate::db::Connection; use crate::source::{Query, Source, Tile, Xyz}; -use crate::utils::query_to_json_string; +use crate::utils::query_to_json; #[derive(Clone, Debug, Serialize, Deserialize)] pub struct FunctionSource { @@ -42,8 +42,7 @@ impl Source for FunctionSource { let empty_query = HashMap::new(); let query = query.as_ref().unwrap_or(&empty_query); - let query_json_string = query_to_json_string(&query) - .map_err(|err| io::Error::new(io::ErrorKind::Other, err))?; + let query_json = query_to_json(query); // Query preparation : the schema and function can't be part of a prepared query, so they // need to be escaped by hand. @@ -69,9 +68,8 @@ impl Source for FunctionSource { ) .map_err(|err| io::Error::new(io::ErrorKind::Other, err))?; - let json = Json(query_json_string); let tile = conn - .query_one(&query, &[&xyz.x, &xyz.y, &xyz.z, &json]) + .query_one(&query, &[&xyz.x, &xyz.y, &xyz.z, &query_json]) .map(|row| row.get(self.function.as_str())) .map_err(|err| io::Error::new(io::ErrorKind::Other, err))?; diff --git a/src/utils.rs b/src/utils.rs index af714c560..6d4bb0314 100755 --- a/src/utils.rs +++ b/src/utils.rs @@ -1,6 +1,8 @@ use std::collections::HashMap; use crate::source::{Query, Xyz}; +use postgres::types::Json; +use serde_json::Value; pub fn prettify_error(message: &'static str) -> impl Fn(E) -> std::io::Error { move |error| std::io::Error::new(std::io::ErrorKind::Other, format!("{}: {}", message, error)) @@ -38,16 +40,16 @@ pub fn json_to_hashmap(value: &serde_json::Value) -> HashMap { hashmap } -pub fn query_to_json_string(query: &Query) -> Result { +pub fn query_to_json(query: &Query) -> Json> { let mut query_as_json = HashMap::new(); for (k, v) in query.iter() { let json_value: serde_json::Value = serde_json::from_str(v).unwrap_or_else(|_| serde_json::Value::String(v.clone())); - query_as_json.insert(k, json_value); + query_as_json.insert(k.clone(), json_value); } - serde_json::to_string(&query_as_json) + Json(query_as_json) } pub fn get_bounds_cte(srid_bounds: String) -> String { From ee7cde9e0387e14c1bdbd1eab74d820aea54df3c Mon Sep 17 00:00:00 2001 From: Stepan Kuzmin Date: Thu, 15 Jul 2021 19:22:57 +0300 Subject: [PATCH 2/3] test: add test for function_source with query params --- .github/workflows/ci.yml | 1 + .github/workflows/grcov.yml | 1 + src/dev.rs | 17 +++++++++----- .../fixtures/function_source_query_params.sql | 21 +++++++++++++++++ tests/initdb-martin.sh | 1 + tests/server_test.rs | 23 +++++++++++++++++++ 6 files changed, 58 insertions(+), 6 deletions(-) create mode 100644 tests/fixtures/function_source_query_params.sql diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e2405c73e..d311c96a7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -98,6 +98,7 @@ jobs: psql -h $POSTGRES_HOST -p $POSTGRES_PORT -U postgres -d test -f tests/fixtures/points1_source.sql psql -h $POSTGRES_HOST -p $POSTGRES_PORT -U postgres -d test -f tests/fixtures/points2_source.sql psql -h $POSTGRES_HOST -p $POSTGRES_PORT -U postgres -d test -f tests/fixtures/function_source.sql + psql -h $POSTGRES_HOST -p $POSTGRES_PORT -U postgres -d test -f tests/fixtures/function_source_query_params.sql env: POSTGRES_HOST: localhost POSTGRES_PORT: ${{ job.services.postgres.ports[5432] }} diff --git a/.github/workflows/grcov.yml b/.github/workflows/grcov.yml index 4148bfbd7..832500440 100644 --- a/.github/workflows/grcov.yml +++ b/.github/workflows/grcov.yml @@ -33,6 +33,7 @@ jobs: psql -h $POSTGRES_HOST -p $POSTGRES_PORT -U postgres -d test -f tests/fixtures/points1_source.sql psql -h $POSTGRES_HOST -p $POSTGRES_PORT -U postgres -d test -f tests/fixtures/points2_source.sql psql -h $POSTGRES_HOST -p $POSTGRES_PORT -U postgres -d test -f tests/fixtures/function_source.sql + psql -h $POSTGRES_HOST -p $POSTGRES_PORT -U postgres -d test -f tests/fixtures/function_source_query_params.sql env: POSTGRES_HOST: localhost POSTGRES_PORT: ${{ job.services.postgres.ports[5432] }} diff --git a/src/dev.rs b/src/dev.rs index 1c4329b05..7b57d0242 100644 --- a/src/dev.rs +++ b/src/dev.rs @@ -63,15 +63,20 @@ pub fn mock_table_sources() -> Option { } pub fn mock_function_sources() -> Option { - let id = "public.function_source"; - let source = FunctionSource { - id: id.to_owned(), + let mut function_sources: FunctionSources = HashMap::new(); + + function_sources.insert("public.function_source".to_owned(), Box::new(FunctionSource { + id: "public.function_source".to_owned(), schema: "public".to_owned(), function: "function_source".to_owned(), - }; + })); + + function_sources.insert("public.function_source_query_params".to_owned(), Box::new(FunctionSource { + id: "public.function_source_query_params".to_owned(), + schema: "public".to_owned(), + function: "function_source_query_params".to_owned(), + })); - let mut function_sources: FunctionSources = HashMap::new(); - function_sources.insert(id.to_owned(), Box::new(source)); Some(function_sources) } diff --git a/tests/fixtures/function_source_query_params.sql b/tests/fixtures/function_source_query_params.sql new file mode 100644 index 000000000..e26ca7a36 --- /dev/null +++ b/tests/fixtures/function_source_query_params.sql @@ -0,0 +1,21 @@ +DROP FUNCTION IF EXISTS public.function_source_query_params; +CREATE OR REPLACE FUNCTION public.function_source_query_params(z integer, x integer, y integer, query_params json) RETURNS bytea AS $$ +DECLARE + mvt bytea; +BEGIN + RAISE DEBUG 'query_params: %', query_params; + + IF (query_params->>'token')::varchar IS NULL THEN + RAISE EXCEPTION 'the `token` json parameter does not exist in `query_params`'; + END IF; + + SELECT INTO mvt ST_AsMVT(tile, 'public.function_source_query_params', 4096, 'geom') FROM ( + SELECT + ST_AsMVTGeom(ST_Transform(geom, 3857), TileBBox(z, x, y, 3857), 4096, 64, true) AS geom + FROM public.table_source + WHERE geom && TileBBox(z, x, y, 4326) + ) as tile WHERE geom IS NOT NULL; + + RETURN mvt; +END +$$ LANGUAGE plpgsql IMMUTABLE STRICT PARALLEL SAFE; \ No newline at end of file diff --git a/tests/initdb-martin.sh b/tests/initdb-martin.sh index ba5a99fe8..ece7a467b 100755 --- a/tests/initdb-martin.sh +++ b/tests/initdb-martin.sh @@ -10,6 +10,7 @@ echo "Loading Martin fixtures into $POSTGRES_DB" psql --dbname="$POSTGRES_DB" -f /fixtures/TileBBox.sql psql --dbname="$POSTGRES_DB" -f /fixtures/table_source.sql psql --dbname="$POSTGRES_DB" -f /fixtures/function_source.sql +psql --dbname="$POSTGRES_DB" -f /fixtures/function_source_query_params.sql psql --dbname="$POSTGRES_DB" -f /fixtures/points1_source.sql psql --dbname="$POSTGRES_DB" -f /fixtures/points2_source.sql diff --git a/tests/server_test.rs b/tests/server_test.rs index 28191b270..e0c9dc1c8 100644 --- a/tests/server_test.rs +++ b/tests/server_test.rs @@ -200,6 +200,29 @@ async fn test_get_function_source_tile_ok() { assert!(response.status().is_success()); } +#[actix_rt::test] +async fn test_get_function_source_query_params_ok() { + init(); + + let state = mock_state(None, mock_function_sources(), false); + let mut app = test::init_service(App::new().data(state).configure(router)).await; + + let req = test::TestRequest::get() + .uri("/rpc/public.function_source_query_params/0/0/0.pbf") + .to_request(); + + let response = test::call_service(&mut app, req).await; + println!("response.status = {:?}", response.status()); + assert!(response.status().is_server_error()); + + let req = test::TestRequest::get() + .uri("/rpc/public.function_source_query_params/0/0/0.pbf?token=martin") + .to_request(); + + let response = test::call_service(&mut app, req).await; + assert!(response.status().is_success()); +} + #[actix_rt::test] async fn test_get_health_returns_ok() { init(); From e78333e90f790e51ea3282fa7ac97bb96040e3af Mon Sep 17 00:00:00 2001 From: Stepan Kuzmin Date: Thu, 15 Jul 2021 19:25:41 +0300 Subject: [PATCH 3/3] fix formatting --- src/dev.rs | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/src/dev.rs b/src/dev.rs index 7b57d0242..a8bc08d67 100644 --- a/src/dev.rs +++ b/src/dev.rs @@ -65,17 +65,23 @@ pub fn mock_table_sources() -> Option { pub fn mock_function_sources() -> Option { let mut function_sources: FunctionSources = HashMap::new(); - function_sources.insert("public.function_source".to_owned(), Box::new(FunctionSource { - id: "public.function_source".to_owned(), - schema: "public".to_owned(), - function: "function_source".to_owned(), - })); + function_sources.insert( + "public.function_source".to_owned(), + Box::new(FunctionSource { + id: "public.function_source".to_owned(), + schema: "public".to_owned(), + function: "function_source".to_owned(), + }), + ); - function_sources.insert("public.function_source_query_params".to_owned(), Box::new(FunctionSource { - id: "public.function_source_query_params".to_owned(), - schema: "public".to_owned(), - function: "function_source_query_params".to_owned(), - })); + function_sources.insert( + "public.function_source_query_params".to_owned(), + Box::new(FunctionSource { + id: "public.function_source_query_params".to_owned(), + schema: "public".to_owned(), + function: "function_source_query_params".to_owned(), + }), + ); Some(function_sources) }