Restrict IDs, optimize single-source req (#522)

* Ensure the source ID only contains ascii alphanumerics, dots, dashes,
and underscores.
* optimize the most common case of getting a single source
* optimize the case of sources with no query parameters
This commit is contained in:
Yuri Astrakhan 2022-12-15 09:14:53 -05:00 committed by GitHub
parent 9efa364eb0
commit ab2a2f9eed
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 90 additions and 36 deletions

View File

@ -48,12 +48,16 @@ impl Source for PgSource {
is_valid_zoom(zoom, self.tilejson.minzoom, self.tilejson.maxzoom)
}
fn support_url_query(&self) -> bool {
self.info.use_url_query
}
async fn get_tile(&self, xyz: &Xyz, url_query: &Option<UrlQuery>) -> Result<Tile> {
let empty_query = HashMap::new();
let url_query = url_query.as_ref().unwrap_or(&empty_query);
let conn = self.pool.get().await?;
let param_types: &[Type] = if self.info.has_query_params {
let param_types: &[Type] = if self.support_url_query() {
&[Type::INT4, Type::INT4, Type::INT4, Type::JSON]
} else {
&[Type::INT4, Type::INT4, Type::INT4]
@ -69,7 +73,7 @@ impl Source for PgSource {
)
})?;
let tile = if self.info.has_query_params {
let tile = if self.support_url_query() {
let json = query_to_json(url_query);
debug!("SQL: {query} [{xyz}, {json:?}]");
let params: &[&(dyn ToSql + Sync)] = &[&xyz.z, &xyz.x, &xyz.y, &json];
@ -82,7 +86,7 @@ impl Source for PgSource {
let tile = tile
.map(|row| row.map_or(Default::default(), |r| r.get::<_, Option<Tile>>(0)))
.map_err(|e| {
if self.info.has_query_params {
if self.support_url_query() {
GetTileWithQueryError(e, self.id.to_string(), *xyz, url_query.clone())
} else {
GetTileError(e, self.id.to_string(), *xyz)
@ -97,7 +101,7 @@ impl Source for PgSource {
#[derive(Clone, Debug)]
pub struct PgSqlInfo {
pub query: String,
pub has_query_params: bool,
pub use_url_query: bool,
pub signature: String,
}
@ -105,7 +109,7 @@ impl PgSqlInfo {
pub fn new(query: String, has_query_params: bool, signature: String) -> Self {
Self {
query,
has_query_params,
use_url_query: has_query_params,
signature,
}
}

View File

@ -38,6 +38,8 @@ pub trait Source: Send + Debug {
fn is_valid_zoom(&self, zoom: i32) -> bool;
fn support_url_query(&self) -> bool;
async fn get_tile(&self, xyz: &Xyz, query: &Option<UrlQuery>) -> Result<Tile>;
}
@ -63,8 +65,17 @@ impl IdResolver {
}
}
/// if name already exists in the self.names structure, but try it with ".1", ".2", etc. until the value matches
/// If source name already exists in the self.names structure,
/// try appending it with ".1", ".2", etc. until the name is unique.
/// Only alphanumeric characters plus dashes/dots/underscores are allowed.
pub fn resolve(&self, mut name: String, unique_name: String) -> String {
// Ensure name has no prohibited characters like spaces, commas, slashes, or non-unicode etc.
// Underscores, dashes, and dots are OK. All other characters will be replaced with dashes.
name = name.replace(
|c: char| !c.is_ascii_alphanumeric() && c != '_' && c != '.' && c != '-',
"-",
);
let mut names = self.names.lock().expect("IdResolver panicked");
if !self.reserved.contains(name.as_str()) {
match names.entry(name) {
@ -119,6 +130,9 @@ mod tests {
assert_eq!(r.resolve("b".to_string(), "a".to_string()), "b");
assert_eq!(r.resolve("a.1".to_string(), "a".to_string()), "a.1.1");
assert_eq!(r.resolve("a.1".to_string(), "b".to_string()), "a.1");
assert_eq!(r.resolve("a b".to_string(), "a b".to_string()), "a-b");
assert_eq!(r.resolve("a b".to_string(), "ab2".to_string()), "a-b.1");
}
#[test]

View File

@ -1,5 +1,5 @@
use crate::pg::utils::parse_x_rewrite_url;
use crate::source::{Source, Xyz};
use crate::source::{Source, UrlQuery, Xyz};
use crate::srv::config::SrvConfig;
use actix_cors::Cors;
use actix_web::dev::Server;
@ -46,23 +46,17 @@ impl AppState {
&self,
source_ids: &str,
zoom: Option<i32>,
) -> Result<(Vec<&dyn Source>, DataFormat)> {
) -> Result<(Vec<&dyn Source>, bool, DataFormat)> {
// TODO?: optimize by pre-allocating max allowed layer count on stack
let mut sources = Vec::new();
let mut format: Option<DataFormat> = None;
let mut use_url_query = false;
for id in source_ids.split(',') {
let src = self
.sources
.get(id)
.ok_or_else(|| error::ErrorNotFound(format!("Source {id} does not exist")))?
.as_ref();
if let Some(z) = zoom {
if !src.is_valid_zoom(z) {
debug!("Zoom {z} is not valid for source {id}");
continue;
}
}
let src = self.get_source(id)?;
let src_fmt = src.get_format();
use_url_query |= src.support_url_query();
// make sure all sources have the same format
match format {
Some(fmt) if fmt == src_fmt => {}
Some(fmt) => Err(error::ErrorNotFound(format!(
@ -70,10 +64,18 @@ impl AppState {
)))?,
None => format = Some(src_fmt),
}
sources.push(src);
// TODO: Use chained-if-let once available
if match zoom {
Some(zoom) if check_zoom(src, id, zoom) => true,
None => true,
_ => false,
} {
sources.push(src);
}
}
let format = format.ok_or_else(|| error::ErrorNotFound("No valid sources found"))?;
Ok((sources, format))
let format = format.unwrap_or(DataFormat::Unknown);
Ok((sources, use_url_query, format))
}
}
@ -236,31 +238,57 @@ fn merge_tilejson(sources: Vec<&dyn Source>, tiles_url: String) -> TileJSON {
#[route("/{source_ids}/{z}/{x}/{y}", method = "GET", method = "HEAD")]
async fn get_tile(
req: HttpRequest,
path: Path<TileRequest>,
query: Query<HashMap<String, String>>,
state: Data<AppState>,
) -> Result<HttpResponse> {
let (sources, format) = state.get_sources(&path.source_ids, Some(path.z))?;
let query = Some(query.into_inner());
let xyz = Xyz {
z: path.z,
x: path.x,
y: path.y,
};
let tile = try_join_all(sources.into_iter().map(|s| s.get_tile(&xyz, &query)))
.await
.map_err(map_internal_error)?
.concat();
// Optimization for a single-source request.
let (tile, format) = if path.source_ids.contains(',') {
let (sources, use_url_query, format) = state.get_sources(&path.source_ids, Some(path.z))?;
if sources.is_empty() {
Err(error::ErrorNotFound("No valid sources found"))?
}
let query = if use_url_query {
Some(Query::<UrlQuery>::from_query(req.query_string())?.into_inner())
} else {
None
};
let tile = try_join_all(sources.into_iter().map(|s| s.get_tile(&xyz, &query)))
.await
.map_err(map_internal_error)?
.concat();
(tile, format)
} else {
let id = &path.source_ids;
let zoom = xyz.z;
let src = state.get_source(id)?;
if !check_zoom(src, id, zoom) {
Err(error::ErrorNotFound(format!(
"Zoom {zoom} is not valid for source {id}",
)))?
}
let query = if src.support_url_query() {
Some(Query::<UrlQuery>::from_query(req.query_string())?.into_inner())
} else {
None
};
let tile = src
.get_tile(&xyz, &query)
.await
.map_err(map_internal_error)?;
(tile, src.get_format())
};
let content = format.content_type();
match tile.len() {
0 => Ok(HttpResponse::NoContent()
.content_type(format.content_type())
.finish()),
_ => Ok(HttpResponse::Ok()
.content_type(format.content_type())
.body(tile)),
0 => Ok(HttpResponse::NoContent().content_type(content).finish()),
_ => Ok(HttpResponse::Ok().content_type(content).body(tile)),
}
}
@ -301,3 +329,11 @@ pub fn new(config: SrvConfig, sources: Sources) -> Server {
.workers(worker_processes)
.run()
}
pub fn check_zoom(src: &dyn Source, id: &str, zoom: i32) -> bool {
let is_valid = src.is_valid_zoom(zoom);
if !is_valid {
debug!("Zoom {zoom} is not valid for source {id}");
}
is_valid
}