From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <pbs-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9])
	by lore.proxmox.com (Postfix) with ESMTPS id A669A1FF16E
	for <inbox@lore.proxmox.com>; Mon, 14 Apr 2025 16:22:22 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
	by firstgate.proxmox.com (Proxmox) with ESMTP id 5D3CE3550E;
	Mon, 14 Apr 2025 16:22:21 +0200 (CEST)
From: Maximiliano Sandoval <m.sandoval@proxmox.com>
To: pbs-devel@lists.proxmox.com
Date: Mon, 14 Apr 2025 16:22:17 +0200
Message-Id: <20250414142217.575289-1-m.sandoval@proxmox.com>
X-Mailer: git-send-email 2.39.5
MIME-Version: 1.0
X-SPAM-LEVEL: Spam detection results:  0
 AWL 0.094 Adjusted score from AWL reputation of From: address
 BAYES_00                 -1.9 Bayes spam probability is 0 to 1%
 DMARC_MISSING             0.1 Missing DMARC policy
 KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
 KAM_SHORT               0.001 Use of a URL Shortener for very short URL
 RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
 Validity was blocked. See
 https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
 information.
 RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
 Validity was blocked. See
 https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
 information.
 RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
 Validity was blocked. See
 https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
 information.
 SPF_HELO_NONE           0.001 SPF: HELO does not publish an SPF Record
 SPF_PASS               -0.001 SPF: sender matches SPF record
Subject: [pbs-devel] [PATCH backup v2] http_client: fallback if
 XDG_RUNTIME_DIR is not set
X-BeenThere: pbs-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox Backup Server development discussion
 <pbs-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pbs-devel>, 
 <mailto:pbs-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pbs-devel/>
List-Post: <mailto:pbs-devel@lists.proxmox.com>
List-Help: <mailto:pbs-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel>, 
 <mailto:pbs-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox Backup Server development discussion
 <pbs-devel@lists.proxmox.com>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: pbs-devel-bounces@lists.proxmox.com
Sender: "pbs-devel" <pbs-devel-bounces@lists.proxmox.com>

xdg::BaseDirectory's [place_directory_file] errors out if
`XDG_RUNTIME_DIR` is not set.

This is not ideal, as per the the base directory [specification]:

> If $XDG_RUNTIME_DIR is not set applications should fall back to a
replacement directory with similar capabilities and print a warning
message. Applications should use this directory for communication and
synchronization purposes and should not place larger files in it, since
it might reside in runtime memory and cannot necessarily be swapped out
to disk.

At the moment, running the proxmox-backup-client with sudo will print an error:
```
storing login ticket failed: $XDG_RUNTIME_DIR must be set
```

We add a helper that places a runtime file `basename` which fallbacks to
`/run/user/{uid}/{prefix}/{basename}`.

[place_directory_file] https://docs.rs/xdg/latest/xdg/struct.BaseDirectories.html#method.place_runtime_file
[specification]: https://specifications.freedesktop.org/basedir-spec/latest/

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
Differences from v1:
 - Actually use `prefix` argument in fallback branch
 - Stop checking for root, always use /run/user/{uid}/{prefix}

 pbs-client/src/http_client.rs | 50 +++++++++++++++++++++++++----------
 1 file changed, 36 insertions(+), 14 deletions(-)

diff --git a/pbs-client/src/http_client.rs b/pbs-client/src/http_client.rs
index c95def07b..9b1a9595c 100644
--- a/pbs-client/src/http_client.rs
+++ b/pbs-client/src/http_client.rs
@@ -1,8 +1,9 @@
 use std::io::{IsTerminal, Write};
+use std::path::PathBuf;
 use std::sync::{Arc, Mutex, RwLock};
 use std::time::Duration;
 
-use anyhow::{bail, format_err, Error};
+use anyhow::{bail, format_err, Context, Error};
 use futures::*;
 #[cfg(not(target_feature = "crt-static"))]
 use hyper::client::connect::dns::GaiResolver;
@@ -27,7 +28,7 @@ use proxmox_async::broadcast_future::BroadcastFuture;
 use proxmox_http::client::HttpsConnector;
 use proxmox_http::uri::{build_authority, json_object_to_query};
 use proxmox_http::{ProxyConfig, RateLimiter};
-use proxmox_log::{error, info, warn};
+use proxmox_log::{debug, error, info, warn};
 
 use pbs_api_types::percent_encoding::DEFAULT_ENCODE_SET;
 use pbs_api_types::{Authid, RateLimitConfig, Userid};
@@ -216,10 +217,7 @@ pub struct HttpClient {
 
 /// Delete stored ticket data (logout)
 pub fn delete_ticket_info(prefix: &str, server: &str, username: &Userid) -> Result<(), Error> {
-    let base = BaseDirectories::with_prefix(prefix)?;
-
-    // usually /run/user/<uid>/...
-    let path = base.place_runtime_file("tickets")?;
+    let path = place_runtime_file(prefix, "tickets")?;
 
     let mode = nix::sys::stat::Mode::from_bits_truncate(0o0600);
 
@@ -305,10 +303,7 @@ fn store_ticket_info(
     ticket: &str,
     token: &str,
 ) -> Result<(), Error> {
-    let base = BaseDirectories::with_prefix(prefix)?;
-
-    // usually /run/user/<uid>/...
-    let path = base.place_runtime_file("tickets")?;
+    let path = place_runtime_file(prefix, "tickets")?;
 
     let mode = nix::sys::stat::Mode::from_bits_truncate(0o0600);
 
@@ -345,10 +340,9 @@ fn store_ticket_info(
 }
 
 fn load_ticket_info(prefix: &str, server: &str, userid: &Userid) -> Option<(String, String)> {
-    let base = BaseDirectories::with_prefix(prefix).ok()?;
-
-    // usually /run/user/<uid>/...
-    let path = base.place_runtime_file("tickets").ok()?;
+    let path = place_runtime_file(prefix, "tickets")
+        .inspect_err(|err| error!("could not place runtime file: {err:#}"))
+        .ok()?;
     let data = file_get_json(path, None).ok()?;
     let now = proxmox_time::epoch_i64();
     let ticket_lifetime = proxmox_auth_api::TICKET_LIFETIME - 60;
@@ -1181,3 +1175,31 @@ impl H2Client {
         Ok(request)
     }
 }
+
+// Returns an absolute path in `XDG_RUNTIME_DIR` where a runtime file may be
+// stored. Leading directories in the returned path are pre-created; if that is
+// not possible, an error is returned.
+//
+// Similar to [BaseDirectories::place_runtime_file] but will fall back to
+// `/run/user/{uid}/{prefix}` if the `XDG_RUNTIME_DIR` variable is not set.
+fn place_runtime_file(prefix: &str, basename: &str) -> Result<PathBuf, Error> {
+    let base =
+        BaseDirectories::with_prefix(prefix).with_context(|| "failed to get base directories")?;
+
+    let path = if base.has_runtime_directory() {
+        base.place_runtime_file(basename)
+            .with_context(|| format!("failed to place runtime file {basename}"))?
+    } else {
+        let uid = nix::unistd::Uid::current();
+        let path = PathBuf::from(format!("/run/user/{uid}/{prefix}/"));
+        std::fs::create_dir_all(&path)?;
+        debug!(
+            "XDG_RUNTIME_DIR is not set, using {} as fallback",
+            path.display()
+        );
+        path.join(basename)
+    };
+    debug!("placing {basename} at {}", path.display());
+
+    Ok(path)
+}
-- 
2.39.5



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel