[php-symfony] Fix problem with clients, that put charset in content t…

…ype header. (OpenAPITools#6078) * Fix problem with clients, that put charset in content type header. With this fix header "Content-Type: application/json; charset=utf-8" working same as "Content-Type: application/json" for parse input data * Fix code style, add $consumes length check. * Add isContentTypeAllowed static method and tests * Fix old tests Right now serializer doesn't support anything beside json and xml. Call tests with application/json instead of form data. Co-authored-by: Yuriy Belenko <yura-bely@mail.ru>
MikailBag · May 31, 2020 · ca95cf6 · ca95cf6
1 parent d0cfc5b
commit ca95cf6
Show file tree

Hide file tree

Showing 15 changed files with 344 additions and 24 deletions.
diff --git a/...i-generator/src/main/java/org/openapitools/codegen/languages/PhpSymfonyServerCodegen.java b/...i-generator/src/main/java/org/openapitools/codegen/languages/PhpSymfonyServerCodegen.java
@@ -55,6 +55,7 @@ public class PhpSymfonyServerCodegen extends AbstractPhpCodegen implements Codeg
     protected String controllerDirName = "Controller";
     protected String serviceDirName = "Service";
     protected String controllerPackage;
+    protected String controllerTestsPackage;
     protected String servicePackage;
     protected Boolean phpLegacySupport = Boolean.TRUE;
 
@@ -301,6 +302,7 @@ public void processOpts() {
         additionalProperties.put("servicePackage", servicePackage);
         additionalProperties.put("apiTestsPackage", apiTestsPackage);
         additionalProperties.put("modelTestsPackage", modelTestsPackage);
+        additionalProperties.put("controllerTestsPackage", controllerTestsPackage);
 
         // make Symonfy-specific properties available
         additionalProperties.put("bundleName", bundleName);
@@ -311,11 +313,13 @@ public void processOpts() {
         // make api and model src path available in mustache template
         additionalProperties.put("apiSrcPath", "." + "/" + toSrcPath(apiPackage, srcBasePath));
         additionalProperties.put("modelSrcPath", "." + "/" + toSrcPath(modelPackage, srcBasePath));
+        additionalProperties.put("controllerSrcPath", "." + "/" + toSrcPath(controllerPackage, srcBasePath));
         additionalProperties.put("testsSrcPath", "." + "/" + toSrcPath(testsPackage, srcBasePath));
         additionalProperties.put("apiTestsSrcPath", "." + "/" + toSrcPath(apiTestsPackage, srcBasePath));
         additionalProperties.put("modelTestsSrcPath", "." + "/" + toSrcPath(modelTestsPackage, srcBasePath));
         additionalProperties.put("apiTestPath", "." + "/" + testsDirName + "/" + apiDirName);
         additionalProperties.put("modelTestPath", "." + "/" + testsDirName + "/" + modelDirName);
+        additionalProperties.put("controllerTestPath", "." + "/" + testsDirName + "/" + controllerDirName);
 
         // make api and model doc path available in mustache template
         additionalProperties.put("apiDocPath", apiDocPath);
@@ -346,6 +350,7 @@ public void processOpts() {
         supportingFiles.add(new SupportingFile("testing/phpunit.xml.mustache", "", "phpunit.xml.dist"));
         supportingFiles.add(new SupportingFile("testing/pom.xml", "", "pom.xml"));
         supportingFiles.add(new SupportingFile("testing/AppKernel.php", toSrcPath(testsPackage, srcBasePath), "AppKernel.php"));
+        supportingFiles.add(new SupportingFile("testing/ControllerTest.mustache", toSrcPath(controllerTestsPackage, srcBasePath), "ControllerTest.php"));
         supportingFiles.add(new SupportingFile("testing/test_config.yml", toSrcPath(testsPackage, srcBasePath), "test_config.yml"));
 
         supportingFiles.add(new SupportingFile("routing.mustache", configDir, "routing.yml"));
@@ -540,6 +545,7 @@ public void setInvokerPackage(String invokerPackage) {
         apiTestsPackage = testsPackage + "\\" + apiDirName;
         modelTestsPackage = testsPackage + "\\" + modelDirName;
         controllerPackage = invokerPackage + "\\" + controllerDirName;
+        controllerTestsPackage = testsPackage + "\\" + controllerDirName;
         servicePackage = invokerPackage + "\\" + serviceDirName;
     }
 

diff --git a/modules/openapi-generator/src/main/resources/php-symfony/Controller.mustache b/modules/openapi-generator/src/main/resources/php-symfony/Controller.mustache
@@ -20,6 +20,7 @@
 namespace {{controllerPackage}};
 
 use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
+use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\HttpKernel\Exception\HttpException;
 use {{servicePackage}}\SerializerInterface;
@@ -176,4 +177,40 @@ class Controller extends AbstractController
         // If we reach this point, we don't have a common ground between server and client
         return null;
     }
+
+    /**
+     * Checks whether Content-Type request header presented in supported formats.
+     *
+     * @param Request $request  Request instance.
+     * @param array   $consumes Array of supported content types.
+     *
+     * @return bool Returns true if Content-Type supported otherwise false.
+     */
+    public static function isContentTypeAllowed(Request $request, array $consumes = [])
+    {
+        if (!empty($consumes) && $consumes[0] !== '*/*') {
+            $currentFormat = $request->getContentType();
+            foreach ($consumes as $mimeType) {
+                // canonize mime type
+                if (is_string($mimeType) && false !== $pos = strpos($mimeType, ';')) {
+                    $mimeType = trim(substr($mimeType, 0, $pos));
+                }
+
+                if (!$format = $request->getFormat($mimeType)) {
+                    // add custom format to request
+                    $format = $mimeType;
+                    $request->setFormat($format, $format);
+                    $currentFormat = $request->getContentType();
+                }
+
+                if ($format === $currentFormat) {
+                    return true;
+                }
+            }
+
+            return false;
+        }
+
+        return true;
+    }
 }
diff --git a/modules/openapi-generator/src/main/resources/php-symfony/api_controller.mustache b/modules/openapi-generator/src/main/resources/php-symfony/api_controller.mustache
@@ -60,8 +60,7 @@ class {{controllerName}} extends Controller
         {{#bodyParams}}
         // Make sure that the client is providing something that we can consume
         $consumes = [{{#consumes}}'{{{mediaType}}}'{{#hasMore}}, {{/hasMore}}{{/consumes}}];
-        $inputFormat = $request->headers->has('Content-Type')?$request->headers->get('Content-Type'):$consumes[0];
-        if (!in_array($inputFormat, $consumes)) {
+        if (!static::isContentTypeAllowed($request, $consumes)) {
             // We can't consume the content that the client is sending us
             return new Response('', 415);
         }
@@ -146,6 +145,7 @@ class {{controllerName}} extends Controller
             {{#allParams}}
             {{^isFile}}
             {{#isBodyParam}}
+            $inputFormat = $request->getMimeType($request->getContentType());
             ${{paramName}} = $this->deserialize(${{paramName}}, '{{#isContainer}}{{#items}}array<{{dataType}}>{{/items}}{{/isContainer}}{{^isContainer}}{{dataType}}{{/isContainer}}', $inputFormat);
             {{/isBodyParam}}
             {{^isBodyParam}}

diff --git a/modules/openapi-generator/src/main/resources/php-symfony/testing/ControllerTest.mustache b/modules/openapi-generator/src/main/resources/php-symfony/testing/ControllerTest.mustache
@@ -0,0 +1,113 @@
+<?php
+/**
+ * ControllerTest
+ * PHP version 5
+ *
+ * @category Class
+ * @package  {{controllerTestsPackage}}
+ * @author   openapi-generator contributors
+ * @link     https://github.com/openapitools/openapi-generator
+ */
+
+{{>partial_header}}
+/**
+ * NOTE: This class is auto generated by the openapi generator program.
+ * https://github.com/openapitools/openapi-generator
+ * Please update the test case below to test the endpoint.
+ */
+
+namespace {{controllerTestsPackage}};
+
+use {{controllerPackage}}\Controller;
+use PHPUnit\Framework\TestCase;
+use Symfony\Component\HttpFoundation\Request;
+
+/**
+ * ControllerTest Class Doc Comment
+ *
+ * @category           Class
+ * @package            {{controllerTestsPackage}}
+ * @author             openapi-generator contributors
+ * @link               https://github.com/openapitools/openapi-generator
+ * @coversDefaultClass \{{controllerPackage}}\Controller
+ */
+class ControllerTest extends TestCase
+{
+
+    /**
+     * Tests isContentTypeAllowed static method.
+     *
+     * @param string $contentType
+     * @param array  $consumes
+     * @param bool   $expectedReturn
+     *
+     * @covers ::isContentTypeAllowed
+     * @dataProvider provideArgumentsForIsContentTypeAllowed
+     */
+    public function testIsContentTypeAllowed($contentType, array $consumes, $expectedReturn)
+    {
+        $request = new Request();
+        $request->headers->set('CONTENT_TYPE', $contentType, true);// last one argument overrides header
+        $this->assertSame(
+            $expectedReturn,
+            Controller::isContentTypeAllowed($request, $consumes),
+            sprintf(
+                'Failed assertion that "Content-Type: %s" %s by [%s] consumes array.',
+                $contentType,
+                ($expectedReturn) ? 'is allowed' : 'is forbidden',
+                implode(', ', $consumes)
+            )
+        );
+    }
+
+    public function provideArgumentsForIsContentTypeAllowed()
+    {
+        return [
+            'usual JSON content type' => [
+                'application/json',
+                ['application/json'],
+                true,
+            ],
+            'extended content type from PR #6078' => [
+                'application/json; charset=utf-8',
+                ['application/json'],
+                true,
+            ],
+            'more than one content types' => [
+                'application/json',
+                ['application/xml', 'application/json; charset=utf-8'],
+                true,
+            ],
+            'empty consumes array' => [
+                'application/json',
+                [],
+                true,
+            ],
+            'empty consumes and content type' => [
+                null,
+                [],
+                true,
+            ],
+            'consumes everything' => [
+                'application/json',
+                ['*/*'],
+                true,
+            ],
+            'fancy custom content type' => [
+                'foobar/foobaz',
+                ['application/xml', 'foobar/foobaz; charset=utf-8'],
+                true,
+            ],
+            'empty content type' => [
+                null,
+                ['application/xml', 'application/json; charset=utf-8'],
+                false,
+            ],
+            'content type out of consumes' => [
+                'text/html',
+                ['application/xml', 'application/json; charset=utf-8'],
+                false,
+            ],
+        ];
+    }
+}
diff --git a/modules/openapi-generator/src/main/resources/php-symfony/testing/api_test.mustache b/modules/openapi-generator/src/main/resources/php-symfony/testing/api_test.mustache
@@ -99,7 +99,7 @@ use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;
         $path = str_replace($pattern, $data, $path);
         {{/pathParams}}
 
-        $crawler = $client->request('{{httpMethod}}', $path);
+        $crawler = $client->request('{{httpMethod}}', $path{{#hasBodyParam}}, [], [], ['CONTENT_TYPE' => 'application/json']{{/hasBodyParam}});
     }
     {{/operation}}
 

diff --git a/modules/openapi-generator/src/main/resources/php-symfony/testing/phpunit.xml.mustache b/modules/openapi-generator/src/main/resources/php-symfony/testing/phpunit.xml.mustache
@@ -9,12 +9,14 @@
         <testsuite>
             <directory>{{apiTestPath}}</directory>
             <directory>{{modelTestPath}}</directory>
+            <directory>{{controllerTestPath}}</directory>
         </testsuite>
     </testsuites>
     <filter>
         <whitelist processUncoveredFilesFromWhitelist="true">
             <directory suffix=".php">{{apiSrcPath}}</directory>
             <directory suffix=".php">{{modelSrcPath}}</directory>
+            <directory suffix=".php">{{controllerSrcPath}}</directory>
         </whitelist>
     </filter>
     <php>

diff --git a/samples/server/petstore/php-symfony/SymfonyBundle-php/Controller/Controller.php b/samples/server/petstore/php-symfony/SymfonyBundle-php/Controller/Controller.php
@@ -30,6 +30,7 @@
 namespace OpenAPI\Server\Controller;
 
 use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
+use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\HttpKernel\Exception\HttpException;
 use OpenAPI\Server\Service\SerializerInterface;
@@ -186,4 +187,40 @@ protected function getOutputFormat($accept, array $produced)
         // If we reach this point, we don't have a common ground between server and client
         return null;
     }
+
+    /**
+     * Checks whether Content-Type request header presented in supported formats.
+     *
+     * @param Request $request  Request instance.
+     * @param array   $consumes Array of supported content types.
+     *
+     * @return bool Returns true if Content-Type supported otherwise false.
+     */
+    public static function isContentTypeAllowed(Request $request, array $consumes = [])
+    {
+        if (!empty($consumes) && $consumes[0] !== '*/*') {
+            $currentFormat = $request->getContentType();
+            foreach ($consumes as $mimeType) {
+                // canonize mime type
+                if (is_string($mimeType) && false !== $pos = strpos($mimeType, ';')) {
+                    $mimeType = trim(substr($mimeType, 0, $pos));
+                }
+
+                if (!$format = $request->getFormat($mimeType)) {
+                    // add custom format to request
+                    $format = $mimeType;
+                    $request->setFormat($format, $format);
+                    $currentFormat = $request->getContentType();
+                }
+
+                if ($format === $currentFormat) {
+                    return true;
+                }
+            }
+
+            return false;
+        }
+
+        return true;
+    }
 }
diff --git a/samples/server/petstore/php-symfony/SymfonyBundle-php/Controller/PetController.php b/samples/server/petstore/php-symfony/SymfonyBundle-php/Controller/PetController.php
@@ -62,8 +62,7 @@ public function addPetAction(Request $request)
     {
         // Make sure that the client is providing something that we can consume
         $consumes = ['application/json', 'application/xml'];
-        $inputFormat = $request->headers->has('Content-Type')?$request->headers->get('Content-Type'):$consumes[0];
-        if (!in_array($inputFormat, $consumes)) {
+        if (!static::isContentTypeAllowed($request, $consumes)) {
             // We can't consume the content that the client is sending us
             return new Response('', 415);
         }
@@ -80,6 +79,7 @@ public function addPetAction(Request $request)
 
         // Deserialize the input values that needs it
         try {
+            $inputFormat = $request->getMimeType($request->getContentType());
             $body = $this->deserialize($body, 'OpenAPI\Server\Model\Pet', $inputFormat);
         } catch (SerializerRuntimeException $exception) {
             return $this->createBadRequestResponse($exception->getMessage());
@@ -491,8 +491,7 @@ public function updatePetAction(Request $request)
     {
         // Make sure that the client is providing something that we can consume
         $consumes = ['application/json', 'application/xml'];
-        $inputFormat = $request->headers->has('Content-Type')?$request->headers->get('Content-Type'):$consumes[0];
-        if (!in_array($inputFormat, $consumes)) {
+        if (!static::isContentTypeAllowed($request, $consumes)) {
             // We can't consume the content that the client is sending us
             return new Response('', 415);
         }
@@ -509,6 +508,7 @@ public function updatePetAction(Request $request)
 
         // Deserialize the input values that needs it
         try {
+            $inputFormat = $request->getMimeType($request->getContentType());
             $body = $this->deserialize($body, 'OpenAPI\Server\Model\Pet', $inputFormat);
         } catch (SerializerRuntimeException $exception) {
             return $this->createBadRequestResponse($exception->getMessage());

diff --git a/samples/server/petstore/php-symfony/SymfonyBundle-php/Controller/StoreController.php b/samples/server/petstore/php-symfony/SymfonyBundle-php/Controller/StoreController.php
@@ -284,8 +284,7 @@ public function placeOrderAction(Request $request)
     {
         // Make sure that the client is providing something that we can consume
         $consumes = [];
-        $inputFormat = $request->headers->has('Content-Type')?$request->headers->get('Content-Type'):$consumes[0];
-        if (!in_array($inputFormat, $consumes)) {
+        if (!static::isContentTypeAllowed($request, $consumes)) {
             // We can't consume the content that the client is sending us
             return new Response('', 415);
         }
@@ -308,6 +307,7 @@ public function placeOrderAction(Request $request)
 
         // Deserialize the input values that needs it
         try {
+            $inputFormat = $request->getMimeType($request->getContentType());
             $body = $this->deserialize($body, 'OpenAPI\Server\Model\Order', $inputFormat);
         } catch (SerializerRuntimeException $exception) {
             return $this->createBadRequestResponse($exception->getMessage());