[
https://issues.apache.org/jira/browse/QPID-7300?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Alex Rudyy updated QPID-7300:
-----------------------------
Assignee: Lorenz Quack
Status: Open (was: Reviewable)
Lorens,
Here are my review comments for the commits
[r1752129|https://svn.apache.org/r1752129] and
[r1752130|https://svn.apache.org/r1752130]:
# Jetty CORS filter validates the the preflight request values listed in
"Access-Control-Request-Headers" against headers specified in
"corsAllowHeaders". If request header is not present in "corsAllowHeaders",
the request fails. If enduser will modify the "corsAllowHeaders" and miss any
of "simple headers", that would make all cors requests failing, , for example
if "Origin" is not in the "corsAllowHeaders", the cors preflight requests would
fail as filter implementation will not allow request having header "Origin"
(which is mandatory by spec). IMHO, if user misses any simple header in
"corsAllowHeaders" value, the HttpManagement should add all missed simple
headers to filter. Another approach would be to disallow modification of
"corsAllowHeaders" by users. Adding validation to check that all simple headers
are present could be another solution.
# Jetty CORS filter implementation sets headers listed in "corsAllowHeaders" as
value for response header "Access-Control-Allow-Headers" which is wrong as spec
requires listing in "Access-Control-Allow-Headers" only response headers, for
example Cache-Control, Content-Language, Content-Type, Expires,
Last-Modified, Pragma. Listing request headers in
"Access-Control-Allow-Headers" looks like a bug to me. However, I do not think
that it would impact CORS communication. I think that we can ignore this issue
for now.
# The default value for "corsAllowHeaders" is set to
"Content-Type,Accept,Origin,X-Requested-With". We do not use header
"X-Requested-With" in Qpid as far as I know. What is the reason for inclusion
of "X-Requested-With" into "corsAllowHeaders" by default?
# Pagination for messages relies on http header "X-Range". This headers should
be always added to the "corsAllowHeaders" and set in filter even if not
specified explicitly by user.
# I am not convinced that we need to allow to end-user the modification of
"corsAllowHeaders", "corsAllowMethods" and "corsAllowCredentials". IMHO,
settings them to incorrect values would make failing incoming cors requests. I
would personally make these attributes derived. If we are really required the
ability to override this attributes I would prefer to use context variables.
> [Java Broker] The REST API should support Cross Origin Resource Sharing (CORS)
> ------------------------------------------------------------------------------
>
> Key: QPID-7300
> URL: https://issues.apache.org/jira/browse/QPID-7300
> Project: Qpid
> Issue Type: New Feature
> Components: Java Broker
> Reporter: Lorenz Quack
> Assignee: Lorenz Quack
> Fix For: qpid-java-6.1
>
>
> Currently the broker has an embedded Web Management Console to manage the
> broker using the REST API.
> However, it is not possible to have externally hosted web management consoles
> due to the same origin policy enforced by the browser.
> The Broker should support [Cross Origin Resource Sharing
> (CORS)|https://www.w3.org/TR/cors/] to allow the broker to be managed through
> externally hosted web management consoles.
> The parameters of CORS should be configurable.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
