fix(dav): file drop nickname

Signed-off-by: skjnldsv <skjnldsv@protonmail.com>

apps/dav/lib/Files/Sharing/FilesDropPlugin.php

@@ -8,6 +8,7 @@ namespace OCA\DAV\Files\Sharing;
 use OCP\Files\Folder;
 use OCP\Files\NotFoundException;
 use OCP\Share\IShare;
+use Sabre\DAV\Exception\BadRequest;
 use Sabre\DAV\Exception\MethodNotAllowed;
 use Sabre\DAV\ServerPlugin;
 use Sabre\HTTP\RequestInterface;
@@ -71,13 +72,12 @@ class FilesDropPlugin extends ServerPlugin {
 			? trim(urldecode($request->getHeader('X-NC-Nickname')))
 			: null;
 
-		//
 		if ($request->getMethod() !== 'PUT') {
 			// If uploading subfolders we need to ensure they get created
 			// within the nickname folder
 			if ($request->getMethod() === 'MKCOL') {
 				if (!$nickname) {
-					throw new MethodNotAllowed('A nickname header is required when uploading subfolders');
+					throw new BadRequest('A nickname header is required when uploading subfolders');
 				}
 			} else {
 				throw new MethodNotAllowed('Only PUT is allowed on files drop');
@@ -113,7 +113,7 @@ class FilesDropPlugin extends ServerPlugin {
 
 		// We need a valid nickname for file requests
 		if ($isFileRequest && !$nickname) {
-			throw new MethodNotAllowed('A nickname header is required for file requests');
+			throw new BadRequest('A nickname header is required for file requests');
 		}
 
 		// We're only allowing the upload of
@@ -121,12 +121,24 @@ class FilesDropPlugin extends ServerPlugin {
 		// This prevents confusion when uploading files and help
 		// classify them by uploaders.
 		if (!$nickname && !$isRootUpload) {
-			throw new MethodNotAllowed('A nickname header is required when uploading subfolders');
+			throw new BadRequest('A nickname header is required when uploading subfolders');
 		}
 
-		// If we have a nickname, let's put everything inside
 		if ($nickname) {
-			// Put all files in the subfolder
+			try {
+				$node->verifyPath($nickname);
+			} catch (\Exception $e) {
+				// If the path is not valid, we throw an exception
+				throw new BadRequest('Invalid nickname: ' . $nickname);
+			}
+
+			// Forbid nicknames starting with a dot
+			if (str_starts_with($nickname, '.')) {
+				throw new BadRequest('Invalid nickname: ' . $nickname);
+			}
+
+			// If we have a nickname, let's put
+			// all files in the subfolder
 			$relativePath = '/' . $nickname . '/' . $relativePath;
 			$relativePath = str_replace('//', '/', $relativePath);
 		}

apps/dav/tests/unit/Files/Sharing/FilesDropPluginTest.php

@@ -13,7 +13,7 @@ use OCP\Files\NotFoundException;
 use OCP\Share\IAttributes;
 use OCP\Share\IShare;
 use PHPUnit\Framework\MockObject\MockObject;
-use Sabre\DAV\Exception\MethodNotAllowed;
+use Sabre\DAV\Exception\BadRequest;
 use Sabre\DAV\Server;
 use Sabre\HTTP\RequestInterface;
 use Sabre\HTTP\ResponseInterface;
@@ -119,7 +119,7 @@ class FilesDropPluginTest extends TestCase {
 		$this->request->method('getMethod')
 			->willReturn('MKCOL');
 
-		$this->expectException(MethodNotAllowed::class);
+		$this->expectException(BadRequest::class);
 
 		$this->plugin->beforeMethod($this->request, $this->response);
 	}

apps/files/src/utils/filenameValidity.ts

@@ -8,16 +8,16 @@ import { t } from '@nextcloud/l10n'
 /**
  * Get the validity of a filename (empty if valid).
  * This can be used for `setCustomValidity` on input elements
- * @param input The filename
+ * @param name The filename
  * @param escape Escape the matched string in the error (only set when used in HTML)
  */
-export function getFilenameValidity(input: string, escape = false): string {
-	if (input.trim() === '') {
-		return t('files', 'This field must not be empty.')
+export function getFilenameValidity(name: string, escape = false): string {
+	if (name.trim() === '') {
+		return t('files', 'Filename must not be empty.')
 	}
 
 	try {
-		validateFilename(input)
+		validateFilename(name)
 		return ''
 	} catch (error) {
 		if (!(error instanceof InvalidFilenameError)) {
@@ -26,16 +26,16 @@ export function getFilenameValidity(input: string, escape = false): string {
 
 		switch (error.reason) {
 		case InvalidFilenameErrorReason.Character:
-			return t('files', 'The character "{char}" is not allowed.', { char: error.segment }, undefined, { escape })
+			return t('files', '"{char}" is not allowed inside a filename.', { char: error.segment }, undefined, { escape })
 		case InvalidFilenameErrorReason.ReservedName:
-			return t('files', '"{segment}" is reserved and cannot be used.', { segment: error.segment }, undefined, { escape: false })
+			return t('files', '"{segment}" is a reserved name and not allowed for filenames.', { segment: error.segment }, undefined, { escape: false })
 		case InvalidFilenameErrorReason.Extension:
 			if (error.segment.match(/\.[a-z]/i)) {
-				return t('files', '"{extension}" is not a supported type.', { extension: error.segment }, undefined, { escape: false })
+				return t('files', '"{extension}" is not an allowed filetype.', { extension: error.segment }, undefined, { escape: false })
 			}
-			return t('files', 'Cannot end with "{extension}".', { extension: error.segment }, undefined, { escape: false })
+			return t('files', 'Filenames must not end with "{extension}".', { extension: error.segment }, undefined, { escape: false })
 		default:
-			return t('files', 'This value is not allowed.')
+			return t('files', 'Invalid filename.')
 		}
 	}
 }

apps/files_sharing/src/services/GuestNameValidity.ts

@@ -0,0 +1,45 @@
+/*!
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+import { InvalidFilenameError, InvalidFilenameErrorReason, validateFilename } from '@nextcloud/files'
+import { t } from '@nextcloud/l10n'
+
+/**
+ * Get the validity of a filename (empty if valid).
+ * This can be used for `setCustomValidity` on input elements
+ * @param name The filename
+ * @param escape Escape the matched string in the error (only set when used in HTML)
+ */
+export function getGuestNameValidity(name: string, escape = false): string {
+	if (name.trim() === '') {
+		return t('files', 'Filename must not be empty.')
+	}
+
+	if (name.startsWith('.')) {
+		return t('files', 'Names must not start with a dot.')
+	}
+
+	try {
+		validateFilename(name)
+		return ''
+	} catch (error) {
+		if (!(error instanceof InvalidFilenameError)) {
+			throw error
+		}
+
+		switch (error.reason) {
+		case InvalidFilenameErrorReason.Character:
+			return t('files', '"{char}" is not allowed inside a name.', { char: error.segment }, undefined, { escape })
+		case InvalidFilenameErrorReason.ReservedName:
+			return t('files', '"{segment}" is a reserved name and not allowed.', { segment: error.segment }, undefined, { escape: false })
+		case InvalidFilenameErrorReason.Extension:
+			if (error.segment.match(/\.[a-z]/i)) {
+				return t('files', '"{extension}" is not an allowed name.', { extension: error.segment }, undefined, { escape: false })
+			}
+			return t('files', 'Names must not end with "{extension}".', { extension: error.segment }, undefined, { escape: false })
+		default:
+			return t('files', 'Invalid name.')
+		}
+	}
+}

apps/files_sharing/src/views/PublicAuthPrompt.vue

@@ -42,7 +42,7 @@ import NcDialog from '@nextcloud/vue/components/NcDialog'
 import NcNoteCard from '@nextcloud/vue/components/NcNoteCard'
 import NcTextField from '@nextcloud/vue/components/NcTextField'
 
-import { getFilenameValidity } from '../../../files/src/utils/filenameValidity'
+import { getGuestNameValidity } from '../services/GuestNameValidity'
 
 export default defineComponent({
 	name: 'PublicAuthPrompt',
@@ -112,7 +112,7 @@ export default defineComponent({
 				return
 			}
 
-			const validity = getFilenameValidity(newName)
+			const validity = getGuestNameValidity(newName)
 			input.setCustomValidity(validity)
 			input.reportValidity()
 		},

build/integration/filesdrop_features/filesdrop.feature

@@ -44,7 +44,7 @@ Feature: FilesDrop
     And Updating last share with
       | permissions | 4 |
     When Dropping file "/folder/a.txt" with "abc"
-    Then the HTTP status code should be "405"
+    Then the HTTP status code should be "400"
 
   Scenario: Files drop forbid MKCOL without a nickname
     Given user "user0" exists
@@ -57,7 +57,7 @@ Feature: FilesDrop
     And Updating last share with
       | permissions | 4 |
     When Creating folder "folder" in drop
-    Then the HTTP status code should be "405"
+    Then the HTTP status code should be "400"
 
   Scenario: Files drop allows MKCOL with a nickname
     Given user "user0" exists
@@ -83,7 +83,7 @@ Feature: FilesDrop
     And Updating last share with
       | permissions | 4 |
     When dropping file "/folder/a.txt" with "abc"
-    Then the HTTP status code should be "405"
+    Then the HTTP status code should be "400"
 
   Scenario: Files request drop
     Given user "user0" exists
@@ -195,4 +195,43 @@ Feature: FilesDrop
       | attributes | [{"scope":"fileRequest","key":"enabled","value":true}] |
       | shareWith |  |
     When Dropping file "/folder/a.txt" with "abc"
-    Then the HTTP status code should be "405"
+    Then the HTTP status code should be "400"
+
+  Scenario: Files request drop with invalid nickname with slashes
+    Given user "user0" exists
+    And As an "user0"
+    And user "user0" created a folder "/drop"
+    And as "user0" creating a share with
+      | path | drop |
+      | shareType | 4 |
+      | permissions | 4 |
+      | attributes | [{"scope":"fileRequest","key":"enabled","value":true}] |
+      | shareWith |  |
+    When Dropping file "/folder/a.txt" with "abc" as "Alice/Bob/Mallory"
+    Then the HTTP status code should be "400"
+
+  Scenario: Files request drop with invalid nickname with forbidden characters
+    Given user "user0" exists
+    And As an "user0"
+    And user "user0" created a folder "/drop"
+    And as "user0" creating a share with
+      | path | drop |
+      | shareType | 4 |
+      | permissions | 4 |
+      | attributes | [{"scope":"fileRequest","key":"enabled","value":true}] |
+      | shareWith |  |
+    When Dropping file "/folder/a.txt" with "abc" as ".htaccess"
+    Then the HTTP status code should be "400"
+
+  Scenario: Files request drop with invalid nickname with forbidden characters
+    Given user "user0" exists
+    And As an "user0"
+    And user "user0" created a folder "/drop"
+    And as "user0" creating a share with
+      | path | drop |
+      | shareType | 4 |
+      | permissions | 4 |
+      | attributes | [{"scope":"fileRequest","key":"enabled","value":true}] |
+      | shareWith |  |
+    When Dropping file "/folder/a.txt" with "abc" as ".Mallory"
+    Then the HTTP status code should be "400"

lib/private/Files/Node/Folder.php

@@ -460,4 +460,12 @@ class Folder extends Node implements \OCP\Files\Folder {
 
 		return $this->search($query);
 	}
+
+	public function verifyPath($fileName, $readonly = false): void {
+		$this->view->verifyPath(
+			$this->getPath(),
+			$fileName,
+			$readonly,
+		);
+	}
 }

lib/private/Files/Node/LazyFolder.php

@@ -561,4 +561,8 @@ class LazyFolder implements Folder {
 	public function getMetadata(): array {
 		return $this->data['metadata'] ?? $this->__call(__FUNCTION__, func_get_args());
 	}
+
+	public function verifyPath($fileName, $readonly = false): void {
+		$this->__call(__FUNCTION__, func_get_args());
+	}
 }

lib/public/Files/Folder.php

@@ -199,4 +199,15 @@ interface Folder extends Node {
 	 * @since 9.1.0
 	 */
 	public function getRecent($limit, $offset = 0);
+
+	/**
+	 * Verify if the given path is valid and allowed from this folder.
+	 *
+	 * @param string $path the path from this folder
+	 * @param string $fileName
+	 * @param bool $readonly Check only if the path is allowed for read-only access
+	 * @throws InvalidPathException
+	 * @since 32.0.0
+	 */
+	public function verifyPath($fileName, $readonly = false): void;
 }