[pbs-devel] [PATCH backup v2] http_client: fallback if XDG_RUNTIME_DIR is not set

[Wolfgang Bumiller]

On Mon, Apr 14, 2025 at 04:22:17PM +0200, Maximiliano Sandoval wrote:
> xdg::BaseDirectory's [place_directory_file] errors out if

Should be `place_runtime_file`, since other directories will fall back
to the regular $HOME paths.

For the runtime dir this makes sense. A user who is not logged into a
seat does not *have* one.

> `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.

^ This is not useful information for why this patch is doing what it
does.

> 
> 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}`.

A runtime directory is created when the user logs into a seat, so a
logged in user always has one, so if not, is the `/run/user/$uid`
directory really the right choice? Are there any other tools that do
this?

For this case I'd argue that we should instead switch from the runtime
directory to the cache directory. This should also work via sudo and
makes more sense. The ticket can still be used if the machine reboots,
after all.
> 
> [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 at 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