fix special characters in site_prefix

fix #689

Author: lovasoa

CHANGELOG.md

@@ -31,6 +31,7 @@
   - Adds support for `ANY`, `ALL`, and `SOME` subqueries, like `SELECT * FROM t WHERE a = ANY (SELECT b FROM t2)` 
 - Add support for `change_percent` without `description` in the big_number component to display the percentage change of a value.
 - Add support for `freeze_columns` and `freeze_headers` in the table component to freeze columns and headers.
+- Fix an error that occured when the site_prefix configuration option was set to a value containing special characters like `-` and a request was made directly to the server without url-encoding the site prefix.
 
 ## 0.30.1 (2024-10-31)
 - fix a bug where table sorting would break if table search was not also enabled.

src/app_config.rs

@@ -348,16 +348,24 @@ fn deserialize_site_prefix<'de, D: Deserializer<'de>>(deserializer: D) -> Result
 /// We also percent-encode special characters in the prefix, but allow it to contain slashes (to allow
 /// hosting on a sub-sub-path).
 fn normalize_site_prefix(prefix: &str) -> String {
-    const TO_ENCODE: AsciiSet = percent_encoding::NON_ALPHANUMERIC.remove(b'/');
+    const TO_ENCODE: AsciiSet = percent_encoding::CONTROLS
+        .add(b' ')
+        .add(b'"')
+        .add(b'#')
+        .add(b'<')
+        .add(b'>')
+        .add(b'?');
 
     let prefix = prefix.trim_start_matches('/').trim_end_matches('/');
     if prefix.is_empty() {
         return default_site_prefix();
     }
     let encoded_prefix = percent_encoding::percent_encode(prefix.as_bytes(), &TO_ENCODE);
 
+    let invalid_chars = ["%09", "%0A", "%0D"];
+
     std::iter::once("/")
-        .chain(encoded_prefix)
+        .chain(encoded_prefix.filter(|c| !invalid_chars.contains(c)))
         .chain(std::iter::once("/"))
         .collect::<String>()
 }
@@ -550,6 +558,13 @@ mod test {
         assert_eq!(normalize_site_prefix("a/b/c"), "/a/b/c/");
         assert_eq!(normalize_site_prefix("a b"), "/a%20b/");
         assert_eq!(normalize_site_prefix("a b/c"), "/a%20b/c/");
+        assert_eq!(normalize_site_prefix("*-+/:;,?%\"'{"), "/*-+/:;,%3F%%22'{/");
+        assert_eq!(
+            normalize_site_prefix(
+                &(0..=0x7F).map(|b| char::from(b)).collect::<String>()
+            ),
+            "/%00%01%02%03%04%05%06%07%08%0B%0C%0E%0F%10%11%12%13%14%15%16%17%18%19%1A%1B%1C%1D%1E%1F%20!%22%23$%&'()*+,-./0123456789:;%3C=%3E%3F@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_`abcdefghijklmnopqrstuvwxyz{|}~%7F/"
+        );
     }
 
     #[test]

src/webserver/http.rs

@@ -314,7 +314,7 @@ async fn serve_file(
     state: &AppState,
     if_modified_since: Option<IfModifiedSince>,
 ) -> actix_web::Result<HttpResponse> {
-    let path = path.strip_prefix(&state.config.site_prefix).unwrap_or(path);
+    let path = strip_site_prefix(path, state);
     if let Some(IfModifiedSince(date)) = if_modified_since {
         let since = DateTime::<Utc>::from(SystemTime::from(date));
         let modified = state
@@ -345,6 +345,11 @@ async fn serve_file(
         })
 }
 
+/// Strips the site prefix from a path
+fn strip_site_prefix<'a>(path: &'a str, state: &AppState) -> &'a str {
+    path.strip_prefix(&state.config.site_prefix).unwrap_or(path)
+}
+
 /// Fallback handler for when a file could not be served
 ///
 /// Recursively traverses upwards in the request's path, looking for a `404.sql` to call as the
@@ -361,7 +366,7 @@ async fn serve_fallback(
 
     let app_state: &web::Data<AppState> = service_request.app_data().expect("app_state");
 
-    // Find the indeces of each char which follows a directroy separator (`/`). Also consider the 0
+    // Find the indices of each char which follows a directroy separator (`/`). Also consider the 0
     // index, as we also have to try to check the empty path, for a root dir `404.sql`.
     for idx in req_path
         .rmatch_indices('/')
@@ -408,6 +413,10 @@ async fn serve_fallback(
 pub async fn main_handler(
     mut service_request: ServiceRequest,
 ) -> actix_web::Result<ServiceResponse> {
+    if let Some(redirect) = redirect_missing_prefix(&service_request) {
+        return Ok(service_request.into_response(redirect));
+    }
+
     let path = req_path(&service_request);
     let sql_file_path = path_to_sql_file(&path);
     let maybe_response = if let Some(sql_path) = sql_file_path {
@@ -419,11 +428,10 @@ pub async fn main_handler(
         }
     } else {
         log::debug!("Serving file: {:?}", path);
-        let app_state = service_request.extract::<web::Data<AppState>>().await?;
         let path = req_path(&service_request);
         let if_modified_since = IfModifiedSince::parse(&service_request).ok();
-
-        serve_file(&path, &app_state, if_modified_since).await
+        let app_state: &web::Data<AppState> = service_request.app_data().expect("app_state");
+        serve_file(&path, app_state, if_modified_since).await
     };
 
     // On 404/NOT_FOUND error, fall back to `404.sql` handler if it exists
@@ -442,13 +450,27 @@ pub async fn main_handler(
     Ok(service_request.into_response(response))
 }
 
+fn redirect_missing_prefix(service_request: &ServiceRequest) -> Option<HttpResponse> {
+    let app_state: &web::Data<AppState> = service_request.app_data().expect("app_state");
+    if !service_request
+        .path()
+        .starts_with(&app_state.config.site_prefix)
+    {
+        let header = (header::LOCATION, app_state.config.site_prefix.clone());
+        return Some(
+            HttpResponse::PermanentRedirect()
+                .insert_header(header)
+                .finish(),
+        );
+    }
+    None
+}
+
 /// Extracts the path from a request and percent-decodes it
 fn req_path(req: &ServiceRequest) -> Cow<'_, str> {
     let encoded_path = req.path();
     let app_state: &web::Data<AppState> = req.app_data().expect("app_state");
-    let encoded_path = encoded_path
-        .strip_prefix(&app_state.config.site_prefix)
-        .unwrap_or(encoded_path);
+    let encoded_path = strip_site_prefix(encoded_path, app_state);
     percent_encoding::percent_decode_str(encoded_path).decode_utf8_lossy()
 }
 
@@ -481,12 +503,12 @@ async fn default_prefix_redirect(
     service_request: ServiceRequest,
 ) -> actix_web::Result<ServiceResponse> {
     let app_state: &web::Data<AppState> = service_request.app_data().expect("app_state");
-    let redirect_path = app_state
-        .config
-        .site_prefix
-        .trim_end_matches('/')
-        .to_string()
-        + service_request.path();
+    let original_path = service_request.path();
+    let site_prefix = &app_state.config.site_prefix;
+    let redirect_path = site_prefix.trim_end_matches('/').to_string() + original_path;
+    log::info!(
+        "Received request to {original_path} (outside of site prefix {site_prefix}), redirecting to {redirect_path}"
+    );
     Ok(service_request.into_response(
         HttpResponse::PermanentRedirect()
             .insert_header((header::LOCATION, redirect_path))