From 3190384d242975233c310a046f847674a96c9e97 Mon Sep 17 00:00:00 2001 From: Khushboo Verma <43381712+vermakhushboo@users.noreply.github.com> Date: Mon, 5 Aug 2024 18:38:41 +0530 Subject: [PATCH 01/15] Add multipart support --- app/controllers/api/functions.php | 63 +++++- composer.lock | 48 ++--- docker-compose.yml | 2 +- tests/e2e/Client.php | 36 +++- .../Functions/FunctionsCustomServerTest.php | 199 +++++++++++++++++- 5 files changed, 305 insertions(+), 43 deletions(-) diff --git a/app/controllers/api/functions.php b/app/controllers/api/functions.php index a61db99bdf..307aee6e84 100644 --- a/app/controllers/api/functions.php +++ b/app/controllers/api/functions.php @@ -17,6 +17,7 @@ use Appwrite\Utopia\Database\Validator\CustomId; use Appwrite\Utopia\Database\Validator\Queries\Deployments; use Appwrite\Utopia\Database\Validator\Queries\Executions; use Appwrite\Utopia\Database\Validator\Queries\Functions; +use Appwrite\Utopia\Fetch\BodyMultipart; use Appwrite\Utopia\Response; use Appwrite\Utopia\Response\Model\Rule; use Executor\Executor; @@ -44,6 +45,7 @@ use Utopia\Storage\Validator\FileSize; use Utopia\Storage\Validator\Upload; use Utopia\Swoole\Request; use Utopia\System\System; +use Utopia\Validator\AnyOf; use Utopia\Validator\ArrayList; use Utopia\Validator\Assoc; use Utopia\Validator\Boolean; @@ -1592,9 +1594,10 @@ App::post('/v1/functions/:functionId/executions') ->param('async', false, new Boolean(), 'Execute code in the background. Default value is false.', true) ->param('path', '/', new Text(2048), 'HTTP path of execution. Path can include query params. Default value is /', true) ->param('method', 'POST', new Whitelist(['GET', 'POST', 'PUT', 'PATCH', 'DELETE', 'OPTIONS'], true), 'HTTP method of execution. Default value is GET.', true) - ->param('headers', [], new Assoc(), 'HTTP headers of execution. Defaults to empty.', true) + ->param('headers', [], new AnyOf([new Text(65535), new Assoc()], AnyOf::TYPE_MIXED), 'HTTP headers of execution. Defaults to empty.', true) ->param('scheduledAt', null, new DatetimeValidator(requireDateInFuture: true), 'Scheduled execution time in [ISO 8601](https://www.iso.org/iso-8601-date-and-time-format.html) format. DateTime value must be in future.', true) ->inject('response') + ->inject('request') ->inject('project') ->inject('dbForProject') ->inject('dbForConsole') @@ -1603,12 +1606,33 @@ App::post('/v1/functions/:functionId/executions') ->inject('queueForUsage') ->inject('queueForFunctions') ->inject('geodb') - ->action(function (string $functionId, string $body, bool $async, string $path, string $method, array $headers, ?string $scheduledAt, Response $response, Document $project, Database $dbForProject, Database $dbForConsole, Document $user, Event $queueForEvents, Usage $queueForUsage, Func $queueForFunctions, Reader $geodb) { + ->action(function (string $functionId, string $body, bool $async, string $path, string $method, mixed $headers, ?string $scheduledAt, Response $response, Request $request, Document $project, Database $dbForProject, Database $dbForConsole, Document $user, Event $queueForEvents, Usage $queueForUsage, Func $queueForFunctions, Reader $geodb) { if(!$async && !is_null($scheduledAt)) { throw new Exception(Exception::GENERAL_BAD_REQUEST, 'Scheduled executions must run asynchronously. Set scheduledAt to a future date, or set async to true.'); } + /** + * @var array $headers + */ + $assocParams = ['headers']; + foreach ($assocParams as $assocParam) { + if (!empty('headers') && !is_array($$assocParam)) { + $$assocParam = \json_decode($$assocParam, true); + } + } + + // 'body' validator + if (\strlen($body) > 20971520) { + throw new Exception("Parameter body can be only up to 20MB in size."); + } + + // 'headers' validator + $validator = new Assoc(); + if (!$validator->isValid($headers)) { + throw new Exception($validator->getDescription(), 400); + } + $function = Authorization::skip(fn () => $dbForProject->getDocument('functions', $functionId)); $isAPIKey = Auth::isAppUser(Authorization::getRoles()); @@ -1915,9 +1939,38 @@ App::post('/v1/functions/:functionId/executions') $execution->setAttribute('responseBody', $executionResponse['body'] ?? ''); $execution->setAttribute('responseHeaders', $headers); - $response - ->setStatusCode(Response::STATUS_CODE_CREATED) - ->dynamic($execution, Response::MODEL_EXECUTION); + $acceptTypes = \explode(', ', $request->getHeader('accept', 'multipart/form-data')); + $isJson = false; + + foreach ($acceptTypes as $acceptType) { + if (\str_starts_with($acceptType, 'application/json') || \str_starts_with($acceptType, 'application/*')) { + $isJson = true; + break; + } + } + + if ($isJson) { + $executionString = \json_encode($execution, JSON_UNESCAPED_UNICODE); + if (!$executionString) { + throw new Exception('Execution resulted in binary response, but JSON response does not allow binaries. Use "Accept: multipart/form-data" header to support binaries.', 400); + } + + $response + ->setStatusCode(Response::STATUS_CODE_OK) + ->addHeader('content-type', 'application/json') + ->send($executionString); + } else { + // Multipart form data response + $multipart = new BodyMultipart(); + foreach ($execution as $key => $value) { + $multipart->setPart($key, $value); + } + + $response + ->setStatusCode(Response::STATUS_CODE_CREATED) + ->addHeader('content-type', $multipart->exportHeader()) + ->send($multipart->exportBody()); + } }); App::get('/v1/functions/:functionId/executions') diff --git a/composer.lock b/composer.lock index 68f109ed72..1509935c09 100644 --- a/composer.lock +++ b/composer.lock @@ -1721,16 +1721,16 @@ }, { "name": "utopia-php/database", - "version": "0.50.0", + "version": "0.50.2", "source": { "type": "git", "url": "https://github.com/utopia-php/database.git", - "reference": "ce3eaccb2f3bbd34b2b97419836fec633b26b8f7" + "reference": "c712d1f6c8ec37886a7a1ad4d60a8cd75dec00aa" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/utopia-php/database/zipball/ce3eaccb2f3bbd34b2b97419836fec633b26b8f7", - "reference": "ce3eaccb2f3bbd34b2b97419836fec633b26b8f7", + "url": "https://api.github.com/repos/utopia-php/database/zipball/c712d1f6c8ec37886a7a1ad4d60a8cd75dec00aa", + "reference": "c712d1f6c8ec37886a7a1ad4d60a8cd75dec00aa", "shasum": "" }, "require": { @@ -1771,9 +1771,9 @@ ], "support": { "issues": "https://github.com/utopia-php/database/issues", - "source": "https://github.com/utopia-php/database/tree/0.50.0" + "source": "https://github.com/utopia-php/database/tree/0.50.2" }, - "time": "2024-06-21T03:21:42+00:00" + "time": "2024-07-31T10:12:19+00:00" }, { "name": "utopia-php/domains", @@ -1923,16 +1923,16 @@ }, { "name": "utopia-php/framework", - "version": "0.33.6", + "version": "0.33.7", "source": { "type": "git", "url": "https://github.com/utopia-php/http.git", - "reference": "8fe57da0cecd57e3b17cd395b4a666a24f4c07a6" + "reference": "78d293d99a262bd63ece750bbf989c7e0643b825" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/utopia-php/http/zipball/8fe57da0cecd57e3b17cd395b4a666a24f4c07a6", - "reference": "8fe57da0cecd57e3b17cd395b4a666a24f4c07a6", + "url": "https://api.github.com/repos/utopia-php/http/zipball/78d293d99a262bd63ece750bbf989c7e0643b825", + "reference": "78d293d99a262bd63ece750bbf989c7e0643b825", "shasum": "" }, "require": { @@ -1962,9 +1962,9 @@ ], "support": { "issues": "https://github.com/utopia-php/http/issues", - "source": "https://github.com/utopia-php/http/tree/0.33.6" + "source": "https://github.com/utopia-php/http/tree/0.33.7" }, - "time": "2024-03-21T18:10:57+00:00" + "time": "2024-08-01T14:01:04+00:00" }, { "name": "utopia-php/image", @@ -2990,16 +2990,16 @@ "packages-dev": [ { "name": "appwrite/sdk-generator", - "version": "0.39.3", + "version": "0.39.4", "source": { "type": "git", "url": "https://github.com/appwrite/sdk-generator.git", - "reference": "16142d88270e368030d7956cadf2d7816413f8c4" + "reference": "501b92d73ae55e0f880ed00f57bc64a54d0ce137" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/appwrite/sdk-generator/zipball/16142d88270e368030d7956cadf2d7816413f8c4", - "reference": "16142d88270e368030d7956cadf2d7816413f8c4", + "url": "https://api.github.com/repos/appwrite/sdk-generator/zipball/501b92d73ae55e0f880ed00f57bc64a54d0ce137", + "reference": "501b92d73ae55e0f880ed00f57bc64a54d0ce137", "shasum": "" }, "require": { @@ -3035,9 +3035,9 @@ "description": "Appwrite PHP library for generating API SDKs for multiple programming languages and platforms", "support": { "issues": "https://github.com/appwrite/sdk-generator/issues", - "source": "https://github.com/appwrite/sdk-generator/tree/0.39.3" + "source": "https://github.com/appwrite/sdk-generator/tree/0.39.4" }, - "time": "2024-07-12T15:29:48+00:00" + "time": "2024-07-26T22:34:10+00:00" }, { "name": "doctrine/deprecations", @@ -3158,16 +3158,16 @@ }, { "name": "laravel/pint", - "version": "v1.16.2", + "version": "v1.17.1", "source": { "type": "git", "url": "https://github.com/laravel/pint.git", - "reference": "51f1ba679a6afe0315621ad143d788bd7ded0eca" + "reference": "b5b6f716db298671c1dfea5b1082ec2c0ae7064f" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/laravel/pint/zipball/51f1ba679a6afe0315621ad143d788bd7ded0eca", - "reference": "51f1ba679a6afe0315621ad143d788bd7ded0eca", + "url": "https://api.github.com/repos/laravel/pint/zipball/b5b6f716db298671c1dfea5b1082ec2c0ae7064f", + "reference": "b5b6f716db298671c1dfea5b1082ec2c0ae7064f", "shasum": "" }, "require": { @@ -3220,7 +3220,7 @@ "issues": "https://github.com/laravel/pint/issues", "source": "https://github.com/laravel/pint" }, - "time": "2024-07-09T15:58:08+00:00" + "time": "2024-08-01T09:06:33+00:00" }, { "name": "matthiasmullie/minify", @@ -5617,5 +5617,5 @@ "platform-overrides": { "php": "8.3" }, - "plugin-api-version": "2.6.0" + "plugin-api-version": "2.3.0" } diff --git a/docker-compose.yml b/docker-compose.yml index d28586bf7b..2a8bc45b72 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -873,7 +873,7 @@ services: hostname: exc1 <<: *x-logging stop_signal: SIGINT - image: openruntimes/executor:0.6.1 + image: openruntimes/executor:0.6.4 restart: unless-stopped networks: - appwrite diff --git a/tests/e2e/Client.php b/tests/e2e/Client.php index c8bf36a2ef..0774f1c6fd 100644 --- a/tests/e2e/Client.php +++ b/tests/e2e/Client.php @@ -2,6 +2,7 @@ namespace Tests\E2E; +use Appwrite\Utopia\Fetch\BodyMultipart; use Exception; class Client @@ -224,18 +225,35 @@ class Client $responseType = $responseHeaders['content-type'] ?? ''; $responseStatus = curl_getinfo($ch, CURLINFO_HTTP_CODE); - if ($decode && substr($responseType, 0, strpos($responseType, ';')) == 'application/json') { - $json = json_decode($responseBody, true); + if ($decode) { + $strpos = strpos($responseType, ';'); + $strpos = \is_bool($strpos) ? \strlen($responseType) : $strpos; + switch (substr($responseType, 0, $strpos)) { + case 'multipart/form-data': + $boundary = \explode('boundary=', $responseHeaders['content-type'] ?? '')[1] ?? ''; + $multipartResponse = new BodyMultipart($boundary); + $multipartResponse->load(\is_bool($responseBody) ? '' : $responseBody); - if ($json === null) { - throw new Exception('Failed to parse response: ' . $responseBody); + $responseBody = $multipartResponse->getParts(); + break; + case 'application/json': + if (\is_bool($responseBody)) { + throw new Exception('Response is not a valid JSON.'); + } + + $json = json_decode($responseBody, true); + + if ($json === null) { + throw new Exception('Failed to parse response: ' . $responseBody); + } + + $responseBody = $json; + $json = null; + break; } - - $responseBody = $json; - $json = null; } - if ((curl_errno($ch))) { + if ((curl_errno($ch)/* || 200 != $responseStatus*/)) { throw new Exception(curl_error($ch) . ' with status code ' . $responseStatus, $responseStatus); } @@ -244,7 +262,7 @@ class Client $responseHeaders['status-code'] = $responseStatus; if ($responseStatus === 500) { - echo 'Server error(' . $method . ': ' . $path . '. Params: ' . json_encode($params) . '): ' . json_encode($responseBody) . "\n"; + echo 'Server error(' . $method . ': ' . $path . '. Params: ' . json_encode($params) . '): ' . json_encode($responseBody) . '\n'; } return [ diff --git a/tests/e2e/Services/Functions/FunctionsCustomServerTest.php b/tests/e2e/Services/Functions/FunctionsCustomServerTest.php index c9f9e4443f..63201905f4 100644 --- a/tests/e2e/Services/Functions/FunctionsCustomServerTest.php +++ b/tests/e2e/Services/Functions/FunctionsCustomServerTest.php @@ -1387,6 +1387,197 @@ class FunctionsCustomServerTest extends Scope $this->assertEquals(204, $response['headers']['status-code']); } + public function testCreateCustomExecutionBinaryResponse() + { + $timeout = 15; + $code = realpath(__DIR__ . '/../../../resources/functions') . "/php-binary-response/code.tar.gz"; + $this->packageCode('php-binary-response'); + + $function = $this->client->call(Client::METHOD_POST, '/functions', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'functionId' => ID::unique(), + 'name' => 'Test PHP Binary executions', + 'runtime' => 'php-8.0', + 'entrypoint' => 'index.php', + 'timeout' => $timeout, + 'execute' => ['any'] + ]); + + $functionId = $function['body']['$id'] ?? ''; + + $this->assertEquals(201, $function['headers']['status-code']); + + $deployment = $this->client->call(Client::METHOD_POST, '/functions/' . $functionId . '/deployments', array_merge([ + 'content-type' => 'multipart/form-data', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'entrypoint' => 'index.php', + 'code' => new CURLFile($code, 'application/x-gzip', basename($code)), + 'activate' => true + ]); + + $deploymentId = $deployment['body']['$id'] ?? ''; + $this->assertEquals(202, $deployment['headers']['status-code']); + + // Poll until deployment is built + while (true) { + $deployment = $this->client->call(Client::METHOD_GET, '/functions/' . $function['body']['$id'] . '/deployments/' . $deploymentId, [ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + 'x-appwrite-key' => $this->getProject()['apiKey'], + ]); + + if ( + $deployment['headers']['status-code'] >= 400 + || \in_array($deployment['body']['status'], ['ready', 'failed']) + ) { + break; + } + + \sleep(1); + } + + $deployment = $this->client->call(Client::METHOD_PATCH, '/functions/' . $functionId . '/deployments/' . $deploymentId, array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), []); + + $this->assertEquals(200, $deployment['headers']['status-code']); + + // Wait a little for activation to finish + sleep(5); + + $execution = $this->client->call(Client::METHOD_POST, '/functions/' . $functionId . '/executions', array_merge([ + 'x-appwrite-project' => $this->getProject()['$id'], + 'accept' => 'multipart/form-data', + ], $this->getHeaders()), [ + 'body' => null, + ]); + + $bytes = unpack('C*byte', $execution['body']['responseBody']); + $this->assertCount(3, $bytes); + $this->assertEquals(0, $bytes['byte1']); + $this->assertEquals(10, $bytes['byte2']); + $this->assertEquals(255, $bytes['byte3']); + + $execution = $this->client->call(Client::METHOD_POST, '/functions/' . $functionId . '/executions', array_merge([ + 'x-appwrite-project' => $this->getProject()['$id'], + 'accept' => 'application/json', + ], $this->getHeaders()), [ + 'body' => null, + ]); + + $this->assertEquals(500, $execution['headers']['status-code']); + $this->assertStringContainsString('Execution resulted in binary response, but JSON response does not allow binaries.', $execution['body']['type']); + + // Cleanup : Delete function + $response = $this->client->call(Client::METHOD_DELETE, '/functions/' . $functionId, [ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + 'x-appwrite-key' => $this->getProject()['apiKey'], + ], []); + + $this->assertEquals(204, $response['headers']['status-code']); + } + + public function testCreateCustomExecutionBinaryRequest() + { + $timeout = 15; + $code = realpath(__DIR__ . '/../../../resources/functions') . "/php-binary-request/code.tar.gz"; + $this->packageCode('php-binary-request'); + + $function = $this->client->call(Client::METHOD_POST, '/functions', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'functionId' => ID::unique(), + 'name' => 'Test PHP Binary executions', + 'runtime' => 'php-8.0', + 'entrypoint' => 'index.php', + 'timeout' => $timeout, + 'execute' => ['any'] + ]); + + $functionId = $function['body']['$id'] ?? ''; + + $this->assertEquals(201, $function['headers']['status-code']); + + $variable = $this->client->call(Client::METHOD_POST, '/functions/' . $functionId . '/variables', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'key' => 'CUSTOM_VARIABLE', + 'value' => 'variable', + ]); + + $this->assertEquals(201, $variable['headers']['status-code']); + + $deployment = $this->client->call(Client::METHOD_POST, '/functions/' . $functionId . '/deployments', array_merge([ + 'content-type' => 'multipart/form-data', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'entrypoint' => 'index.php', + 'code' => new CURLFile($code, 'application/x-gzip', basename($code)), + 'activate' => true + ]); + + $deploymentId = $deployment['body']['$id'] ?? ''; + $this->assertEquals(202, $deployment['headers']['status-code']); + + while (true) { + $deployment = $this->client->call(Client::METHOD_GET, '/functions/' . $function['body']['$id'] . '/deployments/' . $deploymentId, [ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + 'x-appwrite-key' => $this->getProject()['apiKey'], + ]); + + if ( + $deployment['headers']['status-code'] >= 400 + || \in_array($deployment['body']['status'], ['ready', 'failed']) + ) { + break; + } + + \sleep(1); + } + + $deployment = $this->client->call(Client::METHOD_PATCH, '/functions/' . $functionId . '/deployments/' . $deploymentId, array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), []); + + $this->assertEquals(200, $deployment['headers']['status-code']); + + // Wait a little for activation to finish + sleep(5); + + $bytes = pack('C*', ...[0, 20, 255]); + + $execution = $this->client->call(Client::METHOD_POST, '/functions/' . $functionId . '/executions', array_merge([ + 'content-type' => 'multipart/form-data', + 'x-appwrite-project' => $this->getProject()['$id'], + 'accept' => 'application/json', + ], $this->getHeaders()), [ + 'body' => $bytes, + ], false); + + $executionBody = json_decode($execution['body'], true); + + $this->assertEquals(200, $execution['headers']['status-code']); + $this->assertEquals(\md5($bytes), $executionBody['responseBody']); + + // Cleanup : Delete function + $response = $this->client->call(Client::METHOD_DELETE, '/functions/' . $functionId, [ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + 'x-appwrite-key' => $this->getProject()['apiKey'], + ], []); + + $this->assertEquals(204, $response['headers']['status-code']); + } + public function testv2Function() { $timeout = 15; @@ -1897,7 +2088,7 @@ class FunctionsCustomServerTest extends Scope $this->assertEquals(204, $response['headers']['status-code']); } - public function testFunctionsDomainBianryResponse() + public function testFunctionsDomainBinaryResponse() { $timeout = 15; $code = realpath(__DIR__ . '/../../../resources/functions') . "/php-binary-response/code.tar.gz"; @@ -1999,7 +2190,7 @@ class FunctionsCustomServerTest extends Scope $this->assertEquals(204, $response['headers']['status-code']); } - public function testFunctionsDomainBianryRequest() + public function testFunctionsDomainBinaryRequest() { $timeout = 15; $code = realpath(__DIR__ . '/../../../resources/functions') . "/php-binary-request/code.tar.gz"; @@ -2081,9 +2272,9 @@ class FunctionsCustomServerTest extends Scope $proxyClient = new Client(); $proxyClient->setEndpoint('http://' . $domain); - $bytes = pack('C*', ...[0,20,255]); + $bytes = pack('C*', ...[0, 20, 255]); - $response = $proxyClient->call(Client::METHOD_POST, '/', [ 'content-type' => 'text/plain' ], $bytes, false); + $response = $proxyClient->call(Client::METHOD_POST, '/', ['content-type' => 'text/plain'], $bytes, false); $this->assertEquals(200, $response['headers']['status-code']); $this->assertEquals(\md5($bytes), $response['body']); From 6691c985dddb4a45b7fd97d47972fd00c5205972 Mon Sep 17 00:00:00 2001 From: Khushboo Verma <43381712+vermakhushboo@users.noreply.github.com> Date: Tue, 6 Aug 2024 13:49:28 +0530 Subject: [PATCH 02/15] Add headers validator --- app/controllers/api/functions.php | 3 +- src/Appwrite/Functions/Validator/Headers.php | 90 ++++++++++++++++++++ 2 files changed, 92 insertions(+), 1 deletion(-) create mode 100644 src/Appwrite/Functions/Validator/Headers.php diff --git a/app/controllers/api/functions.php b/app/controllers/api/functions.php index 307aee6e84..1f0121cab1 100644 --- a/app/controllers/api/functions.php +++ b/app/controllers/api/functions.php @@ -10,6 +10,7 @@ use Appwrite\Event\Usage; use Appwrite\Event\Validator\FunctionEvent; use Appwrite\Extend\Exception; use Appwrite\Extend\Exception as AppwriteException; +use Appwrite\Functions\Validator\Headers; use Appwrite\Messaging\Adapter\Realtime; use Appwrite\Platform\Tasks\ScheduleExecutions; use Appwrite\Task\Validator\Cron; @@ -1628,7 +1629,7 @@ App::post('/v1/functions/:functionId/executions') } // 'headers' validator - $validator = new Assoc(); + $validator = new Headers(); if (!$validator->isValid($headers)) { throw new Exception($validator->getDescription(), 400); } diff --git a/src/Appwrite/Functions/Validator/Headers.php b/src/Appwrite/Functions/Validator/Headers.php new file mode 100644 index 0000000000..1a491ed687 --- /dev/null +++ b/src/Appwrite/Functions/Validator/Headers.php @@ -0,0 +1,90 @@ +allowEmpty = $allowEmpty; + } + + /** + * Get Description. + * + * Returns validator description + * + * @return string + */ + public function getDescription(): string + { + return 'Invalid header format. Header keys can only contain alphanumeric characters, underscores, and hyphens. Header keys cannot start with "x-appwrite-" prefix.'; + } + + /** + * Is valid. + * + * @param mixed $value + * + * @return bool + */ + public function isValid($value): bool + { + if ($this->allowEmpty && empty($value)) { + return true; + } + + if (\is_string($value)) { + $value = \json_decode($value, true); + } + + if (\json_last_error() == JSON_ERROR_NONE) { + if (\is_array($value)) { + foreach ($value as $key => $val) { + // Check for invalid characters in key and value + if (!preg_match('/^[a-zA-Z0-9_-]+$/', $key)) { + return false; + } + // Check for x-appwrite- prefix + if (0 === strpos($key, 'x-appwrite-')) { + return false; + } + } + } + } + return \json_last_error() == JSON_ERROR_NONE; + } + + /** + * Is array + * + * Function will return true if object is array. + * + * @return bool + */ + public function isArray(): bool + { + return false; + } + + /** + * Get Type + * + * Returns validator type. + * + * @return string + */ + public function getType(): string + { + return self::TYPE_OBJECT; + } +} From d4cb90c6087db9f7ecf9f6eb9f0f92840f7bb132 Mon Sep 17 00:00:00 2001 From: Khushboo Verma <43381712+vermakhushboo@users.noreply.github.com> Date: Tue, 6 Aug 2024 23:01:09 +0530 Subject: [PATCH 03/15] Addressed PR comments --- app/controllers/api/functions.php | 26 ++++++++-- src/Appwrite/Functions/Validator/Headers.php | 2 +- .../Functions/FunctionsCustomServerTest.php | 20 ++++++++ .../unit/Functions/Validator/HeadersTest.php | 48 +++++++++++++++++++ 4 files changed, 90 insertions(+), 6 deletions(-) create mode 100644 tests/unit/Functions/Validator/HeadersTest.php diff --git a/app/controllers/api/functions.php b/app/controllers/api/functions.php index 6376b40bf6..b83229f54f 100644 --- a/app/controllers/api/functions.php +++ b/app/controllers/api/functions.php @@ -1602,7 +1602,7 @@ App::post('/v1/functions/:functionId/executions') ->label('sdk.response.type', Response::CONTENT_TYPE_JSON) ->label('sdk.response.model', Response::MODEL_EXECUTION) ->param('functionId', '', new UID(), 'Function ID.') - ->param('body', '', new Text(0, 0), 'HTTP body of execution. Default value is empty string.', true) + ->param('body', '', new Text(20971520, 0), 'HTTP body of execution. Default value is empty string.', true) ->param('async', false, new Boolean(), 'Execute code in the background. Default value is false.', true) ->param('path', '/', new Text(2048), 'HTTP path of execution. Path can include query params. Default value is /', true) ->param('method', 'POST', new Whitelist(['GET', 'POST', 'PUT', 'PATCH', 'DELETE', 'OPTIONS'], true), 'HTTP method of execution. Default value is GET.', true) @@ -1634,9 +1634,25 @@ App::post('/v1/functions/:functionId/executions') } } - // 'body' validator - if (\strlen($body) > 20971520) { - throw new Exception("Parameter body can be only up to 20MB in size."); + $booleanParams = ['async']; + foreach ($booleanParams as $booleamParam) { + if (!empty($$booleamParam) && !is_bool($$booleamParam)) { + $$booleamParam = $$booleamParam === "true" ? true : false; + } + } + + $datetimeParams = ['scheduledAt']; + foreach ($datetimeParams as $datetimeParam) { + if (!empty($$datetimeParam)) { + $$datetimeParam = new DateTime($$datetimeParam); + } + } + + $validator = new DatetimeValidator(requireDateInFuture: true); + foreach ($datetimeParams as $datetimeParam) { + if (!empty($$datetimeParam) && !$validator->isValid($$datetimeParam)) { + throw new Exception($validator->getDescription(), 400); + } } // 'headers' validator @@ -1951,7 +1967,7 @@ App::post('/v1/functions/:functionId/executions') $execution->setAttribute('responseBody', $executionResponse['body'] ?? ''); $execution->setAttribute('responseHeaders', $headers); - $acceptTypes = \explode(', ', $request->getHeader('accept', 'multipart/form-data')); + $acceptTypes = \explode(', ', $request->getHeader('accept', 'application/json')); $isJson = false; foreach ($acceptTypes as $acceptType) { diff --git a/src/Appwrite/Functions/Validator/Headers.php b/src/Appwrite/Functions/Validator/Headers.php index 1a491ed687..99f2d74249 100644 --- a/src/Appwrite/Functions/Validator/Headers.php +++ b/src/Appwrite/Functions/Validator/Headers.php @@ -61,7 +61,7 @@ class Headers extends Validator } } } - return \json_last_error() == JSON_ERROR_NONE; + return true; } /** diff --git a/tests/e2e/Services/Functions/FunctionsCustomServerTest.php b/tests/e2e/Services/Functions/FunctionsCustomServerTest.php index 3b2a53fb3e..f52fba051e 100644 --- a/tests/e2e/Services/Functions/FunctionsCustomServerTest.php +++ b/tests/e2e/Services/Functions/FunctionsCustomServerTest.php @@ -1385,12 +1385,17 @@ class FunctionsCustomServerTest extends Scope 'body' => null, ]); + $this->assertEquals(201, $execution['headers']['status-code']); + $this->assertStringContainsString('multipart/form-data', $execution['headers']['content-type']); $bytes = unpack('C*byte', $execution['body']['responseBody']); $this->assertCount(3, $bytes); $this->assertEquals(0, $bytes['byte1']); $this->assertEquals(10, $bytes['byte2']); $this->assertEquals(255, $bytes['byte3']); + /** + * Test for FAILURE + */ $execution = $this->client->call(Client::METHOD_POST, '/functions/' . $functionId . '/executions', array_merge([ 'x-appwrite-project' => $this->getProject()['$id'], 'accept' => 'application/json', @@ -1496,6 +1501,21 @@ class FunctionsCustomServerTest extends Scope $this->assertEquals(200, $execution['headers']['status-code']); $this->assertEquals(\md5($bytes), $executionBody['responseBody']); + $this->assertEquals($execution['headers']['content-type'], 'application/json'); + + /** + * Test for FAILURE + */ + $execution = $this->client->call(Client::METHOD_POST, '/functions/' . $functionId . '/executions', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + 'accept' => 'application/json', + ], $this->getHeaders()), [ + 'body' => $bytes, + ], false); + + $executionBody = json_decode($execution['body'], true); + $this->assertNotEquals(\md5($bytes), $executionBody['responseBody']); // Cleanup : Delete function $response = $this->client->call(Client::METHOD_DELETE, '/functions/' . $functionId, [ diff --git a/tests/unit/Functions/Validator/HeadersTest.php b/tests/unit/Functions/Validator/HeadersTest.php new file mode 100644 index 0000000000..b06a71cb37 --- /dev/null +++ b/tests/unit/Functions/Validator/HeadersTest.php @@ -0,0 +1,48 @@ +object = new Headers(); + } + + public function testValues(): void + { + $headers = [ + 'headerKey' => 'headerValue', + ]; + $this->assertEquals($this->object->isValid($headers), true); + + $headers = [ + 'headerKey' => 'headerValue', + 'x-appwrite-key' => 'headerValue', + ]; + $this->assertEquals($this->object->isValid($headers), false); + + $headers = [ + 'headerKey' => 'headerValue', + 'headerKey2' => 'headerValue2', + ]; + $this->assertEquals($this->object->isValid($headers), true); + + $headers = [ + 'headerKey' => 'headerValue', + 'x-appwrite-project' => 'headerValue', + 'headerKey2' => 'headerValue2', + ]; + $this->assertEquals($this->object->isValid($headers), false); + + $headers = [ + 'header/////Key' => 'headerValue', + ]; + $this->assertEquals($this->object->isValid($headers), false); + } +} From 22853b7b595dad2fc9cee47095558b32a10dccc0 Mon Sep 17 00:00:00 2001 From: Khushboo Verma <43381712+vermakhushboo@users.noreply.github.com> Date: Tue, 6 Aug 2024 23:49:09 +0530 Subject: [PATCH 04/15] Fixed test --- tests/e2e/General/HTTPTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/e2e/General/HTTPTest.php b/tests/e2e/General/HTTPTest.php index 5c3b10a9ca..b6dd3543c6 100644 --- a/tests/e2e/General/HTTPTest.php +++ b/tests/e2e/General/HTTPTest.php @@ -131,7 +131,6 @@ class HTTPTest extends Scope 'content-type' => 'application/json', ], json_decode(file_get_contents($directory . $file), true)); - $response['body'] = json_decode($response['body'], true); $this->assertEquals(200, $response['headers']['status-code']); // looks like recent change in the validator $this->assertTrue(empty($response['body']['schemaValidationMessages'])); From dcf1684c8ca0cd0196c98b3080772e7f3876c8cb Mon Sep 17 00:00:00 2001 From: Khushboo Verma <43381712+vermakhushboo@users.noreply.github.com> Date: Wed, 7 Aug 2024 18:45:53 +0530 Subject: [PATCH 05/15] Update dynamic response method --- app/controllers/api/functions.php | 43 ++------------ composer.lock | 30 +++++----- src/Appwrite/Utopia/Response.php | 56 ++++++++++++++++++- .../Functions/FunctionsCustomServerTest.php | 8 +-- 4 files changed, 79 insertions(+), 58 deletions(-) diff --git a/app/controllers/api/functions.php b/app/controllers/api/functions.php index b83229f54f..5fd3519da4 100644 --- a/app/controllers/api/functions.php +++ b/app/controllers/api/functions.php @@ -18,7 +18,6 @@ use Appwrite\Utopia\Database\Validator\CustomId; use Appwrite\Utopia\Database\Validator\Queries\Deployments; use Appwrite\Utopia\Database\Validator\Queries\Executions; use Appwrite\Utopia\Database\Validator\Queries\Functions; -use Appwrite\Utopia\Fetch\BodyMultipart; use Appwrite\Utopia\Response; use Appwrite\Utopia\Response\Model\Rule; use Executor\Executor; @@ -1641,20 +1640,6 @@ App::post('/v1/functions/:functionId/executions') } } - $datetimeParams = ['scheduledAt']; - foreach ($datetimeParams as $datetimeParam) { - if (!empty($$datetimeParam)) { - $$datetimeParam = new DateTime($$datetimeParam); - } - } - - $validator = new DatetimeValidator(requireDateInFuture: true); - foreach ($datetimeParams as $datetimeParam) { - if (!empty($$datetimeParam) && !$validator->isValid($$datetimeParam)) { - throw new Exception($validator->getDescription(), 400); - } - } - // 'headers' validator $validator = new Headers(); if (!$validator->isValid($headers)) { @@ -1967,9 +1952,9 @@ App::post('/v1/functions/:functionId/executions') $execution->setAttribute('responseBody', $executionResponse['body'] ?? ''); $execution->setAttribute('responseHeaders', $headers); - $acceptTypes = \explode(', ', $request->getHeader('accept', 'application/json')); $isJson = false; + $acceptTypes = \explode(', ', $request->getHeader('accept', 'application/json')); foreach ($acceptTypes as $acceptType) { if (\str_starts_with($acceptType, 'application/json') || \str_starts_with($acceptType, 'application/*')) { $isJson = true; @@ -1977,28 +1962,10 @@ App::post('/v1/functions/:functionId/executions') } } - if ($isJson) { - $executionString = \json_encode($execution, JSON_UNESCAPED_UNICODE); - if (!$executionString) { - throw new Exception('Execution resulted in binary response, but JSON response does not allow binaries. Use "Accept: multipart/form-data" header to support binaries.', 400); - } - - $response - ->setStatusCode(Response::STATUS_CODE_OK) - ->addHeader('content-type', 'application/json') - ->send($executionString); - } else { - // Multipart form data response - $multipart = new BodyMultipart(); - foreach ($execution as $key => $value) { - $multipart->setPart($key, $value); - } - - $response - ->setStatusCode(Response::STATUS_CODE_CREATED) - ->addHeader('content-type', $multipart->exportHeader()) - ->send($multipart->exportBody()); - } + $response + ->setStatusCode(Response::STATUS_CODE_CREATED) + ->setContentType($isJson ? Response::CONTENT_TYPE_JSON : Response::CONTENT_TYPE_MULTIPART) + ->dynamic($execution, Response::MODEL_EXECUTION); }); App::get('/v1/functions/:functionId/executions') diff --git a/composer.lock b/composer.lock index 1509935c09..76b1579a87 100644 --- a/composer.lock +++ b/composer.lock @@ -2990,16 +2990,16 @@ "packages-dev": [ { "name": "appwrite/sdk-generator", - "version": "0.39.4", + "version": "0.39.5", "source": { "type": "git", "url": "https://github.com/appwrite/sdk-generator.git", - "reference": "501b92d73ae55e0f880ed00f57bc64a54d0ce137" + "reference": "40d0f66f2f85be74049ad710b46203aa151f53fd" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/appwrite/sdk-generator/zipball/501b92d73ae55e0f880ed00f57bc64a54d0ce137", - "reference": "501b92d73ae55e0f880ed00f57bc64a54d0ce137", + "url": "https://api.github.com/repos/appwrite/sdk-generator/zipball/40d0f66f2f85be74049ad710b46203aa151f53fd", + "reference": "40d0f66f2f85be74049ad710b46203aa151f53fd", "shasum": "" }, "require": { @@ -3035,9 +3035,9 @@ "description": "Appwrite PHP library for generating API SDKs for multiple programming languages and platforms", "support": { "issues": "https://github.com/appwrite/sdk-generator/issues", - "source": "https://github.com/appwrite/sdk-generator/tree/0.39.4" + "source": "https://github.com/appwrite/sdk-generator/tree/0.39.5" }, - "time": "2024-07-26T22:34:10+00:00" + "time": "2024-08-06T00:51:40+00:00" }, { "name": "doctrine/deprecations", @@ -3158,16 +3158,16 @@ }, { "name": "laravel/pint", - "version": "v1.17.1", + "version": "v1.17.2", "source": { "type": "git", "url": "https://github.com/laravel/pint.git", - "reference": "b5b6f716db298671c1dfea5b1082ec2c0ae7064f" + "reference": "e8a88130a25e3f9d4d5785e6a1afca98268ab110" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/laravel/pint/zipball/b5b6f716db298671c1dfea5b1082ec2c0ae7064f", - "reference": "b5b6f716db298671c1dfea5b1082ec2c0ae7064f", + "url": "https://api.github.com/repos/laravel/pint/zipball/e8a88130a25e3f9d4d5785e6a1afca98268ab110", + "reference": "e8a88130a25e3f9d4d5785e6a1afca98268ab110", "shasum": "" }, "require": { @@ -3178,13 +3178,13 @@ "php": "^8.1.0" }, "require-dev": { - "friendsofphp/php-cs-fixer": "^3.59.3", - "illuminate/view": "^10.48.12", - "larastan/larastan": "^2.9.7", + "friendsofphp/php-cs-fixer": "^3.61.1", + "illuminate/view": "^10.48.18", + "larastan/larastan": "^2.9.8", "laravel-zero/framework": "^10.4.0", "mockery/mockery": "^1.6.12", "nunomaduro/termwind": "^1.15.1", - "pestphp/pest": "^2.34.8" + "pestphp/pest": "^2.35.0" }, "bin": [ "builds/pint" @@ -3220,7 +3220,7 @@ "issues": "https://github.com/laravel/pint/issues", "source": "https://github.com/laravel/pint" }, - "time": "2024-08-01T09:06:33+00:00" + "time": "2024-08-06T15:11:54+00:00" }, { "name": "matthiasmullie/minify", diff --git a/src/Appwrite/Utopia/Response.php b/src/Appwrite/Utopia/Response.php index d2e78bb310..f0dace191e 100644 --- a/src/Appwrite/Utopia/Response.php +++ b/src/Appwrite/Utopia/Response.php @@ -2,6 +2,7 @@ namespace Appwrite\Utopia; +use Appwrite\Utopia\Fetch\BodyMultipart; use Appwrite\Utopia\Response\Filter; use Appwrite\Utopia\Response\Model; use Appwrite\Utopia\Response\Model\Account; @@ -107,6 +108,7 @@ use Appwrite\Utopia\Response\Model\Variable; use Appwrite\Utopia\Response\Model\VcsContent; use Appwrite\Utopia\Response\Model\Webhook; use Exception; +use JsonException; use Swoole\Http\Response as SwooleHTTPResponse; // Keep last use Utopia\Database\Document; @@ -486,6 +488,7 @@ class Response extends SwooleResponse */ public const CONTENT_TYPE_YAML = 'application/x-yaml'; public const CONTENT_TYPE_NULL = 'null'; + public const CONTENT_TYPE_MULTIPART = 'multipart/form-data'; /** * List of defined output objects @@ -556,7 +559,11 @@ class Response extends SwooleResponse switch ($this->getContentType()) { case self::CONTENT_TYPE_JSON: - $this->json(!empty($output) ? $output : new \stdClass()); + try { + $this->json(!empty($output) ? $output : new \stdClass()); + } catch (JsonException $e) { + throw new Exception('Failed to parse binary response: ' . $e->getMessage(), 400); + } break; case self::CONTENT_TYPE_YAML: @@ -566,6 +573,10 @@ class Response extends SwooleResponse case self::CONTENT_TYPE_NULL: break; + case self::CONTENT_TYPE_MULTIPART: + $this->multipart(!empty($output) ? $output : new \stdClass()); + break; + default: if ($model === self::MODEL_NONE) { $this->noContent(); @@ -697,6 +708,49 @@ class Response extends SwooleResponse ->send(\yaml_emit($data, YAML_UTF8_ENCODING)); } + /** + * Multipart + * + * This helper is for sending multipart/form-data HTTP response. + * It sets relevant content type header ('multipart/form-data') and convert a PHP array ($data) to valid Multipart using BodyMultipart + * + * @param array $data + * + * @return void + */ + public function multipart(array $data): void + { + $multipart = new BodyMultipart(); + foreach ($data as $key => $value) { + $multipart->setPart($key, $value); + } + + $this + ->send($multipart->exportBody()); + } + + /** + * JSON + * + * This helper is for sending JSON HTTP response. + * It sets relevant content type header ('application/json') and convert a PHP array ($data) to valid JSON using native json_encode + * + * @see http://en.wikipedia.org/wiki/JSON + * + * @param mixed $data + * @return void + */ + public function json($data): void + { + if (!is_array($data) && !$data instanceof \stdClass) { + throw new \Exception('Invalid JSON input var'); + } + + $this + ->setContentType(Response::CONTENT_TYPE_JSON, self::CHARSET_UTF8) + ->send(\json_encode($data, JSON_UNESCAPED_UNICODE | JSON_THROW_ON_ERROR)); + } + /** * @return array */ diff --git a/tests/e2e/Services/Functions/FunctionsCustomServerTest.php b/tests/e2e/Services/Functions/FunctionsCustomServerTest.php index f52fba051e..6f3b3d0c52 100644 --- a/tests/e2e/Services/Functions/FunctionsCustomServerTest.php +++ b/tests/e2e/Services/Functions/FunctionsCustomServerTest.php @@ -1403,8 +1403,8 @@ class FunctionsCustomServerTest extends Scope 'body' => null, ]); - $this->assertEquals(500, $execution['headers']['status-code']); - $this->assertStringContainsString('Execution resulted in binary response, but JSON response does not allow binaries.', $execution['body']['type']); + $this->assertEquals(400, $execution['headers']['status-code']); + $this->assertStringContainsString('Failed to parse binary response', $execution['body']['message']); // Cleanup : Delete function $response = $this->client->call(Client::METHOD_DELETE, '/functions/' . $functionId, [ @@ -1499,9 +1499,9 @@ class FunctionsCustomServerTest extends Scope $executionBody = json_decode($execution['body'], true); - $this->assertEquals(200, $execution['headers']['status-code']); + $this->assertEquals(201, $execution['headers']['status-code']); $this->assertEquals(\md5($bytes), $executionBody['responseBody']); - $this->assertEquals($execution['headers']['content-type'], 'application/json'); + $this->assertStringStartsWith('application/json', $execution['headers']['content-type']); /** * Test for FAILURE From 066dc5906461b2bbe9f08ae8d1895ff83f7fb7b4 Mon Sep 17 00:00:00 2001 From: Khushboo Verma <43381712+vermakhushboo@users.noreply.github.com> Date: Thu, 8 Aug 2024 13:49:33 +0530 Subject: [PATCH 06/15] Update error message --- src/Appwrite/Utopia/Response.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Appwrite/Utopia/Response.php b/src/Appwrite/Utopia/Response.php index f0dace191e..6d8c671d89 100644 --- a/src/Appwrite/Utopia/Response.php +++ b/src/Appwrite/Utopia/Response.php @@ -562,7 +562,7 @@ class Response extends SwooleResponse try { $this->json(!empty($output) ? $output : new \stdClass()); } catch (JsonException $e) { - throw new Exception('Failed to parse binary response: ' . $e->getMessage(), 400); + throw new Exception('Failed to parse response: ' . $e->getMessage(), 400); } break; From e6473db96cc0e89c2e5f2dab3bf4ee15aca65e73 Mon Sep 17 00:00:00 2001 From: Khushboo Verma <43381712+vermakhushboo@users.noreply.github.com> Date: Thu, 8 Aug 2024 15:42:58 +0530 Subject: [PATCH 07/15] Update create execution body limit to 10MB --- app/controllers/api/functions.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/api/functions.php b/app/controllers/api/functions.php index 5fd3519da4..feaabb0f47 100644 --- a/app/controllers/api/functions.php +++ b/app/controllers/api/functions.php @@ -1601,7 +1601,7 @@ App::post('/v1/functions/:functionId/executions') ->label('sdk.response.type', Response::CONTENT_TYPE_JSON) ->label('sdk.response.model', Response::MODEL_EXECUTION) ->param('functionId', '', new UID(), 'Function ID.') - ->param('body', '', new Text(20971520, 0), 'HTTP body of execution. Default value is empty string.', true) + ->param('body', '', new Text(10485760, 0), 'HTTP body of execution. Default value is empty string.', true) ->param('async', false, new Boolean(), 'Execute code in the background. Default value is false.', true) ->param('path', '/', new Text(2048), 'HTTP path of execution. Path can include query params. Default value is /', true) ->param('method', 'POST', new Whitelist(['GET', 'POST', 'PUT', 'PATCH', 'DELETE', 'OPTIONS'], true), 'HTTP method of execution. Default value is GET.', true) From deeffa2a6b80f6b33ce22c748f1ad979bab279fc Mon Sep 17 00:00:00 2001 From: Khushboo Verma <43381712+vermakhushboo@users.noreply.github.com> Date: Fri, 9 Aug 2024 20:38:50 +0530 Subject: [PATCH 08/15] Updated test --- tests/e2e/Services/Functions/FunctionsCustomServerTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/e2e/Services/Functions/FunctionsCustomServerTest.php b/tests/e2e/Services/Functions/FunctionsCustomServerTest.php index 6f3b3d0c52..1e07c3b8e0 100644 --- a/tests/e2e/Services/Functions/FunctionsCustomServerTest.php +++ b/tests/e2e/Services/Functions/FunctionsCustomServerTest.php @@ -1404,7 +1404,7 @@ class FunctionsCustomServerTest extends Scope ]); $this->assertEquals(400, $execution['headers']['status-code']); - $this->assertStringContainsString('Failed to parse binary response', $execution['body']['message']); + $this->assertStringContainsString('Failed to parse response', $execution['body']['message']); // Cleanup : Delete function $response = $this->client->call(Client::METHOD_DELETE, '/functions/' . $functionId, [ From c8d04c11eb947ee9a7e530d73339dd6ed5017057 Mon Sep 17 00:00:00 2001 From: Khushboo Verma <43381712+vermakhushboo@users.noreply.github.com> Date: Sun, 11 Aug 2024 02:25:03 +0530 Subject: [PATCH 09/15] Added more tests to headers validator --- src/Appwrite/Functions/Validator/Headers.php | 22 ++++++-- src/Appwrite/Utopia/Response.php | 2 +- .../Functions/FunctionsCustomServerTest.php | 45 +--------------- .../unit/Functions/Validator/HeadersTest.php | 51 +++++++++++++++++++ 4 files changed, 71 insertions(+), 49 deletions(-) diff --git a/src/Appwrite/Functions/Validator/Headers.php b/src/Appwrite/Functions/Validator/Headers.php index 99f2d74249..fcda90e6a3 100644 --- a/src/Appwrite/Functions/Validator/Headers.php +++ b/src/Appwrite/Functions/Validator/Headers.php @@ -47,21 +47,33 @@ class Headers extends Validator $value = \json_decode($value, true); } - if (\json_last_error() == JSON_ERROR_NONE) { + if (json_last_error() !== JSON_ERROR_NONE) { + return false; + } else { if (\is_array($value)) { foreach ($value as $key => $val) { - // Check for invalid characters in key and value - if (!preg_match('/^[a-zA-Z0-9_-]+$/', $key)) { + // Reject non-string keys + if (!\is_string($key) || \strlen($key) === 0) { + return false; + } + + // Check if the key is a single character and ensure it is an alphabetic character + if (\strlen($key) === 1 && !preg_match('/^[a-zA-Z]$/', $key)) { + return false; + } + + // Check for invalid characters in keys longer than one character + if (\strlen($key) > 1 && !preg_match('/^[a-zA-Z0-9][a-zA-Z0-9-]*[a-zA-Z0-9]$/', $key)) { return false; } // Check for x-appwrite- prefix - if (0 === strpos($key, 'x-appwrite-')) { + if (str_starts_with($key, 'x-appwrite-')) { return false; } } } + return true; } - return true; } /** diff --git a/src/Appwrite/Utopia/Response.php b/src/Appwrite/Utopia/Response.php index 6d8c671d89..a71e9a1e8d 100644 --- a/src/Appwrite/Utopia/Response.php +++ b/src/Appwrite/Utopia/Response.php @@ -743,7 +743,7 @@ class Response extends SwooleResponse public function json($data): void { if (!is_array($data) && !$data instanceof \stdClass) { - throw new \Exception('Invalid JSON input var'); + throw new \Exception('Response body is not a valid JSON object.'); } $this diff --git a/tests/e2e/Services/Functions/FunctionsCustomServerTest.php b/tests/e2e/Services/Functions/FunctionsCustomServerTest.php index 1e07c3b8e0..e4f7ced2d2 100644 --- a/tests/e2e/Services/Functions/FunctionsCustomServerTest.php +++ b/tests/e2e/Services/Functions/FunctionsCustomServerTest.php @@ -1350,23 +1350,7 @@ class FunctionsCustomServerTest extends Scope $deploymentId = $deployment['body']['$id'] ?? ''; $this->assertEquals(202, $deployment['headers']['status-code']); - // Poll until deployment is built - while (true) { - $deployment = $this->client->call(Client::METHOD_GET, '/functions/' . $function['body']['$id'] . '/deployments/' . $deploymentId, [ - 'content-type' => 'application/json', - 'x-appwrite-project' => $this->getProject()['$id'], - 'x-appwrite-key' => $this->getProject()['apiKey'], - ]); - - if ( - $deployment['headers']['status-code'] >= 400 - || \in_array($deployment['body']['status'], ['ready', 'failed']) - ) { - break; - } - - \sleep(1); - } + $this->awaitDeploymentIsBuilt($function['body']['$id'], $deploymentId, checkForSuccess: false); $deployment = $this->client->call(Client::METHOD_PATCH, '/functions/' . $functionId . '/deployments/' . $deploymentId, array_merge([ 'content-type' => 'application/json', @@ -1438,16 +1422,6 @@ class FunctionsCustomServerTest extends Scope $this->assertEquals(201, $function['headers']['status-code']); - $variable = $this->client->call(Client::METHOD_POST, '/functions/' . $functionId . '/variables', array_merge([ - 'content-type' => 'application/json', - 'x-appwrite-project' => $this->getProject()['$id'], - ], $this->getHeaders()), [ - 'key' => 'CUSTOM_VARIABLE', - 'value' => 'variable', - ]); - - $this->assertEquals(201, $variable['headers']['status-code']); - $deployment = $this->client->call(Client::METHOD_POST, '/functions/' . $functionId . '/deployments', array_merge([ 'content-type' => 'multipart/form-data', 'x-appwrite-project' => $this->getProject()['$id'], @@ -1460,22 +1434,7 @@ class FunctionsCustomServerTest extends Scope $deploymentId = $deployment['body']['$id'] ?? ''; $this->assertEquals(202, $deployment['headers']['status-code']); - while (true) { - $deployment = $this->client->call(Client::METHOD_GET, '/functions/' . $function['body']['$id'] . '/deployments/' . $deploymentId, [ - 'content-type' => 'application/json', - 'x-appwrite-project' => $this->getProject()['$id'], - 'x-appwrite-key' => $this->getProject()['apiKey'], - ]); - - if ( - $deployment['headers']['status-code'] >= 400 - || \in_array($deployment['body']['status'], ['ready', 'failed']) - ) { - break; - } - - \sleep(1); - } + $this->awaitDeploymentIsBuilt($function['body']['$id'], $deploymentId, checkForSuccess: false); $deployment = $this->client->call(Client::METHOD_PATCH, '/functions/' . $functionId . '/deployments/' . $deploymentId, array_merge([ 'content-type' => 'application/json', diff --git a/tests/unit/Functions/Validator/HeadersTest.php b/tests/unit/Functions/Validator/HeadersTest.php index b06a71cb37..57baa1949c 100644 --- a/tests/unit/Functions/Validator/HeadersTest.php +++ b/tests/unit/Functions/Validator/HeadersTest.php @@ -44,5 +44,56 @@ class HeadersTest extends TestCase 'header/////Key' => 'headerValue', ]; $this->assertEquals($this->object->isValid($headers), false); + + $headers = [ + 'Content-Type' => 'application/json', + 'X-Custom-Header' => 'value' + ]; + $this->assertEquals($this->object->isValid($headers), true); + + $headers = [ + 'X-Custom-Header_With-Hyphens_and_Underscores' => 'value' + ]; + $this->assertFalse($this->object->isValid($headers)); + + $headers = [ + 'X-Header-123' => 'value' + ]; + $this->assertTrue($this->object->isValid($headers)); + + $headers = [ + 'X-Header<>' => 'value' + ]; + $this->assertFalse($this->object->isValid($headers)); + + $headers = [ + 'X Header' => 'value' + ]; + $this->assertFalse($this->object->isValid($headers)); + + $headers = [ + '' => 'value' + ]; + $this->assertFalse($this->object->isValid($headers)); + + $headers = [ + null => 'value', + ]; + $this->assertFalse($this->object->isValid($headers)); + + $headers = [ + 'X-Header' => null, + ]; + $this->assertTrue($this->object->isValid($headers)); + + $headers = [ + true => 'value', + ]; + $this->assertFalse($this->object->isValid($headers)); + + $headers = [ + 'a' => 'b', + ]; + $this->assertTrue($this->object->isValid($headers)); } } From 8667e9f821f22a68a0d6ef073120116ae5d8c8b1 Mon Sep 17 00:00:00 2001 From: Khushboo Verma <43381712+vermakhushboo@users.noreply.github.com> Date: Sun, 11 Aug 2024 17:56:02 +0530 Subject: [PATCH 10/15] Make application/json as default --- app/controllers/api/functions.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/controllers/api/functions.php b/app/controllers/api/functions.php index 7139afa626..00b5c10656 100644 --- a/app/controllers/api/functions.php +++ b/app/controllers/api/functions.php @@ -1952,12 +1952,12 @@ App::post('/v1/functions/:functionId/executions') $execution->setAttribute('responseBody', $executionResponse['body'] ?? ''); $execution->setAttribute('responseHeaders', $headers); - $isJson = false; + $isJson = true; $acceptTypes = \explode(', ', $request->getHeader('accept', 'application/json')); foreach ($acceptTypes as $acceptType) { - if (\str_starts_with($acceptType, 'application/json') || \str_starts_with($acceptType, 'application/*')) { - $isJson = true; + if (\str_starts_with($acceptType, 'multipart/form-data') || \str_starts_with($acceptType, 'multipart/*')) { + $isJson = false; break; } } From a8973ab26bde36494707fcfcd1db9d9d8190d21d Mon Sep 17 00:00:00 2001 From: Khushboo Verma <43381712+vermakhushboo@users.noreply.github.com> Date: Sun, 11 Aug 2024 18:15:10 +0530 Subject: [PATCH 11/15] Improved header tests --- src/Appwrite/Functions/Validator/Headers.php | 4 ++++ tests/unit/Functions/Validator/HeadersTest.php | 9 +++++++++ 2 files changed, 13 insertions(+) diff --git a/src/Appwrite/Functions/Validator/Headers.php b/src/Appwrite/Functions/Validator/Headers.php index fcda90e6a3..ec129f57b6 100644 --- a/src/Appwrite/Functions/Validator/Headers.php +++ b/src/Appwrite/Functions/Validator/Headers.php @@ -47,6 +47,10 @@ class Headers extends Validator $value = \json_decode($value, true); } + if (!\is_array($value)) { + return false; + } + if (json_last_error() !== JSON_ERROR_NONE) { return false; } else { diff --git a/tests/unit/Functions/Validator/HeadersTest.php b/tests/unit/Functions/Validator/HeadersTest.php index 57baa1949c..b1f60a9aad 100644 --- a/tests/unit/Functions/Validator/HeadersTest.php +++ b/tests/unit/Functions/Validator/HeadersTest.php @@ -95,5 +95,14 @@ class HeadersTest extends TestCase 'a' => 'b', ]; $this->assertTrue($this->object->isValid($headers)); + + $headers = 123; + $this->assertFalse($this->object->isValid($headers)); + + $headers = true; + $this->assertFalse($this->object->isValid($headers)); + + $headers = 'string'; + $this->assertFalse($this->object->isValid($headers)); } } From f6530fd6a57c8796ad8af3a6a338c482c792bcb4 Mon Sep 17 00:00:00 2001 From: Khushboo Verma <43381712+vermakhushboo@users.noreply.github.com> Date: Sun, 11 Aug 2024 18:32:14 +0530 Subject: [PATCH 12/15] Simplified headers validator --- src/Appwrite/Functions/Validator/Headers.php | 43 +++++++++----------- 1 file changed, 20 insertions(+), 23 deletions(-) diff --git a/src/Appwrite/Functions/Validator/Headers.php b/src/Appwrite/Functions/Validator/Headers.php index ec129f57b6..febaf73cd7 100644 --- a/src/Appwrite/Functions/Validator/Headers.php +++ b/src/Appwrite/Functions/Validator/Headers.php @@ -43,41 +43,38 @@ class Headers extends Validator return true; } - if (\is_string($value)) { - $value = \json_decode($value, true); - } - if (!\is_array($value)) { return false; } if (json_last_error() !== JSON_ERROR_NONE) { return false; - } else { - if (\is_array($value)) { - foreach ($value as $key => $val) { - // Reject non-string keys - if (!\is_string($key) || \strlen($key) === 0) { - return false; - } + } - // Check if the key is a single character and ensure it is an alphabetic character - if (\strlen($key) === 1 && !preg_match('/^[a-zA-Z]$/', $key)) { - return false; - } + if (\is_array($value)) { + foreach ($value as $key => $val) { + // Reject non-string keys + if (!\is_string($key) || \strlen($key) === 0) { + return false; + } - // Check for invalid characters in keys longer than one character - if (\strlen($key) > 1 && !preg_match('/^[a-zA-Z0-9][a-zA-Z0-9-]*[a-zA-Z0-9]$/', $key)) { - return false; - } - // Check for x-appwrite- prefix - if (str_starts_with($key, 'x-appwrite-')) { - return false; - } + // Check if the key is a single character and ensure it is an alphabetic character + if (\strlen($key) === 1 && !preg_match('/^[a-zA-Z]$/', $key)) { + return false; + } + + // Check for invalid characters in keys longer than one character + if (\strlen($key) > 1 && !preg_match('/^[a-zA-Z0-9][a-zA-Z0-9-]*[a-zA-Z0-9]$/', $key)) { + return false; + } + // Check for x-appwrite- prefix + if (str_starts_with($key, 'x-appwrite-')) { + return false; } } return true; } + return false; } /** From d895e62c81080c050a6f7d5906ebc3d1d379d45b Mon Sep 17 00:00:00 2001 From: Khushboo Verma <43381712+vermakhushboo@users.noreply.github.com> Date: Sun, 11 Aug 2024 23:06:05 +0530 Subject: [PATCH 13/15] Fixed GraphQL tests --- app/controllers/api/functions.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/app/controllers/api/functions.php b/app/controllers/api/functions.php index 00b5c10656..e34f77ae9a 100644 --- a/app/controllers/api/functions.php +++ b/app/controllers/api/functions.php @@ -1952,19 +1952,19 @@ App::post('/v1/functions/:functionId/executions') $execution->setAttribute('responseBody', $executionResponse['body'] ?? ''); $execution->setAttribute('responseHeaders', $headers); - $isJson = true; - - $acceptTypes = \explode(', ', $request->getHeader('accept', 'application/json')); + $acceptTypes = \explode(', ', $request->getHeader('accept')); foreach ($acceptTypes as $acceptType) { - if (\str_starts_with($acceptType, 'multipart/form-data') || \str_starts_with($acceptType, 'multipart/*')) { - $isJson = false; + if(\str_starts_with($acceptType, 'application/json') || \str_starts_with($acceptType, 'application/*')) { + $response->setContentType(Response::CONTENT_TYPE_JSON); + break; + } elseif (\str_starts_with($acceptType, 'multipart/form-data') || \str_starts_with($acceptType, 'multipart/*')) { + $response->setContentType(Response::CONTENT_TYPE_MULTIPART); break; } } $response ->setStatusCode(Response::STATUS_CODE_CREATED) - ->setContentType($isJson ? Response::CONTENT_TYPE_JSON : Response::CONTENT_TYPE_MULTIPART) ->dynamic($execution, Response::MODEL_EXECUTION); }); From 285b48a9e8b015b5c53d7c649a7fd268aed5994e Mon Sep 17 00:00:00 2001 From: Khushboo Verma <43381712+vermakhushboo@users.noreply.github.com> Date: Mon, 12 Aug 2024 15:39:35 +0530 Subject: [PATCH 14/15] Replace regex with ctype function --- src/Appwrite/Functions/Validator/Headers.php | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/Appwrite/Functions/Validator/Headers.php b/src/Appwrite/Functions/Validator/Headers.php index febaf73cd7..0d41493509 100644 --- a/src/Appwrite/Functions/Validator/Headers.php +++ b/src/Appwrite/Functions/Validator/Headers.php @@ -53,20 +53,24 @@ class Headers extends Validator if (\is_array($value)) { foreach ($value as $key => $val) { + $length = \strlen($key); // Reject non-string keys - if (!\is_string($key) || \strlen($key) === 0) { + if (!\is_string($key) || $length === 0) { return false; } - // Check if the key is a single character and ensure it is an alphabetic character - if (\strlen($key) === 1 && !preg_match('/^[a-zA-Z]$/', $key)) { + // Check first and last character + if (!ctype_alnum($key[0]) || !ctype_alnum($key[$length - 1])) { return false; } - // Check for invalid characters in keys longer than one character - if (\strlen($key) > 1 && !preg_match('/^[a-zA-Z0-9][a-zA-Z0-9-]*[a-zA-Z0-9]$/', $key)) { - return false; + // Check middle characters + for ($i = 1; $i < $length - 1; $i++) { + if (!ctype_alnum($key[$i]) && $key[$i] !== '-') { + return false; + } } + // Check for x-appwrite- prefix if (str_starts_with($key, 'x-appwrite-')) { return false; From cd5a9c5c84ae0bda077b67a1116e5d8db1b7c517 Mon Sep 17 00:00:00 2001 From: Khushboo Verma <43381712+vermakhushboo@users.noreply.github.com> Date: Mon, 12 Aug 2024 23:41:33 +0530 Subject: [PATCH 15/15] Update proxy version --- docker-compose.yml | 2 +- src/Appwrite/Functions/Validator/Headers.php | 4 ---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index 8fbad2f652..53f6509161 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -923,7 +923,7 @@ services: hostname: proxy <<: *x-logging stop_signal: SIGINT - image: openruntimes/proxy:0.3.1 + image: openruntimes/proxy:0.5.4 networks: - appwrite - runtimes diff --git a/src/Appwrite/Functions/Validator/Headers.php b/src/Appwrite/Functions/Validator/Headers.php index 0d41493509..c63fb04549 100644 --- a/src/Appwrite/Functions/Validator/Headers.php +++ b/src/Appwrite/Functions/Validator/Headers.php @@ -47,10 +47,6 @@ class Headers extends Validator return false; } - if (json_last_error() !== JSON_ERROR_NONE) { - return false; - } - if (\is_array($value)) { foreach ($value as $key => $val) { $length = \strlen($key);