D7252: dirs: reject consecutive slashes in paths

durin42 (Augie Fackler) phabricator at mercurial-scm.org
Tue Nov 19 10:38:29 EST 2019


durin42 added a comment.


  In D7252#109627 <https://phab.mercurial-scm.org/D7252#109627>, @Alphare wrote:
  
  > Sorry to necropost, but since this broke the Rust implementation, I was wondering what the best approach would be to replicate this behavior, and I am starting to think that this should be reverted.
  
  Oh, you mean the Rust version doesn't do the same rejection?
  
  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?
  
  > IIUC, currently any new path passes through the `pathauditor` first for validation, so any further check would be completely redundant.
  
  Plausible, but I'd like some sort of test coverage demonstrating that.
  
  > My intuition is that adding this check is purely here to satisfy the fuzzer, but would never happen in real life. Adding checks to this (very) internal data structure comes at a cost, both in performance and in code ergonomics.
  
  Yes, this was largely added to make the fuzzer not get stuck on OOM conditions. That said, if the pathauditor can't catch this, we need to defend against this DoS vector at this layer, and it's such a small check at this layer I'm inclined to keep it unless it is measurably slowing down real uses...

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