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