diff --git a/lib/Controller/WopiController.php b/lib/Controller/WopiController.php index 8f9593d892..df5f7d5715 100644 --- a/lib/Controller/WopiController.php +++ b/lib/Controller/WopiController.php @@ -594,7 +594,7 @@ public function putFile( if ($isPutRelative) { // the new file needs to be installed in the current user dir $userFolder = $this->rootFolder->getUserFolder($wopi->getEditorUid()); - $file = $userFolder->getFirstNodeById($fileId); + $file = $userFolder->getFirstNodeById((int)$fileId); if ($file === null) { return new JSONResponse([], Http::STATUS_NOT_FOUND); } diff --git a/lib/Helper.php b/lib/Helper.php index 3503449738..359bb3f0ef 100644 --- a/lib/Helper.php +++ b/lib/Helper.php @@ -1,5 +1,6 @@ 3) { + throw new \InvalidArgumentException('$fileId does not match the expected format'); } - if (str_contains($fileId, '-')) { - [$fileId, $templateId] = array_pad(explode('/', $fileId), 2, null); + $fileIdPart = $parts[0]; + $instanceId = $parts[1] ?? ''; + $version = $parts[2] ?? '0'; + $templateId = null; + + $hasTemplateMarker = str_contains($fileIdPart, '-'); + if ($hasTemplateMarker) { + [$fileIdPart, $templateId] = array_pad(explode('/', $fileIdPart, 2), 2, null); } - return [ - $fileId, - $instanceId, - $version, - $templateId - ]; + return [$fileIdPart, $instanceId, $version, $templateId]; } /** - * WOPI helper function to convert to ISO 8601 round-trip format. - * @param integer $time Must be seconds since unix epoch + * Convert a Unix timestamp to WOPI's ISO 8601 round-trip format. + * + * @param int $time Seconds since the Unix epoch. + * @return string|false Formatted timestamp, or false if conversion fails. */ - public static function toISO8601($time) { - // TODO: Be more precise and don't ignore milli, micro seconds ? - $datetime = DateTime::createFromFormat('U', $time, new DateTimeZone('UTC')); + public static function toISO8601(int $time): string|false { + // TODO: Be more precise and don't ignore milli, micro seconds? + // e.g. Accept float|DateTimeInterface to support sub-second precision + // (.u is always 000000 with int input). + $datetime = DateTime::createFromFormat('U', (string)$time, new DateTimeZone('UTC')); if ($datetime) { return $datetime->format('Y-m-d\TH:i:s.u\Z'); } @@ -70,24 +77,40 @@ public static function toISO8601($time) { return false; } - public static function getNewFileName(Folder $folder, $filename) { + /** + * Return a unique file name by appending an incrementing suffix if needed. + * + * @param Folder $folder Folder to check for name collisions. + * @param string $filename Proposed file name. + * @return string Collision-free file name. + * @throws \RuntimeException If regex/PCRE outright fails (unlikely). + */ + public static function getNewFileName(Folder $folder, string $filename): string { $fileNum = 1; while ($folder->nodeExists($filename)) { $fileNum++; - $filename = preg_replace('/(\.| \(\d+\)\.)([^.]*)$/', ' (' . $fileNum . ').$2', $filename); - } + $newFilename = preg_replace('/(\.| \(\d+\)\.)([^.]*)$/', ' (' . $fileNum . ').$2', $filename); - return $filename; - } + if ($newFilename === null) { + // unlikely unless regex is broken + throw new \RuntimeException('Failed to generate a unique filename'); + } - public function getGuestNameFromCookie() { - if ($this->userId !== null || !isset($_COOKIE['guestUser']) || $_COOKIE['guestUser'] === '') { - return null; + $filename = $newFilename; } - return $_COOKIE['guestUser']; + + return $filename; } + /** + * Resolve the share associated with a node from shared storage. + * + * TODO: Put this elsewhere. + * + * @param Node $node File node to inspect. + * @return IShare|null The share backing the node, or null if it is not shared. + */ public function getShareFromNode(Node $node): ?IShare { try { $storage = $node->getStorage(); diff --git a/lib/TokenManager.php b/lib/TokenManager.php index 10d5582ac7..3f28c1031c 100644 --- a/lib/TokenManager.php +++ b/lib/TokenManager.php @@ -1,5 +1,6 @@ getFirstNodeById($fileId); + $file = $userFolder->getFirstNodeById((int)$fileId); // Check node readability (for storage wrapper overwrites like terms of services) if ($file === null || !$file->isReadable()) { @@ -121,15 +129,50 @@ public function generateWopiToken(string $fileId, ?string $shareToken = null, ?s $this->eventDispatcher->dispatchTyped(new BeforeNodeReadEvent($file)); $serverHost = $this->urlGenerator->getAbsoluteURL('/'); - $guestName = $editoruid === null ? $this->prepareGuestName($this->helper->getGuestNameFromCookie()) : null; - return $this->wopiMapper->generateFileToken($fileId, $owneruid, $editoruid, $version, $updatable, $serverHost, $guestName, $hideDownload, $direct, 0, $shareToken); + + $guestName = $editoruid === null ? $this->prepareGuestName($this->getGuestNameFromCookie()) : null; + + return $this->wopiMapper->generateFileToken( + (int)$fileId, + $owneruid, + $editoruid, + $version, + $updatable, + $serverHost, + $guestName, + $hideDownload, + $direct, + 0, + $shareToken + ); + } + + private function getGuestNameFromCookie(): ?string { + // prevent a logged-in user from being treated as a guest via the cookie + if ($this->userId === null) { + return null; + } + + $guestUser = $this->request->getCookie('guestUser'); + + if ($guestUser === '' || $guestUser === null) { + return null; + } + + return $guestUser; } /** * This method is receiving the results from the TOKEN_TYPE_FEDERATION generated on the opener server * that is created in {@link newInitiatorToken} */ - public function upgradeToRemoteToken(Wopi $wopi, Wopi $remoteWopi, string $shareToken, string $remoteServer, string $remoteServerToken): Wopi { + public function upgradeToRemoteToken( + Wopi $wopi, + Wopi $remoteWopi, + string $shareToken, + string $remoteServer, + string $remoteServerToken, + ): Wopi { if ($remoteWopi->getTokenType() !== Wopi::TOKEN_TYPE_INITIATOR) { return $wopi; } @@ -150,7 +193,7 @@ public function upgradeToRemoteToken(Wopi $wopi, Wopi $remoteWopi, string $share return $wopi; } - public function upgradeFromDirectInitiator(Direct $direct, Wopi $wopi) { + public function upgradeFromDirectInitiator(Direct $direct, Wopi $wopi): Wopi { $wopi->setTokenType(Wopi::TOKEN_TYPE_REMOTE_GUEST); $wopi->setEditorUid(null); $wopi->setRemoteServer($direct->getInitiatorHost()); @@ -207,7 +250,13 @@ public function generateWopiTokenForTemplate( ); } - public function newInitiatorToken($sourceServer, ?Node $node = null, $shareToken = null, bool $direct = false, $userId = null): Wopi { + public function newInitiatorToken( + string $sourceServer, + ?Node $node = null, + ?string $shareToken = null, + bool $direct = false, + ?string $userId = null, + ): Wopi { if ($node !== null) { $wopi = $this->generateWopiToken((string)$node->getId(), $shareToken, $userId, $direct); $wopi->setServerHost($sourceServer); @@ -226,7 +275,7 @@ public function extendWithInitiatorUserToken(Wopi $wopi, string $initiatorUserHo return $wopi; } - public function prepareGuestName(?string $guestName = null) { + public function prepareGuestName(?string $guestName = null): string { if (empty($guestName)) { return $this->trans->t('Anonymous guest'); } @@ -244,13 +293,10 @@ public function prepareGuestName(?string $guestName = null) { } /** - * @param string $accessToken - * @param string $guestName - * @return void * @throws Exceptions\ExpiredTokenException * @throws Exceptions\UnknownTokenException */ - public function updateGuestName(string $accessToken, string $guestName) { + public function updateGuestName(string $accessToken, string $guestName): void { $wopi = $this->wopiMapper->getWopiForToken($accessToken); $wopi->setGuestDisplayname($this->prepareGuestName($guestName)); $this->wopiMapper->update($wopi); diff --git a/tests/psalm-baseline.xml b/tests/psalm-baseline.xml index 9e0fe758a0..f513d21fbe 100644 --- a/tests/psalm-baseline.xml +++ b/tests/psalm-baseline.xml @@ -80,11 +80,6 @@ - - - - -