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 [212.224.123.68])
by lore.proxmox.com (Postfix) with ESMTPS id 1E03C1FF172
for <inbox@lore.proxmox.com>; Tue, 15 Apr 2025 08:14:21 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id 02E8C3EDE7;
Tue, 15 Apr 2025 08:14:20 +0200 (CEST)
Date: Tue, 15 Apr 2025 08:14:15 +0200
From: Wolfgang Bumiller <w.bumiller@proxmox.com>
To: Maximiliano Sandoval <m.sandoval@proxmox.com>
Message-ID: <ipthgdv4oarlhtkmojxqqayqjavcxya3644wbluxtje5gbvorg@nta7xbr5gnby>
References: <20250414142217.575289-1-m.sandoval@proxmox.com>
MIME-Version: 1.0
Content-Disposition: inline
In-Reply-To: <20250414142217.575289-1-m.sandoval@proxmox.com>
X-SPAM-LEVEL: Spam detection results: 0
AWL 0.080 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
URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See
http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more
information. [docs.rs, freedesktop.org]
Subject: Re: [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>
Cc: 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>
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@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