Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make compatibility with IE easier #31

Closed
simplicitytrade opened this issue Jan 29, 2019 · 4 comments
Closed

Make compatibility with IE easier #31

simplicitytrade opened this issue Jan 29, 2019 · 4 comments
Assignees

Comments

@simplicitytrade
Copy link

simplicitytrade commented Jan 29, 2019

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.

@neomerx
Copy link
Owner

neomerx commented Jan 30, 2019

Hi,

I think the library does not work with headers directly and does not decide how it will be sent back to the client.

Mind you, the library is designed to work is a middleware, so after you've got CORS analysis results...

$cors = Analyzer::instance($this->getCorsSettings())->analyze($request);

you're free to choose how to send the headers to a client (multiple headers or comma separated).

$corsHeaders = $cors->getResponseHeaders();

// the lib has done its job at this point and the developer can decide how to send the headers

// for example,
$corsHeaders[CorsResponseHeaders::EXPOSE_HEADERS] = implode(', ', $corsHeaders[CorsResponseHeaders::EXPOSE_HEADERS]);

// and send it to client

@simplicitytrade
Copy link
Author

simplicitytrade commented Jan 30, 2019

Thanks for your quick reply! I agree with that point that the library is not responsible for setting the headers to the response.

My point is that the other headers are processed with an "implode", whereas this does not happen with the expose headers. You could say that the returned values of the individual headers do not follow a fixed scheme, like in this case.

@neomerx
Copy link
Owner

neomerx commented Jan 30, 2019

I got your point. Though it cannot be added to current version as it breaks interface contract. The method should return string[]

    /**
     * Get headers other than the simple ones that might be exposed to user agent.
     *
     * @param RequestInterface $request
     *
     * @return string[]
     */
    public function getResponseExposedHeaders(RequestInterface $request);

I'll keep it open for the next version. Meanwhile, you can have this functinality by creating the encoder as

        $analyzer = Analyzer::instance(new class extends Settings {
            public function getResponseExposedHeaders(RequestInterface $request)
            {
                return implode(', ', parent::getResponseExposedHeaders($request));
            }
        });

@neomerx neomerx changed the title Exposed-Header not accessible in IE Make compatibility with IE easier Jan 30, 2019
@neomerx neomerx removed the question label Jan 30, 2019
@neomerx neomerx mentioned this issue Feb 7, 2019
@neomerx
Copy link
Owner

neomerx commented Feb 8, 2019

Hi, it has been released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants