This package has framework agnostic Cross-Origin Resource Sharing (CORS) implementation.

When the request header 'Host' value is in form of Host: example.com (without port) the parse_url method isn't capable of parsing the url correctly.

Feeding a url without a prefixed '' to parse_url will result in a non discovered domain name if the port is not present.

But I think a Host header in this form should parseable as url - or am I mistaking?

Host: example.com

https://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.23 https://github.com/neomerx/cors-psr7/blob/master/src/Analyzer.php#L256 https://github.com/neomerx/cors-psr7/blob/master/tests/Http/ParsedUrlTest.php#L59

I am trying to implement this as a middleware on Silex Framework (version 2). Silex by default does not support PSR 7 request and responses, so I use DiactorosFactory Library to convert the Symfony Foundation Request/Response to & from PSR 7 Request and Response.

In the CORS Settings, I have something like this

->setServerOrigin([
    'scheme' => 'http',
    'host'   => 'www.something.test',
    'port'   => 80,
])

And the check fails even when the call is made to the host that is configured above i.e. www.something.test.

I am making a preflight request to http://www.something.test and when it comes to Analyzer.php's isSameHost method $host get's value of www.something.test and $hostURL which should be parsed object, has host property as null.

https://github.com/neomerx/cors-psr7/blob/24944f39483d1a89f66ae9d58cca9f82b8815b35/src/Analyzer.php#L320

This is the because internally it uses php's function parse_url to parse url if it is string.

https://github.com/neomerx/cors-psr7/blob/24944f39483d1a89f66ae9d58cca9f82b8815b35/src/Http/ParsedUrl.php#L64

and after parsing www.something.test returns array which looks like

array(..) {
  ["path"]=>
  string(5) "www.something.test"
}

and thus does not have enough information to match the host.

https://github.com/neomerx/cors-psr7/blob/24944f39483d1a89f66ae9d58cca9f82b8815b35/src/Http/ParsedUrl.php#L71-L73

I am not sure why

https://github.com/neomerx/cors-psr7/blob/24944f39483d1a89f66ae9d58cca9f82b8815b35/src/Analyzer.php#L320-L321

$host where which returns www.something.test passed to createParsedURL as it does not have enough data to properly parse all the request.

Would it be possible to tag an 1.1 release?

This is because versions before 1.0.4 throw an error with PHP 7.2 because of assert('is_string($url) || is_array($url)'); in ParsedUrl.php. Currently only way of ignoring everything below 1.0.4 would be to manually list all the minor releases.

"neomerx/cors-psr7": "1.0.4|1.0.5|1.0.6|1.0.7|1.0.8|1.0.9|1.0.11|1.0.12|1.0.13",

This not optimal and requires manually updating composer.json on my side always when neomerx/cors-psr7 has a new release.

Are you planning to create a factory for Settings?

For example:

(new SettingsFactory)->createFromArray([
    'foo' => 'bar',
    'bar' => 'baz',
    'baz' => 'qux',
]);

I would like not to write this:

https://github.com/sunrise-php/awesome-skeleton/blob/master/src/Middleware/CorsMiddleware.php#L80

What are your thoughts on this?

Currently there is a config option Settings::VALUE_ALLOW_ALL_HEADERS which should allow all headers pass through CORS. It works fine for internal lib logic. No problem here. The problem is that this value * is actually sent to client in Access-Control-Allow-Headers and browser don't understand this value.

It looks the only possible way is listing all allowed headers and special * should be removed.

It was added mostly to make development easier. However since logging has been added to the lib this feature is not so important.

It is recommended avoid using Settings::VALUE_ALLOW_ALL_HEADERS and just list all allowed headers in Settings::KEY_ALLOWED_HEADERS

If using the exposed header keys they are set as an array into the http response header. As a result, there are several header lines with the same name in the response.

The Internet Explorer cannot handle the headers, even if it would be correct according to w3c specification.

In "/src/Strategies/Settings.php" the following function should look like this.

BEFORE:

public function getResponseExposedHeaders(RequestInterface $request)
    {
        return $this->getEnabledItems($this->settings[self::KEY_EXPOSED_HEADERS]);
    }

AFTER:

public function getResponseExposedHeaders(RequestInterface $request)
    {
        return implode(', ', $this->getEnabledItems($this->settings[self::KEY_EXPOSED_HEADERS]));
    }

It would be nice if you could recognize this issue and update the project. Other browsers are not be affected by changing the returned value to a comma-separated string.

What kind of test does the following code does?

     $this->settings->setServerOrigin([
        'scheme' => 'http',
        'host'   => 'example.com',
        'port'   => 123,
    ])

To pass the checks, should the request be coming from http://example.com port 123 or to http://example.com:123?

I am trying to implement the library on Silex Framework and I am trying to match the request is coming to the one I am specifying, and it is not working.

https://github.com/neomerx/cors-psr7/blob/24944f39483d1a89f66ae9d58cca9f82b8815b35/tests/AnalyzerTest.php#L62

Trying to setup package in PHP-DI container and seems like I've found small mistake in init method.

https://github.com/neomerx/cors-psr7/blob/0f968523b0b8215540915464aed08c5c4a4bf8b5/src/Strategies/Settings.php#L158-L159

Second setAllowedOrigins([]) overwrites areAllOriginsAllowed to false value, then enableAllOriginsAllowed() makes no sense one line before.

This PR initializes constants with string values instead of numeric values.

Numeric values causes problems, if the config values should be overwritten in aggregated configs.

For example:

<?php
// cors.global.php

use Neomerx\Cors\Strategies\Settings;

return [
    'cors' => [
        Settings::KEY_SERVER_ORIGIN   => '192.168.0.1',
        Settings::KEY_ALLOWED_ORIGINS => [
            'some.allowed.origin' => true,
        ],
    ],
];

<?php
// cors.local.php

use Neomerx\Cors\Strategies\Settings;

return [
    'cors' => [
        Settings::KEY_SERVER_ORIGIN   => '192.168.0.2',
        Settings::KEY_ALLOWED_ORIGINS => [
            'some.allowed.origin' => true,
        ],
    ],
];

If the two files are merged with an aggregator (like Zend ConfigAggregator), the merged config looks like

[
    'cors' => [
        0 => '192.168.0.1',
        1 => [
            'some.allowed.origin' => true,
        ],
        2 => '192.168.0.2',
        3 => [
            'some.allowed.origin' => true,
        ],
    ],
];

With string values the merged config works like expected

[
    'cors' => [
        'keyServerOrigin' => '192.168.0.2',
        'keyAllowedOrigins' => [
            'some.allowed.origin' => true,
        ],
    ],
];

As suggested in https://github.com/neomerx/cors-psr7/issues/42

@neomerx: not that you might want to enable branch protection to enforce these status checks to complete successfully, or to block the merge if they fail.

Hi

Thanks for an awesome library

Due to the use of static:: in

https://github.com/neomerx/cors-psr7/blob/0f968523b0b8215540915464aed08c5c4a4bf8b5/src/Analyzer.php#L277

Because the class constants are private they can't be accessed from the child class. And when you extend the analyzer it will automatically try to use the child scope when using static:: so it would work if you used self:: in the Analyzer class.

Would you be open to a pull-request to fix this?