From d51ff87c26baf08ad2de0d8af79a96ef8f125b52 Mon Sep 17 00:00:00 2001 From: Evan Simkowitz Date: Fri, 21 Feb 2025 16:32:14 -0800 Subject: [PATCH] Not found paths in prefix fs always treated as dir (#2002) Gracefully handle prefix paths that don't exist, representing them as directories so they can be escaped from. Also removes the ".." file info from the backend, instead only creating it on the frontend --- .../app/view/preview/directorypreview.tsx | 82 ++++++++++--------- frontend/app/view/preview/preview.tsx | 76 +---------------- pkg/remote/fileshare/fsutil/fsutil.go | 4 +- pkg/remote/fileshare/s3fs/s3fs.go | 32 ++++---- pkg/remote/fileshare/wavefs/wavefs.go | 10 +++ pkg/wshrpc/wshremote/wshremote.go | 7 -- 6 files changed, 79 insertions(+), 132 deletions(-) diff --git a/frontend/app/view/preview/directorypreview.tsx b/frontend/app/view/preview/directorypreview.tsx index 7ce987d06..6248df604 100644 --- a/frontend/app/view/preview/directorypreview.tsx +++ b/frontend/app/view/preview/directorypreview.tsx @@ -757,7 +757,6 @@ function DirectoryPreview({ model }: DirectoryPreviewProps) { const [searchText, setSearchText] = useState(""); const [focusIndex, setFocusIndex] = useState(0); const [unfilteredData, setUnfilteredData] = useState([]); - const [filteredData, setFilteredData] = useState([]); const showHiddenFiles = useAtomValue(model.showHiddenFiles); const [selectedPath, setSelectedPath] = useState(""); const [refreshVersion, setRefreshVersion] = useAtom(model.refreshVersion); @@ -776,44 +775,53 @@ function DirectoryPreview({ model }: DirectoryPreviewProps) { }; }, [setRefreshVersion]); - useEffect(() => { - const getContent = async () => { - let entries: FileInfo[]; - try { - const file = await RpcApi.FileReadCommand( - TabRpcClient, - { - info: { - path: await model.formatRemoteUri(dirPath, globalStore.get), + useEffect( + () => + fireAndForget(async () => { + let entries: FileInfo[]; + try { + const file = await RpcApi.FileReadCommand( + TabRpcClient, + { + info: { + path: await model.formatRemoteUri(dirPath, globalStore.get), + }, }, - }, - null - ); - entries = file.entries ?? []; - } catch (e) { - setErrorMsg({ - status: "Cannot Read Directory", - text: `${e}`, - }); - } - setUnfilteredData(entries); - }; - getContent(); - }, [conn, dirPath, refreshVersion]); + null + ); + entries = file.entries ?? []; + entries.unshift({ + name: "..", + path: file?.info?.dir, + isdir: true, + modtime: new Date().getTime(), + mimetype: "directory", + }); + } catch (e) { + setErrorMsg({ + status: "Cannot Read Directory", + text: `${e}`, + }); + } + setUnfilteredData(entries); + }), + [conn, dirPath, refreshVersion] + ); - useEffect(() => { - const filtered = unfilteredData?.filter((fileInfo) => { - if (fileInfo.name == null) { - console.log("fileInfo.name is null", fileInfo); - return false; - } - if (!showHiddenFiles && fileInfo.name.startsWith(".") && fileInfo.name != "..") { - return false; - } - return fileInfo.name.toLowerCase().includes(searchText); - }); - setFilteredData(filtered ?? []); - }, [unfilteredData, showHiddenFiles, searchText]); + const filteredData = useMemo( + () => + unfilteredData?.filter((fileInfo) => { + if (fileInfo.name == null) { + console.log("fileInfo.name is null", fileInfo); + return false; + } + if (!showHiddenFiles && fileInfo.name.startsWith(".") && fileInfo.name != "..") { + return false; + } + return fileInfo.name.toLowerCase().includes(searchText); + }) ?? [], + [unfilteredData, showHiddenFiles, searchText] + ); useEffect(() => { model.directoryKeyDownHandler = (waveEvent: WaveKeyboardEvent): boolean => { diff --git a/frontend/app/view/preview/preview.tsx b/frontend/app/view/preview/preview.tsx index 6a86be3b1..b554bae80 100644 --- a/frontend/app/view/preview/preview.tsx +++ b/frontend/app/view/preview/preview.tsx @@ -5,7 +5,6 @@ import { BlockNodeModel } from "@/app/block/blocktypes"; import { Button } from "@/app/element/button"; import { CopyButton } from "@/app/element/copybutton"; import { CenteredDiv } from "@/app/element/quickelems"; -import { TypeAheadModal } from "@/app/modals/typeaheadmodal"; import { ContextMenuModel } from "@/app/store/contextmenu"; import { tryReinjectKey } from "@/app/store/keymodel"; import { RpcApi } from "@/app/store/wshclientapi"; @@ -18,7 +17,7 @@ import * as services from "@/store/services"; import * as WOS from "@/store/wos"; import { getWebServerEndpoint } from "@/util/endpoints"; import { goHistory, goHistoryBack, goHistoryForward } from "@/util/historyutil"; -import { adaptFromReactOrNativeKeyEvent, checkKeyPressed, keydownWrapper } from "@/util/keyutil"; +import { adaptFromReactOrNativeKeyEvent, checkKeyPressed } from "@/util/keyutil"; import { addOpenMenuItems } from "@/util/previewutil"; import { base64ToString, fireAndForget, isBlank, jotaiLoadableValue, makeConnRoute, stringToBase64 } from "@/util/util"; import { formatRemoteUri } from "@/util/waveutil"; @@ -28,7 +27,7 @@ import { Atom, atom, Getter, PrimitiveAtom, useAtom, useAtomValue, useSetAtom, W import { loadable } from "jotai/utils"; import type * as MonacoTypes from "monaco-editor/esm/vs/editor/editor.api"; import { OverlayScrollbarsComponent } from "overlayscrollbars-react"; -import { createRef, memo, useCallback, useEffect, useMemo, useState } from "react"; +import { createRef, memo, useCallback, useEffect, useMemo } from "react"; import { TransformComponent, TransformWrapper, useControls } from "react-zoom-pan-pinch"; import { CSVView } from "./csvview"; import { DirectoryPreview } from "./directorypreview"; @@ -1073,17 +1072,17 @@ function PreviewView({ model: PreviewModel; }) { const connStatus = useAtomValue(model.connStatus); - const filePath = useAtomValue(model.metaFilePath); const [errorMsg, setErrorMsg] = useAtom(model.errorMsgAtom); const connection = useAtomValue(model.connectionImmediate); const fileInfo = useAtomValue(model.statFile); useEffect(() => { + console.log("fileInfo or connection changed", fileInfo, connection); if (!fileInfo) { return; } setErrorMsg(null); - }, [connection, filePath, fileInfo]); + }, [connection, fileInfo]); if (connStatus?.status != "connected") { return null; @@ -1113,7 +1112,6 @@ function PreviewView({ return ( <> - {/* */}
{errorMsg && setErrorMsg(null)} />}
@@ -1133,72 +1131,6 @@ function PreviewView({ ); } -const OpenFileModal = memo( - ({ - model, - blockRef, - blockId, - }: { - model: PreviewModel; - blockRef: React.RefObject; - blockId: string; - }) => { - const openFileModal = useAtomValue(model.openFileModal); - const curFileName = useAtomValue(model.metaFilePath); - const [filePath, setFilePath] = useState(""); - const isNodeFocused = useAtomValue(model.nodeModel.isFocused); - const handleKeyDown = useCallback( - keydownWrapper((waveEvent: WaveKeyboardEvent): boolean => { - if (checkKeyPressed(waveEvent, "Escape")) { - model.updateOpenFileModalAndError(false); - return true; - } - - const handleCommandOperations = async () => { - if (checkKeyPressed(waveEvent, "Enter")) { - await model.handleOpenFile(filePath); - return true; - } - return false; - }; - - handleCommandOperations().catch((error) => { - console.error("Error handling key down:", error); - model.updateOpenFileModalAndError(true, "An error occurred during operation."); - return false; - }); - return false; - }), - [model, blockId, filePath, curFileName] - ); - const handleFileSuggestionSelect = (value) => { - globalStore.set(model.openFileModal, false); - }; - const handleFileSuggestionChange = (value) => { - setFilePath(value); - }; - const handleBackDropClick = () => { - globalStore.set(model.openFileModal, false); - }; - if (!openFileModal) { - return null; - } - return ( - - ); - } -); - const ErrorOverlay = memo(({ errorMsg, resetOverlay }: { errorMsg: ErrorMsg; resetOverlay: () => void }) => { const showDismiss = errorMsg.showDismiss ?? true; const buttonClassName = "outlined grey font-size-11 vertical-padding-3 horizontal-padding-7"; diff --git a/pkg/remote/fileshare/fsutil/fsutil.go b/pkg/remote/fileshare/fsutil/fsutil.go index 617bbdc70..c58d47597 100644 --- a/pkg/remote/fileshare/fsutil/fsutil.go +++ b/pkg/remote/fileshare/fsutil/fsutil.go @@ -150,6 +150,8 @@ func DetermineCopyDestPath(ctx context.Context, srcConn, destConn *connparse.Con srcInfo, err = srcClient.Stat(ctx, srcConn) if err != nil { return "", "", nil, fmt.Errorf("error getting source file info: %w", err) + } else if srcInfo.NotFound { + return "", "", nil, fmt.Errorf("source file not found: %w", err) } destInfo, err := destClient.Stat(ctx, destConn) destExists := err == nil && !destInfo.NotFound @@ -158,7 +160,7 @@ func DetermineCopyDestPath(ctx context.Context, srcConn, destConn *connparse.Con } originalDestPath := destPath if !srcHasSlash { - if destInfo.IsDir || (!destExists && !destHasSlash && srcInfo.IsDir) { + if (destExists && destInfo.IsDir) || (!destExists && !destHasSlash && srcInfo.IsDir) { destPath = fspath.Join(destPath, fspath.Base(srcConn.Path)) } } diff --git a/pkg/remote/fileshare/s3fs/s3fs.go b/pkg/remote/fileshare/s3fs/s3fs.go index f6d24ef81..9864b58c9 100644 --- a/pkg/remote/fileshare/s3fs/s3fs.go +++ b/pkg/remote/fileshare/s3fs/s3fs.go @@ -62,6 +62,20 @@ func (c S3Client) ReadStream(ctx context.Context, conn *connparse.Connection, da return } rtn <- wshrpc.RespOrErrorUnion[wshrpc.FileData]{Response: wshrpc.FileData{Info: finfo}} + if finfo.NotFound { + rtn <- wshrpc.RespOrErrorUnion[wshrpc.FileData]{Response: wshrpc.FileData{Entries: []*wshrpc.FileInfo{ + { + Path: finfo.Dir, + Dir: fspath.Dir(finfo.Dir), + Name: "..", + IsDir: true, + Size: 0, + ModTime: time.Now().Unix(), + MimeType: "directory", + }, + }}} + return + } if finfo.IsDir { listEntriesCh := c.ListEntriesStream(ctx, conn, nil) defer func() { @@ -455,20 +469,6 @@ func (c S3Client) ListEntriesStream(ctx context.Context, conn *connparse.Connect rtn <- wshutil.RespErr[wshrpc.CommandRemoteListEntriesRtnData](err) return } - parentPath := fsutil.GetParentPath(conn) - if parentPath != "" { - rtn <- wshrpc.RespOrErrorUnion[wshrpc.CommandRemoteListEntriesRtnData]{Response: wshrpc.CommandRemoteListEntriesRtnData{FileInfo: []*wshrpc.FileInfo{ - { - Path: parentPath, - Dir: fsutil.GetParentPathString(parentPath), - Name: "..", - IsDir: true, - Size: 0, - ModTime: time.Now().Unix(), - MimeType: "directory", - }, - }}} - } entries := make([]*wshrpc.FileInfo, 0, wshrpc.DirChunkSize) for _, entry := range entryMap { entries = append(entries, entry) @@ -532,6 +532,7 @@ func (c S3Client) Stat(ctx context.Context, conn *connparse.Connection) (*wshrpc Path: bucketName, Dir: fspath.Separator, NotFound: true, + IsDir: true, }, nil } } @@ -556,7 +557,7 @@ func (c S3Client) Stat(ctx context.Context, conn *connparse.Connection) (*wshrpc MaxKeys: aws.Int32(1), }) if err == nil { - if entries.Contents != nil && len(entries.Contents) > 0 { + if entries.Contents != nil { return &wshrpc.FileInfo{ Name: objectKey, Path: conn.GetPathWithHost(), @@ -575,6 +576,7 @@ func (c S3Client) Stat(ctx context.Context, conn *connparse.Connection) (*wshrpc Name: objectKey, Path: conn.GetPathWithHost(), Dir: fsutil.GetParentPath(conn), + IsDir: true, NotFound: true, }, nil } diff --git a/pkg/remote/fileshare/wavefs/wavefs.go b/pkg/remote/fileshare/wavefs/wavefs.go index cbd672a1d..3d7476fd3 100644 --- a/pkg/remote/fileshare/wavefs/wavefs.go +++ b/pkg/remote/fileshare/wavefs/wavefs.go @@ -105,6 +105,16 @@ func (c WaveClient) Read(ctx context.Context, conn *connparse.Connection, data w if err != nil { return nil, fmt.Errorf("error listing blockfiles: %w", err) } + if len(list) == 0 { + return &wshrpc.FileData{ + Info: &wshrpc.FileInfo{ + Name: fspath.Base(fileName), + Path: fileName, + Dir: fspath.Dir(fileName), + NotFound: true, + IsDir: true, + }}, nil + } return &wshrpc.FileData{Info: data.Info, Entries: list}, nil } diff --git a/pkg/wshrpc/wshremote/wshremote.go b/pkg/wshrpc/wshremote/wshremote.go index afa269fc9..ec90e367c 100644 --- a/pkg/wshrpc/wshremote/wshremote.go +++ b/pkg/wshrpc/wshremote/wshremote.go @@ -108,13 +108,6 @@ func (impl *ServerImpl) remoteStreamFileDir(ctx context.Context, path string, by } } var fileInfoArr []*wshrpc.FileInfo - parent := filepath.Dir(path) - parentFileInfo, err := impl.fileInfoInternal(parent, false) - if err == nil && parent != path { - parentFileInfo.Name = ".." - parentFileInfo.Size = -1 - fileInfoArr = append(fileInfoArr, parentFileInfo) - } for _, innerFileEntry := range innerFilesEntries { if ctx.Err() != nil { return ctx.Err()