fix urlencoding in the filemanager

schrej · schrej · commit 4fd2af028d21 · 2021-01-02T02:15:32.000+01:00
diff --git a/app/Transformers/Daemon/FileObjectTransformer.php b/app/Transformers/Daemon/FileObjectTransformer.php
@@ -23,7 +23,7 @@ class FileObjectTransformer extends BaseDaemonTransformer
     public function transform(array $item)
     {
         return [
-            'name' => rawurlencode(Arr::get($item, 'name')),
+            'name' => Arr::get($item, 'name'),
             'mode' => Arr::get($item, 'mode'),
             'mode_bits' => Arr::get($item, 'mode_bits'),
             'size' => Arr::get($item, 'size'),
diff --git a/resources/scripts/api/server/files/loadDirectory.ts b/resources/scripts/api/server/files/loadDirectory.ts
@@ -18,8 +18,6 @@ export interface FileObject {
 
 export default async (uuid: string, directory?: string): Promise<FileObject[]> => {
     const { data } = await http.get(`/api/client/servers/${uuid}/files/list`, {
-        // At this point the directory is still encoded so we need to decode it since axios
-        // will automatically re-encode this value before sending it along in the request.
         params: { directory: directory ?? '/' },
     });
 
diff --git a/resources/scripts/components/server/files/FileEditContainer.tsx b/resources/scripts/components/server/files/FileEditContainer.tsx
@@ -17,6 +17,8 @@ import modes from '@/modes';
 import useFlash from '@/plugins/useFlash';
 import { ServerContext } from '@/state/server';
 import ErrorBoundary from '@/components/elements/ErrorBoundary';
+import { encodePathSegments, hashToPath } from '@/helpers';
+import { dirname } from 'path';
 
 const LazyCodemirrorEditor = lazy(() => import(/* webpackChunkName: "editor" */'@/components/elements/CodemirrorEditor'));
 
@@ -43,8 +45,9 @@ export default () => {
 
         setError('');
         setLoading(true);
-        setDirectory(hash.replace(/^#/, '').split('/').filter(v => !!v).slice(0, -1).join('/'));
-        getFileContents(uuid, hash.replace(/^#/, ''))
+        const path = hashToPath(hash);
+        setDirectory(dirname(path));
+        getFileContents(uuid, path)
             .then(setContent)
             .catch(error => {
                 console.error(error);
@@ -61,10 +64,10 @@ export default () => {
         setLoading(true);
         clearFlashes('files:view');
         fetchFileContent()
-            .then(content => saveFileContents(uuid, name || decodeURI(hash.replace(/^#/, '')), content))
+            .then(content => saveFileContents(uuid, name || hashToPath(hash), content))
             .then(() => {
                 if (name) {
-                    history.push(`/server/${id}/files/edit#/${name}`);
+                    history.push(`/server/${id}/files/edit#/${encodePathSegments(name)}`);
                     return;
                 }
 
diff --git a/resources/scripts/components/server/files/FileManagerBreadcrumbs.tsx b/resources/scripts/components/server/files/FileManagerBreadcrumbs.tsx
@@ -1,7 +1,7 @@
 import React, { useEffect, useState } from 'react';
 import { ServerContext } from '@/state/server';
 import { NavLink, useLocation } from 'react-router-dom';
-import { cleanDirectoryPath } from '@/helpers';
+import { encodePathSegments, hashToPath } from '@/helpers';
 import tw from 'twin.macro';
 
 interface Props {
@@ -17,22 +17,10 @@ export default ({ renderLeft, withinFileEditor, isNewFile }: Props) => {
     const { hash } = useLocation();
 
     useEffect(() => {
-        let pathHash = cleanDirectoryPath(hash);
-        try {
-            pathHash = decodeURI(pathHash);
-        } catch (e) {
-            console.warn('Error decoding URL parts in hash:', e);
-        }
+        const path = hashToPath(hash);
 
         if (withinFileEditor && !isNewFile) {
-            let name = pathHash.split('/').pop() || null;
-            if (name) {
-                try {
-                    name = decodeURIComponent(name);
-                } catch (e) {
-                    console.warn('Error decoding filename:', e);
-                }
-            }
+            const name = path.split('/').pop() || null;
             setFile(name);
         }
     }, [ withinFileEditor, isNewFile, hash ]);
@@ -62,14 +50,14 @@ export default ({ renderLeft, withinFileEditor, isNewFile }: Props) => {
                     crumb.path ?
                         <React.Fragment key={index}>
                             <NavLink
-                                to={`/server/${id}/files#${crumb.path}`}
+                                to={`/server/${id}/files#${encodePathSegments(crumb.path)}`}
                                 css={tw`px-1 text-neutral-200 no-underline hover:text-neutral-100`}
                             >
-                                {decodeURIComponent(crumb.name)}
+                                {crumb.name}
                             </NavLink>/
                         </React.Fragment>
                         :
-                        <span key={index} css={tw`px-1 text-neutral-300`}>{decodeURIComponent(crumb.name)}</span>
+                        <span key={index} css={tw`px-1 text-neutral-300`}>{crumb.name}</span>
                 ))
             }
             {file &&
diff --git a/resources/scripts/components/server/files/FileManagerContainer.tsx b/resources/scripts/components/server/files/FileManagerContainer.tsx
@@ -19,6 +19,7 @@ import ServerContentBlock from '@/components/elements/ServerContentBlock';
 import { useStoreActions } from '@/state/hooks';
 import ErrorBoundary from '@/components/elements/ErrorBoundary';
 import { FileActionCheckbox } from '@/components/server/files/SelectFileCheckbox';
+import { hashToPath } from '@/helpers';
 
 const sortFiles = (files: FileObject[]): FileObject[] => {
     return files.sort((a, b) => a.name.localeCompare(b.name))
@@ -39,7 +40,7 @@ export default () => {
     useEffect(() => {
         clearFlashes('files');
         setSelectedFiles([]);
-        setDirectory(hash.length > 0 ? hash : '/');
+        setDirectory(hashToPath(hash));
     }, [ hash ]);
 
     useEffect(() => {
diff --git a/resources/scripts/components/server/files/FileObjectRow.tsx b/resources/scripts/components/server/files/FileObjectRow.tsx
@@ -1,17 +1,18 @@
 import { FontAwesomeIcon } from '@fortawesome/react-fontawesome';
 import { faFileAlt, faFileArchive, faFileImport, faFolder } from '@fortawesome/free-solid-svg-icons';
-import { bytesToHuman, cleanDirectoryPath } from '@/helpers';
+import { bytesToHuman, encodePathSegments } from '@/helpers';
 import { differenceInHours, format, formatDistanceToNow } from 'date-fns';
 import React, { memo } from 'react';
 import { FileObject } from '@/api/server/files/loadDirectory';
 import FileDropdownMenu from '@/components/server/files/FileDropdownMenu';
 import { ServerContext } from '@/state/server';
-import { NavLink, useHistory, useRouteMatch } from 'react-router-dom';
+import { NavLink, useRouteMatch } from 'react-router-dom';
 import tw from 'twin.macro';
 import isEqual from 'react-fast-compare';
 import styled from 'styled-components/macro';
 import SelectFileCheckbox from '@/components/server/files/SelectFileCheckbox';
 import { usePermissions } from '@/plugins/usePermissions';
+import { join } from 'path';
 
 const Row = styled.div`
     ${tw`flex bg-neutral-700 rounded-sm mb-px text-sm hover:text-neutral-100 cursor-pointer items-center no-underline hover:bg-neutral-600`};
@@ -21,33 +22,17 @@ const Clickable: React.FC<{ file: FileObject }> = memo(({ file, children }) => {
     const [ canReadContents ] = usePermissions([ 'file.read-content' ]);
     const directory = ServerContext.useStoreState(state => state.files.directory);
 
-    const history = useHistory();
     const match = useRouteMatch();
 
-    const destination = cleanDirectoryPath(`${directory}/${file.name}`).split('/').join('/');
-
-    const onRowClick = (e: React.MouseEvent<HTMLAnchorElement, MouseEvent>) => {
-        // Don't rely on the onClick to work with the generated URL. Because of the way this
-        // component re-renders you'll get redirected into a nested directory structure since
-        // it'll cause the directory variable to update right away when you click.
-        //
-        // Just trust me future me, leave this be.
-        if (!file.isFile) {
-            e.preventDefault();
-            history.push(`#${destination}`);
-        }
-    };
-
     return (
         (!canReadContents || (file.isFile && !file.isEditable())) ?
             <div css={tw`flex flex-1 text-neutral-300 no-underline p-3 cursor-default overflow-hidden truncate`}>
                 {children}
             </div>
             :
             <NavLink
-                to={`${match.url}${file.isFile ? '/edit' : ''}#${destination}`}
+                to={`${match.url}${file.isFile ? '/edit' : ''}#${encodePathSegments(join(directory, file.name))}`}
                 css={tw`flex flex-1 text-neutral-300 no-underline p-3 overflow-hidden truncate`}
-                onClick={onRowClick}
             >
                 {children}
             </NavLink>
@@ -72,7 +57,7 @@ const FileObjectRow = ({ file }: { file: FileObject }) => (
                 }
             </div>
             <div css={tw`flex-1 truncate`}>
-                {decodeURIComponent(file.name)}
+                {file.name}
             </div>
             {file.isFile &&
             <div css={tw`w-1/6 text-right mr-4 hidden sm:block`}>
diff --git a/resources/scripts/helpers.ts b/resources/scripts/helpers.ts
@@ -17,7 +17,7 @@ export function megabytesToHuman (mb: number): string {
 
 export const randomInt = (low: number, high: number) => Math.floor(Math.random() * (high - low) + low);
 
-export const cleanDirectoryPath = (path: string) => path.replace(/(^#\/*)|(\/(\/*))|(^$)/g, '/');
+export const cleanDirectoryPath = (path: string) => path.replace(/(\/(\/*))|(^$)/g, '/');
 
 export const capitalize = (s: string) => s.charAt(0).toUpperCase() + s.slice(1).toLowerCase();
 
@@ -50,3 +50,16 @@ export function fileBitsToString (mode: string, directory: boolean): string {
 
     return buf;
 }
+
+/**
+ * URL-encodes the segments of a path.
+ * This allows to use the path as part of a URL while preserving the slashes.
+ * @param path the path to encode
+ */
+export function encodePathSegments (path: string): string {
+    return path.split('/').map(s => encodeURIComponent(s)).join('/');
+}
+
+export function hashToPath (hash: string): string {
+    return hash.length > 0 ? decodeURIComponent(hash.substr(1)) : '/';
+}

Original file line number	Diff line number	Diff line change
`@@ -23,7 +23,7 @@ class FileObjectTransformer extends BaseDaemonTransformer`
`23`	`23`	`public function transform(array $item)`
`24`	`24`	`{`
`25`	`25`	`return [`
`26`		`- 'name' => rawurlencode(Arr::get($item, 'name')),`
	`26`	`+ 'name' => Arr::get($item, 'name'),`
`27`	`27`	`'mode' => Arr::get($item, 'mode'),`
`28`	`28`	`'mode_bits' => Arr::get($item, 'mode_bits'),`
`29`	`29`	`'size' => Arr::get($item, 'size'),`