CORS support

[Orkin]

No description provided.

[bakura10]

Alignemtn of *

[coveralls]

Coverage decreased (-5.43%) when pulling d34295a on Orkin:cors-support into 0b54400 on zf-fr:master.

[bakura10]

We should avoid nesting too much. Do it the other way, say:

if ($origin === null) {
return;
}

This avoid a large number of nesting.

[coveralls]

Coverage decreased (-5.32%) when pulling 5b950db on Orkin:cors-support into 0b54400 on zf-fr:master.

[coveralls]

Coverage decreased (-5.32%) when pulling 5b950db on Orkin:cors-support into 0b54400 on zf-fr:master.

[Ocramius]

ping @asm89 can you look into this since you got some experience? :)

[bakura10]

I thin there is an error in the exemple. It should be array('GET', 'POST', 'PUT'...).

[bakura10]

Except a few more CS, it looks very good to me :). Just write a few tests and documentation and I think it'll be a nice feature to have.

[coveralls]

Coverage decreased (-5.32%) when pulling 7bd338d on Orkin:cors-support into 0b54400 on zf-fr:master.

[coveralls]

Coverage decreased (-5.38%) when pulling 9b4892c on Orkin:cors-support into 0b54400 on zf-fr:master.

[coveralls]

Coverage decreased (-0.52%) when pulling c157fb6 on Orkin:cors-support into 0b54400 on zf-fr:master.

[bakura10]

Isn't there some standard about which status code a CORS request should send in case the CORS is not authorized?

[Orkin]

You prefer return 401 status code instead of nothing ?

[bakura10]

I think 401 is a great choice. Once again, please refer on the internet if there is a widely used convention for that.

[Ocramius]

401 = unauthenticated. 403 = unauthorized
On 25 Aug 2013 12:27, "Michaël Gallego" notifications@github.com wrote:

In tests/ZfrRestTest/Mvc/CorsListenerTest.php:

•    $this->corsListener->onCors($this->event);
  

•    $this->assertNotEquals(204, $this->event->getResponse()->getStatusCode());
  

• }

• public function testIfAccessControlRequestMethodIsNotInRequest()
• {

•    $request = new HttpRequest();
  

•    $request->setMethod('options');
  

•    $request->getHeaders()->addHeaderLine('Origin', 'origin-header');
  

•    $this->event->setRequest($request);
  

•    $this->corsListener->onCors($this->event);
  

•    $this->assertNotEquals(204, $this->event->getResponse()->getStatusCode());
  

I think 401 is a great choice. Once again, please refer on the internet if
there is a widely used convention for that.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/82/files#r5968031
.

[bakura10]

Ok, so 403 makes more sense then ;-). Thanks for the clarification Marco.

[coveralls]

Coverage decreased (-0.46%) when pulling 0321a2c on Orkin:cors-support into 0b54400 on zf-fr:master.

[Ocramius]

@Orkin after discussing a bit with @bakura10 we thought of moving this to a new zfr-cors library that handles only CORS stuff.

This needs a bit more of thought, but doesn't clutter zfr-rest with logic that is currently not interacting with the routes/resources at all.

What it could be (after this patch, of course - we don't want to push functionality too far):

• a simple listener on that acts before the SendResponseListener
• could allow interaction with other modules that can simply set a flag somewhere (depending on authorization or other checks)
• could interact with libraries like stack-cors to increase framework interoperability

Just some ideas, but as you can see, the CORS problem is too small for REST, and too wide for this library. That's why we'd suggest moving it out.

@bakura10 is setting up a repository and can give you commit access to it I suppose.

[coveralls]

Coverage decreased (-0.46%) when pulling a9f9140 on Orkin:cors-support into 0b54400 on zf-fr:master.

[bakura10]

I'm closing this as we'll have a ZfrCors soon.