Skip to content

Commit

Permalink
Warn about existing files when renaming or creating a directory
Browse files Browse the repository at this point in the history
This slightly improves the situation where one can rename an existing
file but there is still an obvious race. The validation only happens
when one changes the input, if you touch something via for example an
ssh session you will still overwrite the file.

For that we can't use `--no-clobber` as that does not fail with an
informative error.
  • Loading branch information
jelly committed May 28, 2024
1 parent c3b1570 commit 7270461
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 8 deletions.
10 changes: 7 additions & 3 deletions src/fileActions.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ const CreateDirectoryModal = ({ currentPath }) => {
const [name, setName] = useState("");
const [nameError, setNameError] = useState(null);
const [errorMessage, setErrorMessage] = useState(undefined);
const { cwdInfo } = useFilesContext();
const createDirectory = () => {
const path = currentPath + name;
cockpit.spawn(["mkdir", path], { superuser: "try", err: "message" })
Expand Down Expand Up @@ -172,7 +173,7 @@ const CreateDirectoryModal = ({ currentPath }) => {
<TextInput
validated={nameError ? "error" : "default"}
value={name}
onChange={(_, val) => setDirectoryName(val, setName, setNameError, setErrorMessage)}
onChange={(_, val) => setDirectoryName(val, setName, setNameError, setErrorMessage, cwdInfo)}
id="create-directory-input" autoFocus // eslint-disable-line jsx-a11y/no-autofocus
/>
<FormHelper fieldId="create-directory-input" helperTextInvalid={nameError} />
Expand All @@ -185,6 +186,7 @@ const CreateDirectoryModal = ({ currentPath }) => {

const RenameItemModal = ({ path, selected }) => {
const Dialogs = useDialogs();
const { cwdInfo } = useFilesContext();
const [name, setName] = useState(selected.name);
const [nameError, setNameError] = useState(null);
const [errorMessage, setErrorMessage] = useState(undefined);
Expand Down Expand Up @@ -241,7 +243,7 @@ const RenameItemModal = ({ path, selected }) => {
<FormGroup fieldId="rename-item-input" label={_("New name")}>
<TextInput
value={name}
onChange={(_, val) => setDirectoryName(val, setName, setNameError, setErrorMessage)}
onChange={(_, val) => setDirectoryName(val, setName, setNameError, setErrorMessage, cwdInfo)}
id="rename-item-input"
/>
<FormHelper fieldId="rename-item-input" helperTextInvalid={nameError} />
Expand Down Expand Up @@ -436,7 +438,7 @@ const EditPermissionsModal = ({ selected, path }) => {
);
};

const setDirectoryName = (val, setName, setNameError, setErrorMessage) => {
const setDirectoryName = (val, setName, setNameError, setErrorMessage, cwdInfo) => {
setErrorMessage(undefined);
setName(val);

Expand All @@ -446,6 +448,8 @@ const setDirectoryName = (val, setName, setNameError, setErrorMessage) => {
setNameError(_("Name too long."));
} else if (val.includes("/")) {
setNameError(_("Name cannot include a /."));
} else if (cwdInfo?.entries[val]) {
setNameError(_("File or directory already exists"));
} else {
setNameError(null);
}
Expand Down
15 changes: 10 additions & 5 deletions test/check-application
Original file line number Diff line number Diff line change
Expand Up @@ -488,9 +488,11 @@ class TestFiles(testlib.MachineCase):
b.click(".pf-v5-c-modal-box__footer button.pf-m-link") # cancel

# Creating folder with duplicate name should return an error
self.create_directory(b, "newdir")
self.wait_modal_inline_alert(b, "mkdir: cannot create directory ‘/home/admin/newdir’: File exists")
b.click("div.pf-v5-c-modal-box__close button.pf-v5-c-button")
b.click("#dropdown-menu")
b.click("#create-item")
b.set_input_text("#create-directory-input", "newdir")
b.wait_in_text("#create-directory-input-helper", "File or directory already exists")
b.click(".pf-v5-c-modal-box__footer button.pf-m-link") # cancel

# Creating folder with empty name should return an error
self.create_directory(b, "")
Expand Down Expand Up @@ -636,8 +638,11 @@ class TestFiles(testlib.MachineCase):
""")
b.wait_visible("[data-item='newfile']")
b.wait_visible("[data-item='dest']")
self.rename_item(b, "newfile", "dest")
self.wait_modal_inline_alert(b, "mv: cannot overwrite directory '/home/admin/dest' with non-directory")
b.click("[data-item='newfile']")
b.click("#dropdown-menu")
b.click("#rename-item")
b.set_input_text("#rename-item-input", "dest")
b.wait_in_text("#rename-item-input-helper", "File or directory already exists")
b.click("button.pf-m-link:contains('Cancel')")
b.wait_not_present(".pf-v5-c-modal-box")

Expand Down

0 comments on commit 7270461

Please sign in to comment.