From 81d3ea169192f3c48900bb3843c5a4082368dcbf Mon Sep 17 00:00:00 2001 From: provokateurin Date: Mon, 16 Dec 2024 16:18:40 +0100 Subject: [PATCH 1/2] fix(settings): Fix log file download return type Signed-off-by: provokateurin --- .../lib/Controller/LogSettingsController.php | 18 +++++++++--------- apps/settings/openapi.json | 5 ++++- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/apps/settings/lib/Controller/LogSettingsController.php b/apps/settings/lib/Controller/LogSettingsController.php index 62b51946af7d4..21bc4d0d624e4 100644 --- a/apps/settings/lib/Controller/LogSettingsController.php +++ b/apps/settings/lib/Controller/LogSettingsController.php @@ -48,9 +48,7 @@ public function __construct(string $appName, IRequest $request, Log $logger) { * * @NoCSRFRequired * - * @psalm-suppress MoreSpecificReturnType The value of Content-Disposition is not relevant - * @psalm-suppress LessSpecificReturnStatement The value of Content-Disposition is not relevant - * @return StreamResponse + * @return StreamResponse * * 200: Logfile returned */ @@ -58,11 +56,13 @@ public function download() { if (!$this->log instanceof Log) { throw new \UnexpectedValueException('Log file not available'); } - $resp = new StreamResponse($this->log->getLogPath()); - $resp->setHeaders([ - 'Content-Type' => 'application/octet-stream', - 'Content-Disposition' => 'attachment; filename="nextcloud.log"', - ]); - return $resp; + return new StreamResponse( + $this->log->getLogPath(), + Http::STATUS_OK, + [ + 'Content-Type' => 'application/octet-stream', + 'Content-Disposition' => 'attachment; filename="nextcloud.log"', + ], + ); } } diff --git a/apps/settings/openapi.json b/apps/settings/openapi.json index 217a0fae9f7f7..4a3b4d54d19ec 100644 --- a/apps/settings/openapi.json +++ b/apps/settings/openapi.json @@ -44,7 +44,10 @@ "headers": { "Content-Disposition": { "schema": { - "type": "string" + "type": "string", + "enum": [ + "attachment; filename=\"nextcloud.log\"" + ] } } }, From eeadabd5918f3dc219b13607ac81a33d617e115f Mon Sep 17 00:00:00 2001 From: provokateurin Date: Mon, 16 Dec 2024 16:20:48 +0100 Subject: [PATCH 2/2] fix(Http): Only allow valid HTTP status code values via template Signed-off-by: provokateurin --- lib/private/AppFramework/OCS/BaseResponse.php | 4 ++-- lib/private/AppFramework/OCS/V1Response.php | 6 +++--- lib/private/AppFramework/OCS/V2Response.php | 6 +++--- lib/public/AppFramework/Http/DataDisplayResponse.php | 4 ++-- lib/public/AppFramework/Http/DataDownloadResponse.php | 4 ++-- lib/public/AppFramework/Http/DataResponse.php | 4 ++-- lib/public/AppFramework/Http/DownloadResponse.php | 4 ++-- lib/public/AppFramework/Http/FileDisplayResponse.php | 4 ++-- lib/public/AppFramework/Http/JSONResponse.php | 4 ++-- lib/public/AppFramework/Http/NotFoundResponse.php | 4 ++-- lib/public/AppFramework/Http/RedirectResponse.php | 4 ++-- .../AppFramework/Http/RedirectToDefaultAppResponse.php | 4 ++-- lib/public/AppFramework/Http/Response.php | 2 +- lib/public/AppFramework/Http/StandaloneTemplateResponse.php | 6 ++++-- lib/public/AppFramework/Http/StreamResponse.php | 4 ++-- .../AppFramework/Http/Template/PublicTemplateResponse.php | 4 ++-- lib/public/AppFramework/Http/TemplateResponse.php | 4 ++-- lib/public/AppFramework/Http/TextPlainResponse.php | 4 ++-- lib/public/AppFramework/Http/TooManyRequestsResponse.php | 4 ++-- lib/public/AppFramework/Http/ZipResponse.php | 4 ++-- 20 files changed, 43 insertions(+), 41 deletions(-) diff --git a/lib/private/AppFramework/OCS/BaseResponse.php b/lib/private/AppFramework/OCS/BaseResponse.php index 4f460795e7f72..d82a3135da35c 100644 --- a/lib/private/AppFramework/OCS/BaseResponse.php +++ b/lib/private/AppFramework/OCS/BaseResponse.php @@ -33,10 +33,10 @@ /** * @psalm-import-type DataResponseType from DataResponse - * @template S of int + * @template S of Http::STATUS_* * @template-covariant T of DataResponseType * @template H of array - * @template-extends Response> + * @template-extends Response> */ abstract class BaseResponse extends Response { /** @var array */ diff --git a/lib/private/AppFramework/OCS/V1Response.php b/lib/private/AppFramework/OCS/V1Response.php index e6b0165278999..a3e571e32c8d6 100644 --- a/lib/private/AppFramework/OCS/V1Response.php +++ b/lib/private/AppFramework/OCS/V1Response.php @@ -31,17 +31,17 @@ /** * @psalm-import-type DataResponseType from DataResponse - * @template S of int + * @template S of Http::STATUS_* * @template-covariant T of DataResponseType * @template H of array - * @template-extends BaseResponse> + * @template-extends BaseResponse> */ class V1Response extends BaseResponse { /** * The V1 endpoint has very limited http status codes basically everything * is status 200 except 401 * - * @return int + * @return Http::STATUS_* */ public function getStatus() { $status = parent::getStatus(); diff --git a/lib/private/AppFramework/OCS/V2Response.php b/lib/private/AppFramework/OCS/V2Response.php index 1e81a3c7d938c..ade0de9dd26ea 100644 --- a/lib/private/AppFramework/OCS/V2Response.php +++ b/lib/private/AppFramework/OCS/V2Response.php @@ -30,17 +30,17 @@ /** * @psalm-import-type DataResponseType from DataResponse - * @template S of int + * @template S of Http::STATUS_* * @template-covariant T of DataResponseType * @template H of array - * @template-extends BaseResponse> + * @template-extends BaseResponse> */ class V2Response extends BaseResponse { /** * The V2 endpoint just passes on status codes. * Of course we have to map the OCS specific codes to proper HTTP status codes * - * @return int + * @return Http::STATUS_* */ public function getStatus() { $status = parent::getStatus(); diff --git a/lib/public/AppFramework/Http/DataDisplayResponse.php b/lib/public/AppFramework/Http/DataDisplayResponse.php index be2ade50bb551..98ef8db209088 100644 --- a/lib/public/AppFramework/Http/DataDisplayResponse.php +++ b/lib/public/AppFramework/Http/DataDisplayResponse.php @@ -31,9 +31,9 @@ * Class DataDisplayResponse * * @since 8.1.0 - * @template S of int + * @template S of Http::STATUS_* * @template H of array - * @template-extends Response> + * @template-extends Response> */ class DataDisplayResponse extends Response { /** diff --git a/lib/public/AppFramework/Http/DataDownloadResponse.php b/lib/public/AppFramework/Http/DataDownloadResponse.php index f70056047956b..f4ff55ab1e663 100644 --- a/lib/public/AppFramework/Http/DataDownloadResponse.php +++ b/lib/public/AppFramework/Http/DataDownloadResponse.php @@ -30,10 +30,10 @@ * Class DataDownloadResponse * * @since 8.0.0 - * @template S of int + * @template S of Http::STATUS_* * @template C of string * @template H of array - * @template-extends DownloadResponse> + * @template-extends DownloadResponse> */ class DataDownloadResponse extends DownloadResponse { /** diff --git a/lib/public/AppFramework/Http/DataResponse.php b/lib/public/AppFramework/Http/DataResponse.php index 1a56847d63d8e..1ff696a1f1c9a 100644 --- a/lib/public/AppFramework/Http/DataResponse.php +++ b/lib/public/AppFramework/Http/DataResponse.php @@ -32,10 +32,10 @@ * for responders to transform * @since 8.0.0 * @psalm-type DataResponseType = array|int|float|string|bool|object|null|\stdClass|\JsonSerializable - * @template S of int + * @template S of Http::STATUS_* * @template-covariant T of DataResponseType * @template H of array - * @template-extends Response> + * @template-extends Response> */ class DataResponse extends Response { /** diff --git a/lib/public/AppFramework/Http/DownloadResponse.php b/lib/public/AppFramework/Http/DownloadResponse.php index 5b3a235d4447a..a2c7b0fc0b13c 100644 --- a/lib/public/AppFramework/Http/DownloadResponse.php +++ b/lib/public/AppFramework/Http/DownloadResponse.php @@ -31,10 +31,10 @@ /** * Prompts the user to download the a file * @since 7.0.0 - * @template S of int + * @template S of Http::STATUS_* * @template C of string * @template H of array - * @template-extends Response> + * @template-extends Response> */ class DownloadResponse extends Response { /** diff --git a/lib/public/AppFramework/Http/FileDisplayResponse.php b/lib/public/AppFramework/Http/FileDisplayResponse.php index f194a23f1fea7..0b317238bfdfa 100644 --- a/lib/public/AppFramework/Http/FileDisplayResponse.php +++ b/lib/public/AppFramework/Http/FileDisplayResponse.php @@ -32,9 +32,9 @@ * Class FileDisplayResponse * * @since 11.0.0 - * @template S of int + * @template S of Http::STATUS_* * @template H of array - * @template-extends Response> + * @template-extends Response> */ class FileDisplayResponse extends Response implements ICallbackResponse { /** @var File|ISimpleFile */ diff --git a/lib/public/AppFramework/Http/JSONResponse.php b/lib/public/AppFramework/Http/JSONResponse.php index 4385e2ffd557b..a75beb2268f59 100644 --- a/lib/public/AppFramework/Http/JSONResponse.php +++ b/lib/public/AppFramework/Http/JSONResponse.php @@ -33,10 +33,10 @@ /** * A renderer for JSON calls * @since 6.0.0 - * @template S of int + * @template S of Http::STATUS_* * @template-covariant T of null|string|int|float|bool|array|\stdClass|\JsonSerializable * @template H of array - * @template-extends Response> + * @template-extends Response> */ class JSONResponse extends Response { /** diff --git a/lib/public/AppFramework/Http/NotFoundResponse.php b/lib/public/AppFramework/Http/NotFoundResponse.php index d6df0f6046749..df9af085bd932 100644 --- a/lib/public/AppFramework/Http/NotFoundResponse.php +++ b/lib/public/AppFramework/Http/NotFoundResponse.php @@ -30,9 +30,9 @@ /** * A generic 404 response showing an 404 error page as well to the end-user * @since 8.1.0 - * @template S of int + * @template S of Http::STATUS_* * @template H of array - * @template-extends TemplateResponse> + * @template-extends TemplateResponse> */ class NotFoundResponse extends TemplateResponse { /** diff --git a/lib/public/AppFramework/Http/RedirectResponse.php b/lib/public/AppFramework/Http/RedirectResponse.php index b69161959546f..4b5844a0f88ab 100644 --- a/lib/public/AppFramework/Http/RedirectResponse.php +++ b/lib/public/AppFramework/Http/RedirectResponse.php @@ -31,9 +31,9 @@ /** * Redirects to a different URL * @since 7.0.0 - * @template S of int + * @template S of Http::STATUS_* * @template H of array - * @template-extends Response> + * @template-extends Response> */ class RedirectResponse extends Response { private $redirectURL; diff --git a/lib/public/AppFramework/Http/RedirectToDefaultAppResponse.php b/lib/public/AppFramework/Http/RedirectToDefaultAppResponse.php index 7a1bfdbaf8fee..eda3f9a45d5d9 100644 --- a/lib/public/AppFramework/Http/RedirectToDefaultAppResponse.php +++ b/lib/public/AppFramework/Http/RedirectToDefaultAppResponse.php @@ -35,9 +35,9 @@ * * @since 16.0.0 * @deprecated 23.0.0 Use RedirectResponse() with IURLGenerator::linkToDefaultPageUrl() instead - * @template S of int + * @template S of Http::STATUS_* * @template H of array - * @template-extends RedirectResponse> + * @template-extends RedirectResponse> */ class RedirectToDefaultAppResponse extends RedirectResponse { /** diff --git a/lib/public/AppFramework/Http/Response.php b/lib/public/AppFramework/Http/Response.php index d28f45f4c60c7..cd768a683da0c 100644 --- a/lib/public/AppFramework/Http/Response.php +++ b/lib/public/AppFramework/Http/Response.php @@ -42,7 +42,7 @@ * * It handles headers, HTTP status code, last modified and ETag. * @since 6.0.0 - * @template S of int + * @template S of Http::STATUS_* * @template H of array */ class Response { diff --git a/lib/public/AppFramework/Http/StandaloneTemplateResponse.php b/lib/public/AppFramework/Http/StandaloneTemplateResponse.php index 8a39dca71e3f0..76f69c8486b74 100644 --- a/lib/public/AppFramework/Http/StandaloneTemplateResponse.php +++ b/lib/public/AppFramework/Http/StandaloneTemplateResponse.php @@ -26,6 +26,8 @@ */ namespace OCP\AppFramework\Http; +use OCP\AppFramework\Http; + /** * A template response that does not emit the loadAdditionalScripts events. * @@ -33,9 +35,9 @@ * full nextcloud UI. Like the 2FA page, or the grant page in the login flow. * * @since 16.0.0 - * @template S of int + * @template S of Http::STATUS_* * @template H of array - * @template-extends TemplateResponse> + * @template-extends TemplateResponse> */ class StandaloneTemplateResponse extends TemplateResponse { } diff --git a/lib/public/AppFramework/Http/StreamResponse.php b/lib/public/AppFramework/Http/StreamResponse.php index 14394383ba1bd..7b49e414ac6ef 100644 --- a/lib/public/AppFramework/Http/StreamResponse.php +++ b/lib/public/AppFramework/Http/StreamResponse.php @@ -33,9 +33,9 @@ * Class StreamResponse * * @since 8.1.0 - * @template S of int + * @template S of Http::STATUS_* * @template H of array - * @template-extends Response> + * @template-extends Response> */ class StreamResponse extends Response implements ICallbackResponse { /** @var string */ diff --git a/lib/public/AppFramework/Http/Template/PublicTemplateResponse.php b/lib/public/AppFramework/Http/Template/PublicTemplateResponse.php index def25d01c5193..da15faad8a0cf 100644 --- a/lib/public/AppFramework/Http/Template/PublicTemplateResponse.php +++ b/lib/public/AppFramework/Http/Template/PublicTemplateResponse.php @@ -34,8 +34,8 @@ * * @since 14.0.0 * @template H of array - * @template S of int - * @template-extends TemplateResponse> + * @template S of Http::STATUS_* + * @template-extends TemplateResponse> */ class PublicTemplateResponse extends TemplateResponse { private $headerTitle = ''; diff --git a/lib/public/AppFramework/Http/TemplateResponse.php b/lib/public/AppFramework/Http/TemplateResponse.php index 627906fcc203e..7edd2a4d51992 100644 --- a/lib/public/AppFramework/Http/TemplateResponse.php +++ b/lib/public/AppFramework/Http/TemplateResponse.php @@ -35,9 +35,9 @@ * Response for a normal template * @since 6.0.0 * - * @template S of int + * @template S of Http::STATUS_* * @template H of array - * @template-extends Response> + * @template-extends Response> */ class TemplateResponse extends Response { /** diff --git a/lib/public/AppFramework/Http/TextPlainResponse.php b/lib/public/AppFramework/Http/TextPlainResponse.php index 7bcd353e10212..830c8940e1168 100644 --- a/lib/public/AppFramework/Http/TextPlainResponse.php +++ b/lib/public/AppFramework/Http/TextPlainResponse.php @@ -31,9 +31,9 @@ /** * A renderer for text responses * @since 22.0.0 - * @template S of int + * @template S of Http::STATUS_* * @template H of array - * @template-extends Response> + * @template-extends Response> */ class TextPlainResponse extends Response { /** @var string */ diff --git a/lib/public/AppFramework/Http/TooManyRequestsResponse.php b/lib/public/AppFramework/Http/TooManyRequestsResponse.php index 688fb6cc38539..18ff4744cf868 100644 --- a/lib/public/AppFramework/Http/TooManyRequestsResponse.php +++ b/lib/public/AppFramework/Http/TooManyRequestsResponse.php @@ -32,9 +32,9 @@ /** * A generic 429 response showing an 404 error page as well to the end-user * @since 19.0.0 - * @template S of int + * @template S of Http::STATUS_* * @template H of array - * @template-extends Response> + * @template-extends Response> */ class TooManyRequestsResponse extends Response { /** diff --git a/lib/public/AppFramework/Http/ZipResponse.php b/lib/public/AppFramework/Http/ZipResponse.php index cd7f71f858d91..c1ecbe4759401 100644 --- a/lib/public/AppFramework/Http/ZipResponse.php +++ b/lib/public/AppFramework/Http/ZipResponse.php @@ -37,9 +37,9 @@ * Public library to send several files in one zip archive. * * @since 15.0.0 - * @template S of int + * @template S of Http::STATUS_* * @template H of array - * @template-extends Response> + * @template-extends Response> */ class ZipResponse extends Response implements ICallbackResponse { /** @var array{internalName: string, resource: resource, size: int, time: int}[] Files to be added to the zip response */