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 4907E1FF172
for <inbox@lore.proxmox.com>; Wed, 16 Apr 2025 14:58:22 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id 8DF6B3740B;
Wed, 16 Apr 2025 14:58:20 +0200 (CEST)
References: <20250414142217.575289-1-m.sandoval@proxmox.com>
<ipthgdv4oarlhtkmojxqqayqjavcxya3644wbluxtje5gbvorg@nta7xbr5gnby>
User-agent: mu4e 1.10.8; emacs 30.1
From: Maximiliano Sandoval <m.sandoval@proxmox.com>
To: Wolfgang Bumiller <w.bumiller@proxmox.com>
Date: Wed, 16 Apr 2025 14:57:13 +0200
In-reply-to: <ipthgdv4oarlhtkmojxqqayqjavcxya3644wbluxtje5gbvorg@nta7xbr5gnby>
Message-ID: <s8oy0w0fely.fsf@proxmox.com>
MIME-Version: 1.0
X-SPAM-LEVEL: Spam detection results: 0
AWL 0.096 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
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] superseded: Re: [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>
Wolfgang Bumiller <w.bumiller@proxmox.com> writes:
> 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
superseded by
https://lore.proxmox.com/pbs-devel/20250416125651.334868-1-m.sandoval@proxmox.com/T/#t.
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel