Open tertsdiepraam opened 5 months ago
Here's my take, also taking into account the criteria for inclusion.
So I would say, if this needs addressing, it needs addressing for everybody, and the glob crate is the place to do it. I do think that supporting both !
and ^
is perfectly reasonable. Perhaps the easiest way forward is to simply give them a PR to review?
I agree with your assessment and I'll try to put up a PR for glob
and link this issue in addition to the one in uutils.
There seem to be some other subtle differences documented by sudo
:
- A bracket expression starting with an unquoted
'^'
character CONTINUES to specify a non-matching list;- an explicit period
'.'
in a bracket expression matching list, e.g."[.abc]"
does NOT match a leading period in a filename;- a left-square-bracket
'['
which does not introduce a valid bracket expression is treated as an ordinary character;- a differing number of consecutive slashes within pattern and string will NOT match;
- a trailing
'\'
inFNM_ESCAPE
mode is treated as an ordinary'\'
character.
Some of these will not be very important. The third point for example seems fine if the glob
crate is more restrictive and just yields an error. That's incompatible, but it can't be misused accidentally. The fifth one also seems less important because it seems like glob
doesn't do escaping. I think these are some tests for cases 2,3 and 4:
use glob::Pattern;
fn main() {
let p = Pattern::new("[.abc]foo.txt").unwrap();
let r = p.matches(".foo.txt");
println!("Case 2: Class should not match leading `.`: {}", !r);
let p = Pattern::new("[.txt");
println!("Case 3: Unmatched [ is allowed: {}", p.is_ok());
let p = Pattern::new("foo/bar.txt").unwrap();
let r = p.matches("foo//bar.txt");
println!("Case 4: Different number of slashes should not match: {}", !r);
}
This prints:
Case 2: Class should not match leading `.`: false
Case 3: Unmatched [ is allowed: false
Case 4: Different number of slashes should not match: true
This means that glob
already does case 4 correctly, but doesn't match the behaviour on 2 and 3.
Edit: glob
does provide a require_literal_leading_dot
option that provides the correct behaviour for the leading dot issue.
Describe the bug
Reading your recent blog post regarding dependencies, I learned that
sudo-rs
depends onglob
. I figured that you might have a similar problem as uutils with this crate. In particular, we once opened this issue: https://github.com/rust-lang/glob/issues/116.To recap that issue:
glob
only allows[!...]
for negation of character classes, but not[^...]
. The standardfnmatch
andglob
functions usually do allow^
to be used, including the implementation bysudo
, even though it does not seem to be documented.I checked
src/sudoers/tokens.rs
and could not find a mitigation for this there. There also aren't any occurrences of'^'
in the code base according to GitHub search, which I would expect to see if you implemented a workaround for this issue.I'm not sure how big of an issue this is, but it's probably at least an incompatibility that should be documented. I'm also not really sure how to create a test case for this, but if you can point me to documentation for that, I'd be happy to try to create one.