Headers being set incorrectly by CorsMiddleware.

[ADmad]

The CorsMiddleware incorrectly sets response headers first and then calls $next(). This causes problem when an exception is thrown; the headers are lost since a new response instance is created by the exception render. To avoid this problem you should set the headers on response instance after calling $next().

[lpj145]

i try it, and when middleware call $next first, app stop and not continue runing middleware code. *** It occur only exception's.

[bravo-kernel]

As I pointed out before there is a good example of manipulating after next right here: https://github.com/UseMuffin/Throttle/blob/master/src/Middleware/ThrottleMiddleware.php#L57

[ADmad]

This CORS middleware lib also does the same. Gets response instance first by calling $next() and then sets the CORS headers.

[ozee31]

Thank you for your issue, I look at this ASAP

[lpj145]

@ADmad CORS you mentioned not works again, this middleware works all frameworks with PSR7 Middleware, but this not works again, i think this cause by throw, but i don't understand how does it works.

[eymen-elkum]

My working case:

1. I am using ApiListener
2. In config/app.php:

   'Error' => [
       'errorLevel' => E_ALL,
       'exceptionRenderer' => 'Crud\Error\ExceptionRenderer',
       'skipLog' => [],
       'log' => true,
       'trace' => true,
   ],
   . . . . . 
   'Cors' => [
       'exceptionRenderer' => false
   ],

3. In config/bootstrap.php:

   Plugin::load('Cors', ['bootstrap' => true, 'routes' => false]);

4. In src / Controller / ErrorController.php:

   public function beforeRender(Event $event)
   {
       parent::beforeRender($event);
   
       //note that I used the deprecated header function
       $this->response->header(['Access-Control-Allow-Origin' => '*']);
       //$this->response->withHeader('Access-Control-Allow-Origin', '*');
       $this->viewBuilder()->setTemplatePath('Error');
   }

5. In my src / Controller / Api / ApiController.php:

   
       $this->loadComponent('Crud.Crud', [
           'actions' => [
               'Crud.Index',
               'Crud.View',
               'Crud.Add',
               'Crud.Edit',
               'Crud.Delete'
           ],
           'listeners' => [
               'Crud.Api',
               'Crud.ApiPagination',
               'Crud.RelatedModels',
               'Crud.ApiQueryLog'
           ]
       ]);

In my case the errors working as expected, but note that I used the deprecated function 

[eymen-elkum]

@ozee31 I suggest to revert my PR #6 for now

[ADmad]

@ayman-alkom The mistake in the readme PR is the result of withHeader() needs to be assigned back to $this->response.

[eymen-elkum]

@ADmad how can we do that? like this?

$this->response = $this->response->withHeader('Access-Control-Allow-Origin', '*');

[ADmad]

Yes.

[eymen-elkum]

I may tried it and didn't work, but I will try again

[ozee31]

I have fixed $this->response = $this->response->withHeader('Access-Control-Allow-Origin', '*'); in a new release.

[bravo-kernel]

@ozee31 I cannot test because my demo/blog app does not have an App\Controller\ErrorController (and thus crashes). Would it be possible to only extend if it exists? Not sure if Cake does this too but SO mentions this possible solution: http://stackoverflow.com/questions/17324871/extend-php-class-only-if-other-class-is-defined.

[ADmad]

@ozee31 You should still do the change suggested in my original post.

[ozee31]

I am watching

[bravo-kernel]

@ozee31 just tested and the release does not fix the problem yet

[lpj145]

ok.... i try much options... but no solution... i make new middleware but no solution. My solution is going edit JsonApiException and copy olders responses to new reponses with Exception... i try try try... this is expensive for me. In last week, just doing this.

[lpj145]

the problem is throw, if middleware run $next() and found throw, code stop and die with new exception JsonApiException. JsonApiException prepare json.vnd/application but no prepare other headers.

[eymen-elkum]

@lpj145 my project is working well, but with the ApiListener and not JsonApiListener. Try it and tell me if that is working

[ozee31]

I have the same problem with the default configuration without using Cors ErrorsController. All my attempts are for the moment a failure with an Exception

[bravo-kernel]

This does indeed seem to work with the ApiListener.

First created this empty App/Controller/ErrorController:

<?php
namespace App\Controller;

use Cake\Controller\ErrorController as BaseErrorController;

class ErrorController extends BaseErrorController
{
}

ApiListener

Update: False positive, it seemed the image below was not using the ApiListener ExceptionRenderer at all. The response json is produced by default CakePHP fallback.

ApiListener seems to work as expected:

• Access-Control-Allow-Origin header added to the response (OK)
• ApiListener produces JSON response with validation error message (OK)

JsonApiListener

First test

Only changing the listener from ApiListener to JsonApiListener:

• Access-Control-Allow-Origin header added to the response (OK)
• no JSON API response (NOK)

Second test

Changing the ExceptionRenderer in app.php to;

    'Cors' => [
        'exceptionRenderer' => '\Crud\Error\JsonApiExceptionRenderer'
    ],

• Access-Control-Allow-Origin header is missing from the response (NOK)
• JsonApiListener produces JSON API response with validation error message (OK)

[bravo-kernel]

I will try and investigate the JsonApiListener the coming days.

[ozee31]

Hey,

Can you test branch https://github.com/ozee31/cakephp-cors/tree/v1.1.0

The Cors ExceptionRenderer extends class dynamicly and add Cors Header. I removed the ErrorController.

in you app.php use :

'Error' => [
    'errorLevel' => E_ALL,
    'exceptionRenderer' => 'Crud\Error\ExceptionRenderer', // Or an other
    'skipLog' => [],
    'log' => true,
    'trace' => true,
],

'Cors' => [
    'exceptionRenderer' => 'Cors\Error\AppExceptionRenderer', // not false because add the Header and extend 'Error.execptionRenderer
]

[lpj145]

@ozee31 exceptionRenderer needed JsonApiException but not have true data structure.

[lpj145]

i try it, but no have body... please, Cors not have problem, JsonApiException works always create new Controller, i debug it and found error with throw.

[bravo-kernel]

@ozee31 just tested and.... your v1.1.0 solves the problem 🍰 💃

Working app.php config for JsonApiListener:

    'Error' => [
        'errorLevel' => E_ALL & ~E_DEPRECATED,
        'exceptionRenderer' => 'Crud\Error\JsonApiExceptionRenderer',
        'skipLog' => [],
        'log' => true,
        'trace' => true,
    ],

    /**
     * Cors required to make sure headers are added on (validation) errors as well.
     */
     'Cors' => [
         'exceptionRenderer' => '\Cors\Error\AppExceptionRenderer'
     ],

[lpj145]

@ozee31 this works for me. Thanks.

[bravo-kernel]

@ozee31 could you please create a new release so I can update blog and close this issue.

[ozee31]

Yes i publish new release ASAP

[ozee31]

It's released https://github.com/ozee31/cakephp-cors/releases/tag/v1.1.0

[bravo-kernel]

👍