D7252: dirs: reject consecutive slashes in paths
Alphare (Raphaël Gomès)
phabricator at mercurial-scm.org
Tue Nov 19 10:52:35 EST 2019
Alphare added a comment.
> Oh, you mean the Rust version doesn't do the same rejection?
It does not, currently.
> Given that you're about to do a hash lookup, I'm a little skeptical that a `endswith('/')` check would show up meaningfully in a profiler, but I'm willing to be proven wrong?
I have the same intuition you do, it's mostly about not repeating the same operations that have any impact at all on performance/code.
> I should add: if we can substantiate that such a path can't make it through the pathauditor (and we have tests for that in the pathauditor layer so we don't break that in the future!) we can push this consecutive-slashes check into dirs_fuzzer.cc and remove it from dirs.c.
It was my candid intuition that the `pathauditor` was already "ensured" as the barrier for path-based vulnerabilities.
Since my plan for the Rust implementation was to enforce that very fact, I guess I'll be the one to write said tests when I write a Rust `pathauditor`. It should come up not too long after my current work (in-progress) about matchers, since handling unknown files in any capacity will require the `pathauditor`.
I'm not 100% sure of the implications of writing the aforementioned tests, if it's too much work, I can replicate this check in Rust for the time being.
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST ACTION
https://phab.mercurial-scm.org/D7252/new/
REVISION DETAIL
https://phab.mercurial-scm.org/D7252
To: durin42, #hg-reviewers, indygreg
Cc: Alphare, mercurial-devel
More information about the Mercurial-devel
mailing list