From 08c938e85915bfa6417fe86c16b1629b9a5dacfb Mon Sep 17 00:00:00 2001 From: Durham Goode Date: Fri, 28 Aug 2020 09:39:34 -0700 Subject: [PATCH] dirstate: block addition of paths containing "." and ".." Summary: Mergedrivers can call dirstate.add directly and are adding paths with "." and "..". Let's block those paths. Reviewed By: quark-zju Differential Revision: D23375469 fbshipit-source-id: 64e9f20169cfd50325ecd8ebcc1dd3be7a5cb202 --- eden/scm/edenscm/mercurial/dirstate.py | 4 ++++ eden/scm/lib/treestate/Cargo.toml | 1 + eden/scm/lib/treestate/src/tree.rs | 4 ++++ eden/scm/lib/treestate/src/treestate.rs | 7 +++---- eden/scm/tests/test-dirstate.t | 14 ++++++++++++++ 5 files changed, 26 insertions(+), 4 deletions(-) diff --git a/eden/scm/edenscm/mercurial/dirstate.py b/eden/scm/edenscm/mercurial/dirstate.py index 8d17a81b9b..6c741523c3 100644 --- a/eden/scm/edenscm/mercurial/dirstate.py +++ b/eden/scm/edenscm/mercurial/dirstate.py @@ -577,6 +577,10 @@ class dirstate(object): raise error.Abort( _("cannot add non-root-relative path to dirstate: %s") % f ) + relativedirs = [".", ".."] + if any(s for s in f.split("/") if s in relativedirs): + raise error.Abort(_("cannot add path with relative parents: %s") % f) + # shadows for d in util.finddirs(f): if self._map.hastrackeddir(d): diff --git a/eden/scm/lib/treestate/Cargo.toml b/eden/scm/lib/treestate/Cargo.toml index 7c33be1b0b..799be3bf69 100644 --- a/eden/scm/lib/treestate/Cargo.toml +++ b/eden/scm/lib/treestate/Cargo.toml @@ -9,6 +9,7 @@ anyhow = "1.0.20" bitflags = "1.0.4" byteorder = "1.2.7" thiserror = "1.0.5" +types = { path = "../types" } twox-hash = "*" vlqencoding = { version = "0.1.0", path = "../vlqencoding" } diff --git a/eden/scm/lib/treestate/src/tree.rs b/eden/scm/lib/treestate/src/tree.rs index 387e98acd1..04db14ebe6 100644 --- a/eden/scm/lib/treestate/src/tree.rs +++ b/eden/scm/lib/treestate/src/tree.rs @@ -17,6 +17,8 @@ use std::cell::Cell; use std::collections::Bound; use std::io::{Cursor, Read, Write}; +use types::RepoPath; + /// A node entry is an entry in a directory, either a file or another directory. #[derive(Debug)] pub(crate) enum NodeEntry { @@ -609,6 +611,8 @@ where /// Add a file to the node. The name may contain a path, in which case sufficient /// subdirectories are updated to add or update the file. fn add(&mut self, store: &dyn StoreView, name: KeyRef, info: &T) -> Result { + // Construct a RepoPath so we match the core path validation logic. + let _ = RepoPath::from_utf8(name)?; let (new_entry, file_added) = match self.path_recurse(store, name)? { PathRecurse::Directory(_dir, path, node) => { // The file is in a subdirectory. Add it to the subdirectory. diff --git a/eden/scm/lib/treestate/src/treestate.rs b/eden/scm/lib/treestate/src/treestate.rs index 0288043d5d..0a98e5e960 100644 --- a/eden/scm/lib/treestate/src/treestate.rs +++ b/eden/scm/lib/treestate/src/treestate.rs @@ -210,9 +210,8 @@ mod tests { } // Some random paths extracted from fb-hgext, plus some manually added entries, shuffled. - const SAMPLE_PATHS: [&[u8]; 22] = [ + const SAMPLE_PATHS: [&[u8]; 21] = [ b".fbarcanist", - b"build/.", b"phabricator/phabricator_graphql_client_urllib.pyc", b"hgext3rd/__init__.py", b"hgext3rd/.git/objects/14/8f179e7e702ddedb54c53f2726e7f81b14a33f", @@ -303,8 +302,8 @@ mod tests { let dir = TempDir::new("treestate").expect("tempdir"); let mut state = new_treestate(dir.path().join("1")); assert!(state.has_dir(b"/").unwrap()); - assert!(state.has_dir(b"build/").unwrap()); - assert!(!state.has_dir(b"build2/").unwrap()); + assert!(state.has_dir(b"hgext3rd/").unwrap()); + assert!(!state.has_dir(b"hgext4th/").unwrap()); assert!(state.has_dir(b"rust/radixbuf/.git/objects/").unwrap()); assert!(!state.has_dir(b"rust/radixbuf/.git2/objects/").unwrap()); } diff --git a/eden/scm/tests/test-dirstate.t b/eden/scm/tests/test-dirstate.t index a6fd7411bb..a91f24df6f 100644 --- a/eden/scm/tests/test-dirstate.t +++ b/eden/scm/tests/test-dirstate.t @@ -93,3 +93,17 @@ Verify that status reports deleted files correctly $ hg status ! a $ hg diff + +Dirstate should block addition of paths with relative parent components + $ hg up -C . + 0 files updated, 0 files merged, 0 files removed, 0 files unresolved + $ ls + $ hg debugsh -c "repo.dirstate.add('foo/../b')" + abort: cannot add path with relative parents: foo/../b + [255] + $ touch b + $ mkdir foo + $ hg add foo/../b + $ hg commit -m "add b" + $ hg status --change . + A b