From bd27741099580c87f96051cd253f12f256110106 Mon Sep 17 00:00:00 2001 From: Josh Date: Sun, 31 May 2026 02:59:15 -0400 Subject: [PATCH 01/14] docs(IRequest): add/update based on implementation Signed-off-by: Josh --- lib/public/IRequest.php | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/lib/public/IRequest.php b/lib/public/IRequest.php index 913ecbdd4cd1d..f34bd2a4379df 100644 --- a/lib/public/IRequest.php +++ b/lib/public/IRequest.php @@ -6,8 +6,6 @@ * SPDX-FileCopyrightText: 2016 ownCloud, Inc. * SPDX-License-Identifier: AGPL-3.0-only */ -// use OCP namespace for all classes that are considered public. -// This means that they should be used by apps instead of the internal Nextcloud classes namespace OCP; @@ -92,11 +90,13 @@ interface IRequest { public const JSON_CONTENT_TYPE_REGEX = '/^application\/(?:[a-z0-9.-]+\+)?json\b/'; /** - * @param string $name + * Returns the value of a request header, or an empty string if missing. + * + * Also supports a few related server variables that are commonly available + * through the request environment. * * @psalm-taint-source input * - * @return string * @since 6.0.0 */ public function getHeader(string $name): string; @@ -122,7 +122,6 @@ public function getParam(string $key, $default = null); /** * Returns all params that were received, be it from the request - * * (as GET or POST) or through the URL by the route * * @psalm-taint-source input @@ -219,8 +218,12 @@ public function getId(): string; public function getRemoteAddress(): string; /** - * Returns the server protocol. It respects reverse proxy servers and load - * balancers. + * Returns the server protocol. It respects one or more reverse proxies servers + * and load balancers. Precedence: + * 1. `overwriteprotocol` config value + * 2. `X-Forwarded-Proto` header value + * 3. $_SERVER['HTTPS'] value + * If an invalid protocol is provided, defaults to http, continues, but logs as an error. * * @return string Server protocol (http or https) * @since 8.1.0 @@ -258,7 +261,7 @@ public function getRequestUri(): string; public function getRawPathInfo(): string; /** - * Get PathInfo from request + * Get PathInfo from request (rawurldecoded) * * @psalm-taint-source input * From 3313b5bc06e8260022a10e78fc2663c25d544b55 Mon Sep 17 00:00:00 2001 From: Josh Date: Sun, 31 May 2026 03:01:30 -0400 Subject: [PATCH 02/14] refactor(Request): drop docblocks already in IRequest Oh and slighly refactor getHeader() since this was tied to some other work that lead to this cleanup. Signed-off-by: Josh --- lib/private/AppFramework/Http/Request.php | 165 +++------------------- 1 file changed, 16 insertions(+), 149 deletions(-) diff --git a/lib/private/AppFramework/Http/Request.php b/lib/private/AppFramework/Http/Request.php index f3d4d221fa075..3c72d588b98e4 100644 --- a/lib/private/AppFramework/Http/Request.php +++ b/lib/private/AppFramework/Http/Request.php @@ -251,49 +251,30 @@ public function __unset($id) { throw new \RuntimeException('You cannot change the contents of the request object'); } - /** - * Returns the value for a specific http header. - * - * This method returns an empty string if the header did not exist. - * - * @param string $name - * @return string - */ #[\Override] public function getHeader(string $name): string { - $name = strtoupper(str_replace('-', '_', $name)); - if (isset($this->server['HTTP_' . $name])) { - return $this->server['HTTP_' . $name]; + $elementName = strtoupper(str_replace('-', '_', $name)); + + // Check if standard HTTP header + $clientHeaderKey = 'HTTP_' . $elementName; + if (isset($this->server[$clientHeaderKey])) { + return $this->server[$clientHeaderKey]; } - // There's a few headers that seem to end up in the top-level - // server array. - switch ($name) { - case 'CONTENT_TYPE': - case 'CONTENT_LENGTH': - case 'REMOTE_ADDR': - if (isset($this->server[$name])) { - return $this->server[$name]; - } - break; + // Check if special request-related element + $specialKeys = [ + 'CONTENT_TYPE' => true, + 'CONTENT_LENGTH' => true, + 'REMOTE_ADDR' = true, + ]; + + if (isset($specialKeys[$elementName]) && isset($this->server[$elementName])) { + return $this->server[$elementName]; } return ''; } - /** - * Lets you access post and get parameters by the index - * In case of json requests the encoded json body is accessed - * - * @param string $key the key which you want to access in the URL Parameter - * placeholder, $_POST or $_GET array. - * The priority how they're returned is the following: - * 1. URL parameters - * 2. POST parameters - * 3. GET parameters - * @param mixed $default If the key is not found, this value will be returned - * @return mixed the content of the array - */ #[\Override] public function getParam(string $key, $default = null) { return isset($this->parameters[$key]) @@ -301,50 +282,26 @@ public function getParam(string $key, $default = null) { : $default; } - /** - * Returns all params that were received, be it from the request - * (as GET or POST) or through the URL by the route - * @return array the array with all parameters - */ #[\Override] public function getParams(): array { return is_array($this->parameters) ? $this->parameters : []; } - /** - * Returns the method of the request - * @return string the method of the request (POST, GET, etc) - */ #[\Override] public function getMethod(): string { return $this->method; } - /** - * Shortcut for accessing an uploaded file through the $_FILES array - * @param string $key the key that will be taken from the $_FILES array - * @return array the file in the $_FILES element - */ #[\Override] public function getUploadedFile(string $key) { return isset($this->files[$key]) ? $this->files[$key] : null; } - /** - * Shortcut for getting env variables - * @param string $key the key that will be taken from the $_ENV array - * @return array the value in the $_ENV element - */ #[\Override] public function getEnv(string $key) { return isset($this->env[$key]) ? $this->env[$key] : null; } - /** - * Shortcut for getting cookie variables - * @param string $key the key that will be taken from the $_COOKIE array - * @return string the value in the $_COOKIE element - */ #[\Override] public function getCookie(string $key) { return isset($this->cookies[$key]) ? $this->cookies[$key] : null; @@ -435,11 +392,6 @@ public function throwDecodingExceptionIfAny(): void { } } - - /** - * Checks if the CSRF check was correct - * @return bool true if CSRF check passed - */ #[\Override] public function passesCSRFCheck(): bool { if ($this->csrfTokenManager === null) { @@ -471,8 +423,6 @@ public function passesCSRFCheck(): bool { /** * Whether the cookie checks are required - * - * @return bool */ private function cookieCheckRequired(): bool { if ($this->getHeader('OCS-APIREQUEST')) { @@ -487,8 +437,6 @@ private function cookieCheckRequired(): bool { /** * Wrapper around session_get_cookie_params - * - * @return array */ public function getCookieParams(): array { return session_get_cookie_params(); @@ -496,9 +444,6 @@ public function getCookieParams(): array { /** * Appends the __Host- prefix to the cookie if applicable - * - * @param string $name - * @return string */ protected function getProtectedCookieName(string $name): string { $cookieParams = $this->getCookieParams(); @@ -510,13 +455,6 @@ protected function getProtectedCookieName(string $name): string { return $prefix . $name; } - /** - * Checks if the strict cookie has been sent with the request if the request - * is including any cookies. - * - * @return bool - * @since 9.1.0 - */ #[\Override] public function passesStrictCookieCheck(): bool { if (!$this->cookieCheckRequired()) { @@ -531,13 +469,6 @@ public function passesStrictCookieCheck(): bool { return false; } - /** - * Checks if the lax cookie has been sent with the request if the request - * is including any cookies. - * - * @return bool - * @since 9.1.0 - */ #[\Override] public function passesLaxCookieCheck(): bool { if (!$this->cookieCheckRequired()) { @@ -551,12 +482,6 @@ public function passesLaxCookieCheck(): bool { return false; } - - /** - * Returns an ID for the request, value is not guaranteed to be unique and is mostly meant for logging - * If `mod_unique_id` is installed this value will be taken. - * @return string - */ #[\Override] public function getId(): string { return $this->requestId->getId(); @@ -578,13 +503,6 @@ protected function isTrustedProxy($trustedProxies, $remoteAddress) { } } - /** - * Returns the remote address, if the connection came from a trusted proxy - * and `forwarded_for_headers` has been configured then the IP address - * specified in this header will be returned instead. - * Do always use this instead of $_SERVER['REMOTE_ADDR'] - * @return string IP address - */ #[\Override] public function getRemoteAddress(): string { $remoteAddress = isset($this->server['REMOTE_ADDR']) ? $this->server['REMOTE_ADDR'] : ''; @@ -630,7 +548,6 @@ public function getRemoteAddress(): string { /** * Check overwrite condition - * @return bool */ private function isOverwriteCondition(): bool { $regex = '/' . $this->config->getSystemValueString('overwritecondaddr', '') . '/'; @@ -638,16 +555,6 @@ private function isOverwriteCondition(): bool { return $regex === '//' || preg_match($regex, $remoteAddr) === 1; } - /** - * Returns the server protocol. It respects one or more reverse proxies servers - * and load balancers. Precedence: - * 1. `overwriteprotocol` config value - * 2. `X-Forwarded-Proto` header value - * 3. $_SERVER['HTTPS'] value - * If an invalid protocol is provided, defaults to http, continues, but logs as an error. - * - * @return string Server protocol (http or https) - */ #[\Override] public function getServerProtocol(): string { $proto = 'http'; @@ -683,11 +590,6 @@ public function getServerProtocol(): string { return $proto === 'https' ? 'https' : 'http'; } - /** - * Returns the used HTTP protocol. - * - * @return string HTTP protocol. HTTP/2, HTTP/1.1 or HTTP/1.0. - */ #[\Override] public function getHttpProtocol(): string { $claimedProtocol = $this->server['SERVER_PROTOCOL'] ?? ''; @@ -709,11 +611,6 @@ public function getHttpProtocol(): string { return 'HTTP/1.1'; } - /** - * Returns the request uri, even if the website uses one or more - * reverse proxies - * @return string - */ #[\Override] public function getRequestUri(): string { $uri = isset($this->server['REQUEST_URI']) ? $this->server['REQUEST_URI'] : ''; @@ -723,11 +620,6 @@ public function getRequestUri(): string { return $uri; } - /** - * Get raw PathInfo from request (not urldecoded) - * @throws \Exception - * @return string Path info - */ #[\Override] public function getRawPathInfo(): string { $requestUri = isset($this->server['REQUEST_URI']) ? $this->server['REQUEST_URI'] : ''; @@ -769,22 +661,12 @@ public function getRawPathInfo(): string { } } - /** - * Get PathInfo from request (rawurldecoded) - * @throws \Exception - * @return string|false Path info or false when not found - */ #[\Override] public function getPathInfo(): string|false { $pathInfo = $this->getRawPathInfo(); return \Sabre\HTTP\decodePath($pathInfo); } - /** - * Returns the script name, even if the website uses one or more - * reverse proxies - * @return string the script name - */ #[\Override] public function getScriptName(): string { $name = $this->server['SCRIPT_NAME'] ?? ''; @@ -798,11 +680,6 @@ public function getScriptName(): string { return $name; } - /** - * Checks whether the user agent matches a given regex - * @param array $agent array of agent names - * @return bool true if at least one of the given agent matches, false otherwise - */ #[\Override] public function isUserAgent(array $agent): bool { if (!isset($this->server['HTTP_USER_AGENT'])) { @@ -816,11 +693,6 @@ public function isUserAgent(array $agent): bool { return false; } - /** - * Returns the unverified server host from the headers without checking - * whether it is a trusted domain - * @return string Server host - */ #[\Override] public function getInsecureServerHost(): string { if ($this->fromTrustedProxy() && $this->getOverwriteHost() !== null) { @@ -846,12 +718,6 @@ public function getInsecureServerHost(): string { return $host; } - - /** - * Returns the server host from the headers, or the first configured - * trusted domain if the host isn't in the trusted list - * @return string Server host - */ #[\Override] public function getServerHost(): string { // overwritehost is always trusted @@ -882,6 +748,7 @@ public function getServerHost(): string { /** * Returns the overwritehost setting from the config if set and * if the overwrite condition is met + * * @return string|null overwritehost value or null if not defined or the defined condition * isn't met */ From bda3209894a6f62b6c37a8035cfb6b16985150ff Mon Sep 17 00:00:00 2001 From: Josh Date: Sun, 31 May 2026 10:17:42 -0400 Subject: [PATCH 03/14] docs(IRequest): improve the accuracy and clarity class docblock Signed-off-by: Josh --- lib/public/IRequest.php | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/lib/public/IRequest.php b/lib/public/IRequest.php index f34bd2a4379df..596dbbe7086a8 100644 --- a/lib/public/IRequest.php +++ b/lib/public/IRequest.php @@ -10,26 +10,27 @@ namespace OCP; /** - * This interface provides an immutable object with with accessors to - * request variables and headers. + * Immutable request wrapper with accessors for request variables and other + * request-related data. + * + * Request data should be retrieved through this interface whenever possible. * - * Access request variables by method and name. + * Parameters can be accessed through dedicated methods or via magic property + * access, for example: * - * Examples: + * $request->post['myvar']; // POST body parameters on POST requests + * $request->myvar; // merged request parameters * - * $request->post['myvar']; // Only look for POST variables - * $request->myvar; or $request->{'myvar'}; or $request->{$myvar} - * Looks in the combined GET, POST and urlParams array. + * Magic access to a named parameter reads from the merged request parameter + * set. Method-specific properties such as `get`, `post`, `put`, and `patch` + * are only available for the matching HTTP method and may throw a + * \LogicException otherwise. * - * If you access e.g. ->post but the current HTTP request method - * is GET a \LogicException will be thrown. - * - * NOTE: - * - When accessing ->put a stream resource is returned and the accessor - * will return false on subsequent access to ->put or ->patch. - * - When accessing ->patch and the Content-Type is either application/json - * or application/x-www-form-urlencoded (most cases) it will act like ->get - * and ->post and return an array. Otherwise the raw data will be returned. + * In PUT requests, if the body is JSON or form-encoded, `->put` behaves like + * the other method-specific accessors and returns parsed request parameters. + * Otherwise, for non-empty request bodies, it returns a readable stream + * resource for the raw request body. Such streamed PUT bodies can only be + * accessed once; repeated access throws a \LogicException. * * @property-read string[] $server * @property-read string[] $urlParams From fcc20b56f9a651574c8bf59762b9ab9629cb3d8d Mon Sep 17 00:00:00 2001 From: Josh Date: Sun, 31 May 2026 11:47:43 -0400 Subject: [PATCH 04/14] docs(Request): update implementation docblock properties Signed-off-by: Josh --- lib/private/AppFramework/Http/Request.php | 24 +++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/lib/private/AppFramework/Http/Request.php b/lib/private/AppFramework/Http/Request.php index 3c72d588b98e4..9adbfd122fbc5 100644 --- a/lib/private/AppFramework/Http/Request.php +++ b/lib/private/AppFramework/Http/Request.php @@ -19,16 +19,20 @@ use Symfony\Component\HttpFoundation\IpUtils; /** - * Class for accessing variables in the request. - * This class provides an immutable object with request variables. + * Default immutable IRequest implementation. * - * @property mixed[] $cookies - * @property mixed[] $env - * @property mixed[] $files - * @property string $method - * @property mixed[] $parameters - * @property mixed[] $server - * @template-implements \ArrayAccess + * @property-read array $get + * @property-read array $post + * @property-read array|resource $put + * @property-read array $patch + * @property-read string $method + * @property-read array $server + * @property-read array $urlParams + * @property-read array $cookies + * @property-read array $env + * @property-read array $files + * @property-read array $parameters + * @template-implements \ArrayAccess */ class Request implements \ArrayAccess, \Countable, IRequest { public const USER_AGENT_IE = '/(MSIE)|(Trident)/'; @@ -265,7 +269,7 @@ public function getHeader(string $name): string { $specialKeys = [ 'CONTENT_TYPE' => true, 'CONTENT_LENGTH' => true, - 'REMOTE_ADDR' = true, + 'REMOTE_ADDR' => true, ]; if (isset($specialKeys[$elementName]) && isset($this->server[$elementName])) { From 0a551a9613a92ec00cd49dddad585ec7d9022e40 Mon Sep 17 00:00:00 2001 From: Josh Date: Sun, 31 May 2026 11:48:19 -0400 Subject: [PATCH 05/14] docs(IRequest): add missing magic properties to docblock Signed-off-by: Josh --- lib/public/IRequest.php | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/lib/public/IRequest.php b/lib/public/IRequest.php index 596dbbe7086a8..4e344e7d1985f 100644 --- a/lib/public/IRequest.php +++ b/lib/public/IRequest.php @@ -32,8 +32,14 @@ * resource for the raw request body. Such streamed PUT bodies can only be * accessed once; repeated access throws a \LogicException. * - * @property-read string[] $server + * @property-read array $get + * @property-read array $post + * @property-read array|resource $put + * @property-read array $patch + * @property-read string $method + * @property-read array $server * @property-read string[] $urlParams + * * @since 6.0.0 */ interface IRequest { @@ -93,8 +99,10 @@ interface IRequest { /** * Returns the value of a request header, or an empty string if missing. * - * Also supports a few related server variables that are commonly available - * through the request environment. + * Header names are matched case-insensitively. + * + * Besides normal HTTP headers, also supports selected request-related + * server values such as `REMOTE_ADDR`. * * @psalm-taint-source input * From de0cd2a32867c6a47efe73c6686e90bdca144a6e Mon Sep 17 00:00:00 2001 From: Josh Date: Sun, 31 May 2026 11:58:32 -0400 Subject: [PATCH 06/14] docs(IRequest): correct inaccuracies and improve clarity of request parameter set methods Signed-off-by: Josh --- lib/public/IRequest.php | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/lib/public/IRequest.php b/lib/public/IRequest.php index 4e344e7d1985f..70b1c8921386e 100644 --- a/lib/public/IRequest.php +++ b/lib/public/IRequest.php @@ -111,31 +111,30 @@ interface IRequest { public function getHeader(string $name): string; /** - * Lets you access post and get parameters by the index - * In case of json requests the encoded json body is accessed + * Returns a parameter value from the merged parameter set. + * + * The merged parameter set is primarily composed from route URL parameters, + * POST parameters and GET parameters. Depending on request content type and + * prior access, lazily decoded request-body parameters may also be present. * * @psalm-taint-source input * - * @param string $key the key which you want to access in the URL Parameter - * placeholder, $_POST or $_GET array. - * The priority how they're returned is the following: - * 1. URL parameters - * 2. POST parameters - * 3. GET parameters - * @param mixed $default If the key is not found, this value will be returned - * @return mixed the content of the array + * @param string $key the key to look up + * @param mixed $default the value to return if the key is not found + * @return mixed the parameter value, or $default if the key is not present * @since 6.0.0 */ public function getParam(string $key, $default = null); - /** - * Returns all params that were received, be it from the request - * (as GET or POST) or through the URL by the route + * Returns the merged parameter set currently available on the request. + * + * This includes request parameters from GET, POST and route URL parameters, + * and may also include decoded request-body parameters. * * @psalm-taint-source input * - * @return array the array with all parameters + * @return array the merged parameters * @since 6.0.0 */ public function getParams(): array; From a635445b22a9407db32101823e19ab7fb5f0acd3 Mon Sep 17 00:00:00 2001 From: Josh Date: Sun, 31 May 2026 12:04:34 -0400 Subject: [PATCH 07/14] docs(IRequest): minor wording improvements to some of the simpler retriever methods Signed-off-by: Josh --- lib/public/IRequest.php | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/lib/public/IRequest.php b/lib/public/IRequest.php index 70b1c8921386e..f69073217f2cf 100644 --- a/lib/public/IRequest.php +++ b/lib/public/IRequest.php @@ -140,40 +140,38 @@ public function getParam(string $key, $default = null); public function getParams(): array; /** - * Returns the method of the request + * Returns the request method. * - * @return string the method of the request (POST, GET, etc) + * @return string the HTTP method, for example GET, POST, PUT, or PATCH * @since 6.0.0 */ public function getMethod(): string; /** - * Shortcut for accessing an uploaded file through the $_FILES array + * Returns an uploaded file entry from the `$_FILES` data, if present. * - * @param string $key the key that will be taken from the $_FILES array - * @return array the file in the $_FILES element + * @param string $key the file field name + * @return array|null the matching uploaded file entry, or null if missing * @since 6.0.0 */ public function getUploadedFile(string $key); - /** - * Shortcut for getting env variables + * Returns an environment value from the request environment, if present. * - * @param string $key the key that will be taken from the $_ENV array - * @return array the value in the $_ENV element + * @param string $key the environment variable name + * @return mixed|null the environment value, or null if missing * @since 6.0.0 */ public function getEnv(string $key); - /** - * Shortcut for getting cookie variables + * Returns a cookie value, if present. * * @psalm-taint-source input * - * @param string $key the key that will be taken from the $_COOKIE array - * @return string|null the value in the $_COOKIE element + * @param string $key the cookie name + * @return string|null the cookie value, or null if missing * @since 6.0.0 */ public function getCookie(string $key); From 96432302bed85331afc55331c413ff25ead8baa3 Mon Sep 17 00:00:00 2001 From: Josh Date: Sun, 31 May 2026 12:11:39 -0400 Subject: [PATCH 08/14] docs(IRequest): improve same-site cookie method docblocks Signed-off-by: Josh --- lib/public/IRequest.php | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/lib/public/IRequest.php b/lib/public/IRequest.php index f69073217f2cf..bb3c6e4ec4546 100644 --- a/lib/public/IRequest.php +++ b/lib/public/IRequest.php @@ -176,29 +176,33 @@ public function getEnv(string $key); */ public function getCookie(string $key); - /** - * Checks if the CSRF check was correct + * Checks whether the request passes CSRF validation. + * + * Depending on the request, this may include same-site cookie checks and + * token validation from request parameters or headers. OCS API requests are + * handled specially by the implementation (if the OCS-APIRequest header is + * included in the request). * - * @return bool true if CSRF check passed + * @return bool true if the request passes CSRF validation * @since 6.0.0 */ public function passesCSRFCheck(): bool; /** - * Checks if the strict cookie has been sent with the request if the request - * is including any cookies. + * Checks whether the strict same-site cookie requirement is satisfied when + * session or authentication cookies are part of the request. * - * @return bool + * @return bool true if the strict cookie check passes * @since 9.0.0 */ public function passesStrictCookieCheck(): bool; /** - * Checks if the lax cookie has been sent with the request if the request - * is including any cookies. + * Checks whether the lax same-site cookie requirement is satisfied when + * session or authentication cookies are part of the request. * - * @return bool + * @return bool true if the lax cookie check passes * @since 9.0.0 */ public function passesLaxCookieCheck(): bool; From 14d9019155362630436a5e647651be670c7ea7fb Mon Sep 17 00:00:00 2001 From: Josh Date: Sun, 31 May 2026 12:26:23 -0400 Subject: [PATCH 09/14] docs(IRequest): update remaining method docblocks for accuracy and clarity Signed-off-by: Josh --- lib/public/IRequest.php | 87 ++++++++++++++++++++++++----------------- 1 file changed, 52 insertions(+), 35 deletions(-) diff --git a/lib/public/IRequest.php b/lib/public/IRequest.php index bb3c6e4ec4546..aa9b6ed642738 100644 --- a/lib/public/IRequest.php +++ b/lib/public/IRequest.php @@ -208,8 +208,10 @@ public function passesStrictCookieCheck(): bool; public function passesLaxCookieCheck(): bool; /** - * Returns an ID for the request, value is not guaranteed to be unique and is mostly meant for logging - * If `mod_unique_id` is installed this value will be taken. + * Returns a request identifier intended primarily for logging and tracing. + * + * The value is not guaranteed to be globally unique. If `mod_unique_id` is + * installed, that value may be used by the implementation. * * @return string * @since 8.1.0 @@ -217,10 +219,14 @@ public function passesLaxCookieCheck(): bool; public function getId(): string; /** - * Returns the remote address, if the connection came from a trusted proxy - * and `forwarded_for_headers` has been configured then the IP address - * specified in this header will be returned instead. - * Do always use this instead of $_SERVER['REMOTE_ADDR'] + * Returns the effective remote IP address. + * + * If the connection comes from a trusted proxy and `forwarded_for_headers` + * is configured, the client IP from those forwarded headers is used + * instead. + * + * Do not use `$_SERVER['REMOTE_ADDR']` directly when this method is + * available. * * @return string IP address * @since 8.1.0 @@ -228,29 +234,31 @@ public function getId(): string; public function getRemoteAddress(): string; /** - * Returns the server protocol. It respects one or more reverse proxies servers - * and load balancers. Precedence: + * Returns the effective server protocol. + * + * Respects reverse proxies and load balancers. Precedence: * 1. `overwriteprotocol` config value * 2. `X-Forwarded-Proto` header value - * 3. $_SERVER['HTTPS'] value - * If an invalid protocol is provided, defaults to http, continues, but logs as an error. + * 3. `$_SERVER['HTTPS']` value * - * @return string Server protocol (http or https) + * Invalid values fall back to `http`. + * + * @return string Server protocol: `http` or `https` * @since 8.1.0 */ public function getServerProtocol(): string; /** - * Returns the used HTTP protocol. + * Returns the HTTP protocol version used for the request. * - * @return string HTTP protocol. HTTP/2, HTTP/1.1 or HTTP/1.0. + * @return string HTTP protocol, for example HTTP/2, HTTP/1.1, or HTTP/1.0 * @since 8.2.0 */ public function getHttpProtocol(): string; /** - * Returns the request uri, even if the website uses one or more - * reverse proxies + * Returns the request URI, taking reverse-proxy and overwrite settings into + * account. * * @psalm-taint-source input * @@ -260,30 +268,30 @@ public function getHttpProtocol(): string; public function getRequestUri(): string; /** - * Get raw PathInfo from request (not urldecoded) + * Returns raw path info from the request without URL decoding. * * @psalm-taint-source input * * @throws \Exception - * @return string Path info + * @return string path info * @since 8.1.0 */ public function getRawPathInfo(): string; /** - * Get PathInfo from request (rawurldecoded) + * Returns decoded path info from the request. * * @psalm-taint-source input * * @throws \Exception - * @return string|false Path info or false when not found + * @return string|false path info, or false when it cannot be determined * @since 8.1.0 */ public function getPathInfo(); /** - * Returns the script name, even if the website uses one or more - * reverse proxies + * Returns the effective script name, taking reverse-proxy and overwrite + * settings into account. * * @return string the script name * @since 8.1.0 @@ -291,38 +299,46 @@ public function getPathInfo(); public function getScriptName(): string; /** - * Checks whether the user agent matches a given regex + * Checks whether the current user agent matches at least one of the given + * regular expressions. * - * @param array $agent array of agent names - * @return bool true if at least one of the given agent matches, false otherwise + * @param array $agent array of user-agent regex patterns + * @return bool true if at least one pattern matches, false otherwise * @since 8.1.0 */ public function isUserAgent(array $agent): bool; /** - * Returns the unverified server host from the headers without checking - * whether it is a trusted domain + * Returns the effective host value without validating it against the trusted + * domains configuration. + * + * This may be derived from request headers, proxy headers, or server + * variables, depending on the deployment setup. * * @psalm-taint-source input * - * @return string Server host + * @return string server host * @since 8.1.0 */ public function getInsecureServerHost(): string; /** - * Returns the server host from the headers, or the first configured - * trusted domain if the host isn't in the trusted list + * Returns the validated effective server host. * - * @return string Server host + * The implementation may use overwrite host configuration first. Otherwise + * it derives the host from the request and returns it only if it is trusted; + * if not, it falls back to the first configured trusted domain. + * + * @return string server host * @since 8.1.0 */ public function getServerHost(): string; /** - * If decoding the request content failed, throw an exception. - * Currently only \JsonException for json decoding errors, - * but in the future may throw other exceptions for other decoding issues. + * Throws any stored request-content decoding exception. + * + * Currently this is used for JSON decoding errors, but implementations may + * throw other decoding-related exceptions in the future. * * @throws \Exception * @since 32.0.0 @@ -330,9 +346,10 @@ public function getServerHost(): string; public function throwDecodingExceptionIfAny(): void; /** - * Returns the format of the response to this request. + * Returns the requested response format, if it can be determined. * - * The `Accept` header and the `format` query parameter control the format. + * The `format` request parameter takes precedence. Otherwise the format may + * be inferred from the `Accept` header. * * @return string|null * @since 33.0.0 From 14cd982529a288eabd443d6820fc8b9d82bb3b00 Mon Sep 17 00:00:00 2001 From: Josh Date: Sun, 31 May 2026 12:42:37 -0400 Subject: [PATCH 10/14] chore(RequestId): drop duplicate getId docblock from implementation Signed-off-by: Josh --- lib/private/AppFramework/Http/RequestId.php | 5 ----- 1 file changed, 5 deletions(-) diff --git a/lib/private/AppFramework/Http/RequestId.php b/lib/private/AppFramework/Http/RequestId.php index 45dd2246be6cd..96646660fe6f9 100644 --- a/lib/private/AppFramework/Http/RequestId.php +++ b/lib/private/AppFramework/Http/RequestId.php @@ -17,11 +17,6 @@ public function __construct( ) { } - /** - * Returns an ID for the request, value is not guaranteed to be unique and is mostly meant for logging - * If `mod_unique_id` is installed this value will be taken. - * @return string - */ #[\Override] public function getId(): string { if (empty($this->requestId)) { From 26b28ea8db91505d9208cf8fd7bc7a984ef710cc Mon Sep 17 00:00:00 2001 From: Josh Date: Sun, 31 May 2026 12:43:47 -0400 Subject: [PATCH 11/14] docs(IRequestId): update to consistency with wrapped call in IRequest Signed-off-by: Josh --- lib/public/IRequestId.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/public/IRequestId.php b/lib/public/IRequestId.php index 7c5a57716100d..d5e8251400f45 100644 --- a/lib/public/IRequestId.php +++ b/lib/public/IRequestId.php @@ -11,9 +11,12 @@ * @since 24.0.0 */ interface IRequestId { + /** - * Returns an ID for the request, value is not guaranteed to be unique and is mostly meant for logging - * If `mod_unique_id` is installed this value will be taken. + * Returns a request identifier intended primarily for logging and tracing. + * + * The value is not guaranteed to be globally unique. If `mod_unique_id` is + * installed, that value may be used by the implementation. * * @return string * @since 24.0.0 From 9cd1557a9285ad360902a0266d23186342fd1dc5 Mon Sep 17 00:00:00 2001 From: Josh Date: Sun, 31 May 2026 12:59:55 -0400 Subject: [PATCH 12/14] docs(Request): add/update/cleanup internal implementation docs Signed-off-by: Josh --- lib/private/AppFramework/Http/Request.php | 154 ++++++++++++---------- 1 file changed, 88 insertions(+), 66 deletions(-) diff --git a/lib/private/AppFramework/Http/Request.php b/lib/private/AppFramework/Http/Request.php index 9adbfd122fbc5..4aeccdbdc0425 100644 --- a/lib/private/AppFramework/Http/Request.php +++ b/lib/private/AppFramework/Http/Request.php @@ -48,9 +48,24 @@ class Request implements \ArrayAccess, \Countable, IRequest { // Android Chrome user agent: https://developers.google.com/chrome/mobile/docs/user-agent public const USER_AGENT_ANDROID_MOBILE_CHROME = '#Android.*Chrome/[.0-9]*#'; public const USER_AGENT_FREEBOX = '#^Mozilla/5\.0$#'; + public const REGEX_LOCALHOST = '/^(127\.0\.0\.1|localhost|\[::1\])$/'; + + /** + * Whether the raw PUT body stream has already been returned. + */ private bool $isPutStreamContentAlreadySent = false; + + /** + * Internal request data store. + */ protected array $items = []; + + /** + * Magic properties that are exposed directly from $items. + * + * @var list + */ protected array $allowedKeys = [ 'get', 'post', @@ -64,24 +79,27 @@ class Request implements \ArrayAccess, \Countable, IRequest { 'requesttoken', ]; + /** + * Whether request-body decoding has already been attempted. + */ protected bool $contentDecoded = false; + + /** + * Deferred decoding error from the request body, if any. + */ private ?\JsonException $decodingException = null; /** - * @param array $vars An associative array with the following optional values: - * - array 'urlParams' the parameters which were matched from the URL + * @param array $vars Associative request data with the following optional keys: + * - array 'urlParams' route parameters extracted from the URL * - array 'get' the $_GET array - * - array|string 'post' the $_POST array or JSON string + * - array 'post' the $_POST array * - array 'files' the $_FILES array * - array 'server' the $_SERVER array * - array 'env' the $_ENV array * - array 'cookies' the $_COOKIE array - * - string 'method' the request method (GET, POST etc) - * - string|false 'requesttoken' the requesttoken or false when not available - * @param IRequestId $requestId - * @param IConfig $config - * @param CsrfTokenManager|null $csrfTokenManager - * @param string $inputStream + * - string 'method' the HTTP request method, for example GET or POST + * - string|false 'requesttoken' the request token, or false if unavailable * @see https://www.php.net/manual/en/reserved.variables.php */ public function __construct( @@ -108,8 +126,16 @@ public function __construct( $this->items['params'] ); } + /** + * Replaces the current URL parameters and merges them into the parameter set. + * + * URL parameters take precedence over previously merged values with the same + * key. + * * @param array $parameters + * + * @internal public only so it can be consumed by OC\AppFramework\App */ public function setUrlParameters(array $parameters) { $this->items['urlParams'] = $parameters; @@ -120,8 +146,7 @@ public function setUrlParameters(array $parameters) { } /** - * Countable method - * @return int + * Returns the number of merged request parameters. */ #[\Override] public function count(): int { @@ -129,24 +154,12 @@ public function count(): int { } /** - * ArrayAccess methods + * Whether a merged request parameter exists. * - * Gives access to the combined GET, POST and urlParams arrays + * ArrayAccess operates on the merged parameter set. * - * Examples: - * - * $var = $request['myvar']; - * - * or - * - * if(!isset($request['myvar']) { - * // Do something - * } - * - * $request['myvar'] = 'something'; // This throws an exception. - * - * @param string $offset The key to lookup - * @return boolean + * @param string $offset Parameter name + * @return bool */ #[\Override] public function offsetExists($offset): bool { @@ -154,8 +167,9 @@ public function offsetExists($offset): bool { } /** - * @see offsetExists - * @param string $offset + * Returns a merged request parameter value, or null if it is missing. + * + * @param string $offset Parameter name * @return mixed */ #[\Override] @@ -165,7 +179,8 @@ public function offsetGet($offset) { } /** - * @see offsetExists + * Request objects are immutable. + * * @param string $offset * @param mixed $value */ @@ -175,7 +190,8 @@ public function offsetSet($offset, $value): void { } /** - * @see offsetExists + * Request objects are immutable. + * * @param string $offset */ #[\Override] @@ -184,7 +200,8 @@ public function offsetUnset($offset): void { } /** - * Magic property accessors + * Request objects are immutable. + * * @param string $name * @param mixed $value */ @@ -193,17 +210,16 @@ public function __set($name, $value) { } /** - * Access request variables by method and name. - * Examples: + * Returns request data through magic property access. * - * $request->post['myvar']; // Only look for POST variables - * $request->myvar; or $request->{'myvar'}; or $request->{$myvar} - * Looks in the combined GET, POST and urlParams array. + * Named properties read from the merged parameter set. Method-specific + * properties (`get`, `post`, `put`, `patch`) are only available for the + * matching HTTP method and throw a \LogicException otherwise. * - * If you access e.g. ->post but the current HTTP request method - * is GET a \LogicException will be thrown. + * Depending on the method and content type, `put` may return either parsed + * parameters or a readable stream for the raw request body. * - * @param string $name The key to look for. + * @param string $name Property name * @throws \LogicException * @return mixed|null */ @@ -238,6 +254,8 @@ public function __get($name) { } /** + * Whether a magic property is available. + * * @param string $name * @return bool */ @@ -249,6 +267,8 @@ public function __isset($name) { } /** + * Request objects are immutable. + * * @param string $id */ public function __unset($id) { @@ -312,18 +332,17 @@ public function getCookie(string $key) { } /** - * Returns the request body content. + * Returns request body content for method-specific magic accessors. * - * If the HTTP request method is PUT and the body - * not application/x-www-form-urlencoded or application/json a stream - * resource is returned, otherwise an array. - * - * @return array|string|resource The request body content or a resource to read the body stream. + * For PUT requests with a non-empty body that is neither JSON nor + * form-encoded, a readable stream resource for the raw body is returned. + * Otherwise, parsed parameters are returned as an array. * + * @return array|string|resource The request body content or a resource for the raw body stream * @throws \LogicException */ protected function getContent() { - // If the content can't be parsed into an array then return a stream resource. + // If the content cannot be parsed into parameters, return a raw body stream. if ($this->isPutStreamContent()) { if ($this->isPutStreamContentAlreadySent) { throw new \LogicException( @@ -348,7 +367,13 @@ private function isPutStreamContent(): bool { } /** - * Attempt to decode the content and populate parameters + * Decodes the request body, if applicable, and merges decoded parameters + * into the parameter set. + * + * JSON-compatible content types are decoded from the input stream. For + * non-GET and non-POST form-encoded requests, the input stream is parsed + * into parameters. Decoding errors are stored and can later be rethrown via + * throwDecodingExceptionIfAny(). */ protected function decodeContent() { if ($this->contentDecoded) { @@ -356,7 +381,7 @@ protected function decodeContent() { } $params = []; - // 'application/json' and other JSON-related content types must be decoded manually. + // JSON-compatible content types must be decoded manually. if (preg_match(self::JSON_CONTENT_TYPE_REGEX, $this->getHeader('Content-Type')) === 1) { $content = file_get_contents($this->inputStream); if ($content !== '') { @@ -372,8 +397,7 @@ protected function decodeContent() { $this->items['post'] = $params; } } - // Handle application/x-www-form-urlencoded for methods other than GET - // or post correctly + // Handle form-encoded request bodies for methods other than GET and POST. } elseif ($this->method !== 'GET' && $this->method !== 'POST' && str_contains($this->getHeader('Content-Type'), 'application/x-www-form-urlencoded')) { @@ -426,7 +450,7 @@ public function passesCSRFCheck(): bool { } /** - * Whether the cookie checks are required + * Whether cookie-based same-site checks are required for this request. */ private function cookieCheckRequired(): bool { if ($this->getHeader('OCS-APIREQUEST')) { @@ -440,14 +464,14 @@ private function cookieCheckRequired(): bool { } /** - * Wrapper around session_get_cookie_params + * Wrapper around session_get_cookie_params(). */ public function getCookieParams(): array { return session_get_cookie_params(); } /** - * Appends the __Host- prefix to the cookie if applicable + * Returns the cookie name with the __Host- prefix applied when appropriate. */ protected function getProtectedCookieName(string $name): string { $cookieParams = $this->getCookieParams(); @@ -492,16 +516,19 @@ public function getId(): string { } /** - * Checks if given $remoteAddress matches any entry in the given array $trustedProxies. - * For details regarding what "match" means, refer to `matchesTrustedProxy`. - * @return boolean true if $remoteAddress matches any entry in $trustedProxies, false otherwise + * Checks whether the given remote address matches one of the configured + * trusted proxies. + * + * Invalid trusted proxy configuration is treated as non-matching. + * + * @return bool true if $remoteAddress matches a trusted proxy, false otherwise */ protected function isTrustedProxy($trustedProxies, $remoteAddress) { try { return IpUtils::checkIp($remoteAddress, $trustedProxies); } catch (\Throwable) { - // We can not log to our log here as the logger is using `getRemoteAddress` which uses the function, so we would have a cyclic dependency - // Reaching this line means `trustedProxies` is in invalid format. + // Cannot log through the regular logger here because it may depend on + // getRemoteAddress(), which would create a cyclic dependency. error_log('Nextcloud trustedProxies has malformed entries'); return false; } @@ -550,9 +577,6 @@ public function getRemoteAddress(): string { return $remoteAddress; } - /** - * Check overwrite condition - */ private function isOverwriteCondition(): bool { $regex = '/' . $this->config->getSystemValueString('overwritecondaddr', '') . '/'; $remoteAddr = isset($this->server['REMOTE_ADDR']) ? $this->server['REMOTE_ADDR'] : ''; @@ -750,11 +774,9 @@ public function getServerHost(): string { } /** - * Returns the overwritehost setting from the config if set and - * if the overwrite condition is met + * Returns the overwritehost config value if configured and applicable. * - * @return string|null overwritehost value or null if not defined or the defined condition - * isn't met + * @return string|null */ private function getOverwriteHost() { if ($this->config->getSystemValueString('overwritehost') !== '' && $this->isOverwriteCondition()) { From 1b4c9d3e805b360259b2ecfb847d9bdb2bca5a42 Mon Sep 17 00:00:00 2001 From: Josh Date: Sun, 31 May 2026 13:13:00 -0400 Subject: [PATCH 13/14] chore(Request): drop no longer applicable psalm-baseline entries Signed-off-by: Josh --- build/psalm-baseline.xml | 3 --- 1 file changed, 3 deletions(-) diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index a8c6a2e1a33ff..ef8d61bcd8855 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -3337,9 +3337,6 @@ getOverwriteHost()]]> - cookies[$key]) ? $this->cookies[$key] : null]]> - env[$key]) ? $this->env[$key] : null]]> - files[$key]) ? $this->files[$key] : null]]> From 8c4f8525eafaab9056275dfbf1d4e206c1d35ee0 Mon Sep 17 00:00:00 2001 From: Josh Date: Sun, 31 May 2026 13:13:53 -0400 Subject: [PATCH 14/14] chore(IRequest): lint/cs fixup Signed-off-by: Josh --- lib/public/IRequest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/public/IRequest.php b/lib/public/IRequest.php index aa9b6ed642738..2510c48834d4b 100644 --- a/lib/public/IRequest.php +++ b/lib/public/IRequest.php @@ -12,7 +12,7 @@ /** * Immutable request wrapper with accessors for request variables and other * request-related data. - * + * * Request data should be retrieved through this interface whenever possible. * * Parameters can be accessed through dedicated methods or via magic property