Merge pull request #16 from Jupeyy/pr_path_to_url

Properly encode path like urls.

game/client-containers/src/container.rs

@@ -16,7 +16,7 @@ use anyhow::anyhow;
 use base::hash::{fmt_hash, Hash};
 use base_io_traits::{fs_traits::FileSystemInterface, http_traits::HttpClientInterface};
 
-use base_io::{io::Io, runtime::IoRuntimeTask};
+use base_io::{io::Io, path_to_url::relative_path_to_url, runtime::IoRuntimeTask};
 use either::Either;
 use game_interface::types::resource_key::ResourceKey;
 use graphics::{
@@ -490,12 +490,10 @@ where
     ) {
         // try to download index
         if http_index.is_none() {
-            if let Some(download_url) = base_path.to_str().and_then(|base_path| {
-                resource_http_download_url
-                    .join(base_path)
-                    .and_then(|path| path.join("index.json"))
-                    .ok()
-            }) {
+            if let Some(download_url) = relative_path_to_url(&base_path.join("index.json"))
+                .ok()
+                .and_then(|name| resource_http_download_url.join(&name).ok())
+            {
                 let r = http
                     .download_text(download_url)
                     .await
@@ -626,11 +624,10 @@ where
             // if loading still failed, switch to http download
             if files.is_none() {
                 let name = format!("{}_{}.tar", key.name.as_str(), fmt_hash(&hash));
-                if let Some(game_server_http) = game_server_http
-                    .as_ref()
-                    .zip(base_path.to_str())
-                    .and_then(|(url, base_path)| url.join(base_path).ok()?.join(&name).ok())
-                {
+                if let Some(game_server_http) = game_server_http.as_ref().and_then(|url| {
+                    url.join(relative_path_to_url(&base_path.join(&name)).ok()?.as_str())
+                        .ok()
+                }) {
                     let _g = http_download_tasks.acquire().await?;
                     if let Ok(file) = http.download_binary(game_server_http, &hash).await {
                         if let Ok(tar_files) = read_tar(&file) {
@@ -660,11 +657,10 @@ where
             // Note: for now only try image files, doesn't seem worth it for sound files
             if files.is_none() {
                 let name = format!("{}_{}.png", key.name.as_str(), fmt_hash(&hash));
-                if let Some(game_server_http) = game_server_http
-                    .as_ref()
-                    .zip(base_path.to_str())
-                    .and_then(|(url, base_path)| url.join(base_path).ok()?.join(&name).ok())
-                {
+                if let Some(game_server_http) = game_server_http.as_ref().and_then(|url| {
+                    url.join(&relative_path_to_url(&base_path.join(&name)).ok()?)
+                        .ok()
+                }) {
                     let _g = http_download_tasks.acquire().await?;
                     if let Ok(file) = http.download_binary(game_server_http, &hash).await {
                         if Self::verify_resource("png", &name, &file) {
@@ -777,21 +773,18 @@ where
 
             // else try to load the entry from http (if active)
             if files.is_none() {
-                if let Some((url, name, hash, ty)) = http_entry.zip(base_path.to_str()).and_then(
-                    |((entry, download_url), base_path)| {
-                        let name = format!(
-                            "{}_{}.{}",
-                            key.name.as_str(),
-                            fmt_hash(&entry.hash),
-                            entry.ty
-                        );
-                        download_url
-                            .join(base_path)
-                            .and_then(|url| url.join(&name))
-                            .map(|url| (url, name, entry.hash, entry.ty))
-                            .ok()
-                    },
-                ) {
+                if let Some((url, name, hash, ty)) = http_entry.and_then(|(entry, download_url)| {
+                    let name = format!(
+                        "{}_{}.{}",
+                        key.name.as_str(),
+                        fmt_hash(&entry.hash),
+                        entry.ty
+                    );
+                    download_url
+                        .join(&relative_path_to_url(&base_path.join(&name)).ok()?)
+                        .map(|url| (url, name, entry.hash, entry.ty))
+                        .ok()
+                }) {
                     let res = {
                         let _g = http_download_tasks.acquire().await?;
                         http.download_binary(url, &hash).await

game/client-map/src/client_map.rs

@@ -1,7 +1,7 @@
 use std::{collections::BTreeMap, path::Path, rc::Rc, sync::Arc};
 
 use anyhow::anyhow;
-use base_io::{io::Io, runtime::IoRuntimeTask};
+use base_io::{io::Io, path_to_url::relative_path_to_url, runtime::IoRuntimeTask};
 use client_render_base::map::render_map_base::{ClientMapRender, RenderMapLoading};
 use client_render_game::render_game::{RenderGameCreateOptions, RenderGameInterface, RenderModTy};
 use config::config::ConfigDebug;
@@ -121,8 +121,9 @@ impl ClientMapLoadingFile {
                         // try to download file
                         if let Some(resource_download_server) = resource_download_server_thread
                             .and_then(|url| {
-                                url.join(download_map_file_name.as_os_str().to_str().unwrap_or(""))
+                                relative_path_to_url(&download_map_file_name)
                                     .ok()
+                                    .and_then(|name| url.join(&name).ok())
                             })
                         {
                             let file = http
@@ -189,7 +190,11 @@ impl ClientMapLoadingFile {
                                     // try to download file
                                     if let Some(resource_download_server) =
                                         resource_download_server_thread.and_then(|url| {
-                                            url.join(&download_game_mod_file_name).ok()
+                                            relative_path_to_url(
+                                                download_game_mod_file_name.as_ref(),
+                                            )
+                                            .ok()
+                                            .and_then(|name| url.join(&name).ok())
                                         })
                                     {
                                         let file = http

game/client-render-base/src/map/render_map_base.rs

@@ -18,7 +18,7 @@ use base::{
     hash::{fmt_hash, Hash},
     join_all,
 };
-use base_io::{io::Io, runtime::IoRuntimeTask};
+use base_io::{io::Io, path_to_url::relative_path_to_url, runtime::IoRuntimeTask};
 use config::config::ConfigDebug;
 use graphics::{
     graphics::graphics::Graphics,
@@ -36,12 +36,12 @@ use image::png::{is_png_image_valid, load_png_image};
 use map::map::Map;
 use math::math::vector::vec2;
 use rayon::iter::{IntoParallelIterator, ParallelIterator};
-use vanilla::collision::collision::Collision;
 use sound::{
     commands::SoundSceneCreateProps, ogg_vorbis::verify_ogg_vorbis, scene_handle::SoundSceneHandle,
     sound::SoundManager,
 };
 use url::Url;
+use vanilla::collision::collision::Collision;
 
 pub struct ClientMapFileData {
     pub collision: Collision,
@@ -177,7 +177,11 @@ impl RenderMapLoading {
                                         // try to download file
                                         if let Some(resource_download_server) =
                                             resource_download_server.and_then(|url| {
-                                                url.join(&download_read_file_path).ok()
+                                                relative_path_to_url(
+                                                    download_read_file_path.as_ref(),
+                                                )
+                                                .ok()
+                                                .and_then(|name| url.join(&name).ok())
                                             })
                                         {
                                             let file = http

game/client-ui/Cargo.toml

@@ -44,6 +44,7 @@ anyhow = { version = "1.0.95", features = ["backtrace"] }
 async-trait = "0.1.83"
 email_address = { version = "0.2.9", features = ["serde"] }
 url = { version = "2.5.4", features = ["serde"] }
+urlencoding = "2.1.3"
 gcd = "2.3.0"
 num-rational = "0.4.2"
 num-traits = "0.2.19"

game/client-ui/src/main_menu/page.rs

@@ -22,6 +22,7 @@ use game_base::{
     server_browser::{ServerBrowserData, ServerBrowserInfo, ServerBrowserServer},
 };
 
+use game_base::local_server_info::LocalServerInfo;
 use game_config::config::{Config, ConfigGame};
 use graphics::{
     graphics::graphics::Graphics,
@@ -34,7 +35,6 @@ use graphics::{
     },
 };
 use master_server_types::{addr::Protocol, servers::BrowserServers};
-use game_base::local_server_info::LocalServerInfo;
 use sound::{scene_object::SceneObject, sound::SoundManager};
 use ui_base::types::{UiRenderPipe, UiState};
 use ui_generic::traits::UiPageInterface;
@@ -205,10 +205,13 @@ impl MainMenuUi {
             .spawn(async move {
                 Ok(http
                     .download_text(
-                        format!("https://info.ddnet.org/info?name={}", name)
-                            .as_str()
-                            .try_into()
-                            .unwrap(),
+                        format!(
+                            "https://info.ddnet.org/info?name={}",
+                            urlencoding::encode(&name)
+                        )
+                        .as_str()
+                        .try_into()
+                        .unwrap(),
                     )
                     .await?)
             })

lib/base-http/Cargo.toml

@@ -7,6 +7,7 @@ edition = "2021"
 
 [dependencies]
 base = { path = "../base" }
+base-io = { path = "../base-io" }
 base-io-traits = { path = "../base-io-traits" }
 
 anyhow = { version = "1.0.95", features = ["backtrace"] }

lib/base-http/src/http_server.rs

@@ -4,6 +4,7 @@ use std::{
     time::Duration,
 };
 
+use base_io::path_to_url::relative_path_to_url;
 use tokio::net::TcpSocket;
 use tower_http::services::ServeDir;
 
@@ -44,18 +45,13 @@ impl HttpDownloadServer {
 
                         for (name, served_file) in served_files {
                             let path: &Path = name.as_ref();
-                            if path.is_absolute() {
-                                log::warn!("Found file with unsupported absolute path: {name}");
-                                anyhow::bail!("Cannot serve file with absolute path: {name}");
-                            }
-                            let path: String = path
-                                .components()
-                                .map(|p| {
-                                    urlencoding::encode(&p.as_os_str().to_string_lossy())
-                                        .to_string()
-                                })
-                                .collect::<Vec<_>>()
-                                .join("/");
+                            let path = match relative_path_to_url(path) {
+                                Ok(path) => path,
+                                Err(err) => {
+                                    log::warn!("Found file with unsupported absolute path: {name}");
+                                    anyhow::bail!(err);
+                                }
+                            };
                             app = app.route(
                                 &format!("/{}", path),
                                 axum::routing::get(|| async move { served_file }),

lib/base-io/Cargo.toml

@@ -6,7 +6,9 @@ edition = "2021"
 [dependencies]
 base-io-traits = { path = "../base-io-traits" }
 hiarc = { path = "../hiarc", features = ["enable_anyhow", "enable_tokio"] }
+
 anyhow = { version = "1.0.95", features = ["backtrace"] }
+urlencoding = "2.1.3"
 
 [target.'cfg(target_arch = "wasm32")'.dependencies]
 tokio = { version = "1.42.0", features = ["sync", "macros"] }

lib/base-io/src/lib.rs

@@ -1,3 +1,4 @@
 pub mod io;
+pub mod path_to_url;
 pub mod runtime;
 pub mod yield_now;

lib/base-io/src/path_to_url.rs

@@ -0,0 +1,20 @@
+use std::path::Path;
+
+use anyhow::anyhow;
+
+/// Converts path components to a properly url encoded path string.
+pub fn relative_path_to_url(path: &Path) -> anyhow::Result<String> {
+    if path.is_absolute() {
+        anyhow::bail!("Only relative paths are supported, but absolute path was found: {path:?}");
+    }
+    Ok(path
+        .components()
+        .map(|p| {
+            p.as_os_str()
+                .to_str()
+                .map(|p| urlencoding::encode(p).to_string())
+        })
+        .collect::<Option<Vec<_>>>()
+        .ok_or_else(|| anyhow!("One or more components were not valid strings: {path:?}"))?
+        .join("/"))
+}