mirror of
https://github.com/rustdesk/rustdesk.git
synced 2026-09-18 10:21:03 +03:00
fix(linux): the newest-process walk must not answer with a grep or an older PID
Three findings from review of the commit before this one. `/proc/<pid>/environ` failing to read left the walk on to the next PID, which in `newest_first` mode is an older process -- possibly of a session that has since logged out -- where the `ps ... | tail -1` pipeline this replaces stopped at the one PID it had already picked. A read that fails is a process carrying none of the requested names, not a process to skip. The `seen` latch that was meant to hold the newest process is deleted: `accept` is reached once per matching process, so returning on the first is what it already did. The regex is matched against the whole `/proc/<pid>/cmdline`, where the pipeline had a `grep -v 'grep'`. A user running `grep Xwayland` is otherwise the newest match for that pattern and answers with whatever environment their shell had -- an X forwarding endpoint over ssh, say. This is the one place the walk still differs from the `get_envs` it grew out of, which never had that filter and could take an ssh `grep` over the portal it was looking for. `get_envs` is left exactly as it was. Its completeness test was every requested name *present*; stating it through `accept` turned it into every name *non-empty* and, with the empty-value change that went with it, moved which process the existing `get_display_xauth_wayland` caller settles on. `accept` is now told the count and asks the question the loop it replaced asked. This supersedes the `get_envs` bullet of the previous commit message: an exported-but-empty value counts as found again, as it always did. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019QgsYAUYKDei1AM5yHJsMX
This commit is contained in:
@@ -1571,9 +1571,7 @@ fn get_envs<'a>(
|
||||
process_pat: &str,
|
||||
names: &[&'a str],
|
||||
) -> std::collections::HashMap<&'a str, String> {
|
||||
get_envs_where(uid, process_pat, names, false, |found| {
|
||||
found.values().all(|value| !value.is_empty())
|
||||
})
|
||||
get_envs_where(uid, process_pat, names, false, |count| count == names.len())
|
||||
}
|
||||
|
||||
/// The newest process matching `process_pat`, whatever it happens to carry: the semantics of the
|
||||
@@ -1585,17 +1583,12 @@ fn get_envs_of_newest<'a>(
|
||||
process_pat: &str,
|
||||
names: &[&'a str],
|
||||
) -> std::collections::HashMap<&'a str, String> {
|
||||
let mut seen = false;
|
||||
get_envs_where(uid, process_pat, names, true, |_| {
|
||||
!std::mem::replace(&mut seen, true)
|
||||
})
|
||||
get_envs_where(uid, process_pat, names, true, |_| true)
|
||||
}
|
||||
|
||||
/// `get_envs` with the caller's own notion of a complete answer: the newest process `accept`
|
||||
/// takes wins outright, and the count-based ranking is only the fallback when none is accepted.
|
||||
/// Ranking by how many of `names` a process carries cannot know that some of them are mandatory
|
||||
/// and others interchangeable, so it can rank a process holding three optional values above the
|
||||
/// one holding the mandatory pair.
|
||||
/// `get_envs` with the caller's own process order and its own notion of a complete answer, told
|
||||
/// how many of `names` the process carries: the first process `accept` takes wins outright, and
|
||||
/// the count-based ranking is only the fallback for when no process is accepted at all.
|
||||
fn get_envs_where<'a, F>(
|
||||
uid: &str,
|
||||
process_pat: &str,
|
||||
@@ -1604,7 +1597,7 @@ fn get_envs_where<'a, F>(
|
||||
mut accept: F,
|
||||
) -> std::collections::HashMap<&'a str, String>
|
||||
where
|
||||
F: FnMut(&std::collections::HashMap<&'a str, String>) -> bool,
|
||||
F: FnMut(usize) -> bool,
|
||||
{
|
||||
// The tie-breaking logic uses a u64 bitmask, limiting us to 64 variables.
|
||||
debug_assert!(
|
||||
@@ -1667,15 +1660,18 @@ where
|
||||
continue;
|
||||
};
|
||||
let cmdline_str = String::from_utf8_lossy(&cmdline).replace('\0', " ");
|
||||
if !re.is_match(&cmdline_str) {
|
||||
// The `grep -v 'grep'` of the pipeline this replaces. A user grepping for one of these
|
||||
// patterns is otherwise the newest match for it, and answers with whatever environment
|
||||
// their shell had -- an X forwarding endpoint over ssh, say.
|
||||
if cmdline_str.contains("grep") || !re.is_match(&cmdline_str) {
|
||||
continue;
|
||||
}
|
||||
|
||||
// Read environ and extract matching variables
|
||||
let environ_path = proc_path.join("environ");
|
||||
let Ok(environ) = std::fs::read(&environ_path) else {
|
||||
continue;
|
||||
};
|
||||
// Read environ and extract matching variables. A read that fails -- the process exited
|
||||
// between these two reads -- is a process carrying none of `names`, not a process to
|
||||
// skip: skipping it would hand `newest_first` on to an older PID, where the pipeline
|
||||
// this replaces stopped at the single PID its `tail -1` had already picked.
|
||||
let environ = std::fs::read(proc_path.join("environ")).unwrap_or_default();
|
||||
|
||||
let mut found = empty.clone();
|
||||
let mut found_count = 0usize;
|
||||
@@ -1695,10 +1691,7 @@ where
|
||||
continue;
|
||||
};
|
||||
if let Some(slot) = found.get_mut(key) {
|
||||
// An exported-but-empty value (`DISPLAY=`) is not an answer: counting it would
|
||||
// score this process as a match and, on a single-name query, return the empty
|
||||
// value before a process that has a real one is ever examined.
|
||||
if slot.is_empty() && !val_bytes.is_empty() {
|
||||
if slot.is_empty() {
|
||||
*slot = String::from_utf8_lossy(val_bytes).into_owned();
|
||||
found_count += 1;
|
||||
|
||||
@@ -1713,7 +1706,7 @@ where
|
||||
}
|
||||
}
|
||||
|
||||
if accept(&found) {
|
||||
if accept(found_count) {
|
||||
return found;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user