URL refactor

Author: joepio

lib/src/agents.rs

@@ -73,7 +73,11 @@ impl Agent {
         Ok(Agent {
             private_key: None,
             public_key: public_key.into(),
-            subject: format!("{}/agents/{}", store.get_server_url(), public_key),
+            subject: store
+                .get_server_url()
+                .clone()
+                .set_path(&format!("agents/{}", public_key))
+                .to_string(),
             name: None,
             created_at: crate::utils::now(),
         })

lib/src/atomic_url.rs

@@ -111,6 +111,7 @@ impl AtomicUrl {
         Ok(self)
     }
 
+    /// Removes existing path, sets the new one. Escapes special characters
     pub fn set_path(&mut self, path: &str) -> &Self {
         self.url.set_path(path);
         self

lib/src/plugins/bookmark.rs

@@ -30,7 +30,7 @@ pub fn bookmark_endpoint() -> Endpoint {
 fn handle_bookmark_request(
     url: url::Url,
     store: &impl Storelike,
-    _: Option<&str>,
+    _agent: Option<&str>,
 ) -> AtomicResult<Resource> {
     let params = url.query_pairs();
     let mut path = None;

lib/src/resources.rs

@@ -189,7 +189,7 @@ impl Resource {
 
     /// Create a new resource with a generated Subject
     pub fn new_generate_subject(store: &impl Storelike) -> Resource {
-        let generated = format!("{}/{}", store.get_server_url(), random_string(10));
+        let generated = format!("{}{}", store.get_server_url(), random_string(10));
 
         Resource::new(generated)
     }

server/src/handlers/download.rs

@@ -12,13 +12,16 @@ pub async fn handle_download(
     req: actix_web::HttpRequest,
 ) -> AtomicServerResult<HttpResponse> {
     let headers = req.headers();
-    let server_url = &appstate.config.server_url;
     let store = &appstate.store;
 
     // We replace `/download` with `/` to get the subject of the Resource.
     let subject = if let Some(pth) = path {
-        let subject = format!("{}/{}", server_url, pth);
-        subject
+        appstate
+            .store
+            .get_server_url()
+            .clone()
+            .set_path(pth.as_str())
+            .to_string()
     } else {
         // There is no end string, so It's the root of the URL, the base URL!
         return Err("Put `/download` in front of an File URL to download it.".into());

server/src/handlers/resource.rs

@@ -1,6 +1,10 @@
 use crate::{
-    appstate::AppState, content_types::get_accept, content_types::ContentType,
-    errors::AtomicServerResult, helpers::get_client_agent, timer::Timer,
+    appstate::AppState,
+    content_types::get_accept,
+    content_types::ContentType,
+    errors::AtomicServerResult,
+    helpers::{get_client_agent, get_subject},
+    timer::Timer,
 };
 use actix_web::{web, HttpResponse};
 use atomic_lib::Storelike;
@@ -14,42 +18,19 @@ pub async fn handle_get_resource(
     req: actix_web::HttpRequest,
     conn: actix_web::dev::ConnectionInfo,
 ) -> AtomicServerResult<HttpResponse> {
+    let store = &appstate.store;
     let mut timer = Timer::new();
-
-    let domain = &appstate.config.opts.domain;
-    let host = conn.host();
-    let find = host.find(domain);
-    let subdomain = if let Some(index) = find {
-        if index == 0 {
-            None
-        } else {
-            Some(host[0..index - 1].to_string())
-        }
-    } else {
-        panic!("Wrong domain! A requested URL did not contain the host for this domain. This should not be able to happen.");
-    };
     let headers = req.headers();
     let content_type = get_accept(headers);
-
-    // You'd think there would be a simpler way of getting the requested URL...
-    // See https://github.com/actix/actix-web/issues/2895
-    let mut subject = appstate.store.get_server_url().clone();
-
-    // Doe this include the query params?
-    subject.set_path(&req.uri().to_string());
-
-    if let Some(sd) = subdomain {
-        subject.set_subdomain(Some(&sd))?;
-    }
-
-    let store = &appstate.store;
     timer.add("parse_headers");
 
+    let subject = get_subject(&req, &conn, &appstate)?;
+    timer.add("parse_subject");
+
     let for_agent = get_client_agent(headers, &appstate, subject.to_string())?;
     timer.add("get_agent");
 
     let mut builder = HttpResponse::Ok();
-
     tracing::debug!("get_resource: {} as {}", subject, content_type.to_mime());
     builder.append_header(("Content-Type", content_type.to_mime()));
     // This prevents the browser from displaying the JSON response upon re-opening a closed tab
@@ -60,8 +41,9 @@ pub async fn handle_get_resource(
     ));
     // When users uses back button, don't show the JSON response
     builder.append_header(("Vary", "Accept"));
+    timer.add("set_headers");
 
-    let resource = store.get_resource_extended(subject.as_str(), false, for_agent.as_deref())?;
+    let resource = store.get_resource_extended(&subject, false, for_agent.as_deref())?;
     timer.add("get_resource");
 
     let response_body = match content_type {

server/src/handlers/upload.rs

@@ -9,7 +9,11 @@ use atomic_lib::{
 use futures::{StreamExt, TryStreamExt};
 use serde::Deserialize;
 
-use crate::{appstate::AppState, errors::AtomicServerResult, helpers::get_client_agent};
+use crate::{
+    appstate::AppState,
+    errors::AtomicServerResult,
+    helpers::{get_client_agent, get_subject},
+};
 
 #[derive(Deserialize, Debug)]
 pub struct UploadQuery {
@@ -28,17 +32,11 @@ pub async fn upload_handler(
     appstate: web::Data<AppState>,
     query: web::Query<UploadQuery>,
     req: actix_web::HttpRequest,
+    conn: actix_web::dev::ConnectionInfo,
 ) -> AtomicServerResult<HttpResponse> {
     let store = &appstate.store;
     let parent = store.get_resource(&query.parent)?;
-    let subject = format!(
-        "{}{}",
-        store.get_server_url(),
-        req.head()
-            .uri
-            .path_and_query()
-            .ok_or("Path must be given")?
-    );
+    let subject = get_subject(&req, &conn, &appstate)?;
     if let Some(agent) = get_client_agent(req.headers(), &appstate, subject)? {
         check_write(store, &parent, &agent)?;
     } else {
@@ -83,8 +81,16 @@ pub async fn upload_handler(
             .map_err(|_e| "Too large")?;
 
         let subject_path = format!("files/{}", urlencoding::encode(&file_id));
-        let new_subject = format!("{}/{}", store.get_server_url(), subject_path);
-        let download_url = format!("{}/download/{}", store.get_server_url(), subject_path);
+        let new_subject = store
+            .get_server_url()
+            .clone()
+            .set_path(&subject_path)
+            .to_string();
+        let download_url = store
+            .get_server_url()
+            .clone()
+            .set_path(&format!("download/{}", subject_path))
+            .to_string();
 
         let mut resource = atomic_lib::Resource::new_instance(urls::FILE, store)?;
         resource.set_subject(new_subject);

server/src/helpers.rs

@@ -1,7 +1,7 @@
 //! Functions useful in the server
 
 use actix_web::http::header::HeaderMap;
-use atomic_lib::authentication::AuthValues;
+use atomic_lib::{authentication::AuthValues, Storelike};
 
 use crate::{appstate::AppState, errors::AtomicServerResult};
 
@@ -77,3 +77,32 @@ pub fn get_client_agent(
     .map_err(|e| format!("Authentication failed: {}", e))?;
     Ok(Some(for_agent))
 }
+
+/// Extracts the full URL from the request, connection and the store.
+// You'd think there would be a simpler way of getting the requested URL...
+// See https://github.com/actix/actix-web/issues/2895
+pub fn get_subject(
+    req: &actix_web::HttpRequest,
+    conn: &actix_web::dev::ConnectionInfo,
+    appstate: &AppState,
+) -> AtomicServerResult<String> {
+    let domain = &appstate.config.opts.domain;
+    let host = conn.host();
+    let subdomain = if let Some(index) = host.find(domain) {
+        if index == 0 {
+            None
+        } else {
+            Some(host[0..index - 1].to_string())
+        }
+    } else {
+        panic!("Wrong domain! A requested URL did not contain the host for this domain. This should not be able to happen.");
+    };
+
+    let mut subject_url = appstate.store.get_server_url().clone();
+    if let Some(sd) = subdomain {
+        subject_url.set_subdomain(Some(&sd))?;
+    }
+    let server_without_last_slash = subject_url.to_string().trim_end_matches('/').to_string();
+    let subject = format!("{}{}", server_without_last_slash, &req.uri().to_string());
+    Ok(subject)
+}