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

CORS not working correctly #3598

Closed
manufunk opened this issue Aug 20, 2019 · 23 comments
Closed

CORS not working correctly #3598

manufunk opened this issue Aug 20, 2019 · 23 comments
Labels
kind/bug Something isn't working
Milestone

Comments

@manufunk
Copy link

manufunk commented Aug 20, 2019

Describe the bug
I have a simple REST interface that needs to be accessed by a SPA which is failing because of CORS. I checked again and tried to call my interface via POSTMAN and I do not see any cors headers.

quarkus.http.cors=true
quarkus.http.cors.origins=http://localhost:8080
quarkus.http.cors.headers=accept, origin, authorization, content-type, x-requested-with
quarkus.http.cors.methods=GET,POST,OPTIONS

However if I implement the JAX-RS filter the cors header is showing up.
Is there a known BUG or could anybody confirm with version 0.21?

@Provider
public class CORSFilter implements ContainerResponseFilter {

    // Logger
    private final Logger log = LoggerFactory.getLogger(CORSFilter.class);

    @Override
    public void filter(final ContainerRequestContext requestContext,
                       final ContainerResponseContext cres) throws IOException {
        cres.getHeaders().add("Access-Control-Allow-Origin","http://localhost:8080");
        cres.getHeaders().add("Access-Control-Allow-Headers", "accept, origin, authorization, content-type, x-requested-with");
        cres.getHeaders().add("Access-Control-Allow-Methods", "GET,POST,OPTIONS");
        cres.getHeaders().add("Access-Control-Max-Age", "1209600");
    }

}

UPDATE:
I figured out that if I set the quarkus.http.cors, the headers for the method OPTIONS are correctly set however the actual "POST" call will cause the following error (this does not happen by setting the JAX-RS header which should be not necessary):

Access to XMLHttpRequest at 'https://DOMAIN/PATH' from origin 'https://DOMAIN' has been blocked by CORS policy: No 'Access-Control-Allow-Origin' header is present on the requested resource
@manufunk manufunk added the kind/bug Something isn't working label Aug 20, 2019
@machi1990
Copy link
Member

machi1990 commented Aug 20, 2019

@Manu206 hi, thanks for reporting this issue. The past couple of days cors behaviour has been reworked and it should be much better. It is my hope that it fixes the issue you have too.

If possible, could you provide a simple producer so that I check if it is the case? Or otherwise, can you try to build a SNAPSHOT version using the master branch and see if the issue is still present?
Thanks.

@manufunk
Copy link
Author

I tested again with latest release 0.22 and still there is an issue.

Headers are still defined like:

quarkus.http.cors=true
quarkus.http.cors.origins=http://localhost:8080
quarkus.http.cors.headers=accept, origin, authorization, content-type, x-requested-with
quarkus.http.cors.methods=GET,POST,OPTIONS

Postman headers look like:

 Connection:"keep-alive"
 Access-Control-Allow-Origin:"http://localhost:8080"
 Access-Control-Allow-Credentials:"true"
 Content-Type:"application/json"
 Content-Length:"459"
 Date:"Tue, 17 Sep 2019 09:22:09 GMT"

If I set the headers via Corsfilter like mentioned earlier I get:

Access-Control-Allow-Headers:"accept, origin, authorization, content-type, x-requested-with"
Date:"Tue, 17 Sep 2019 09:19:01 GMT"
Connection:"keep-alive"
Access-Control-Allow-Origin:"http://localhost:8080"
Access-Control-Allow-Credentials:"true"
Content-Type:"application/json"
Content-Length:"459"
Access-Control-Allow-Methods:"GET,POST,OPTIONS"

Seems like Methods and Allow Headers are missing via Quarkus config.

@gsmet
Copy link
Member

gsmet commented Sep 17, 2019

I wonder if it's related to #3312 .

@Ladicek did you make progress on this issue?
@sberyozkin is this something you're familiar with and could handle?

@Ladicek
Copy link
Contributor

Ladicek commented Sep 17, 2019

This was pretty easy to fix (just had to make sure CORSFilter comes sooner than ResteasyFilter), but before I was able to find some time to write proper tests, HTTP got totally revamped in Quarkus. We need to check if this still happens.

@Ladicek
Copy link
Contributor

Ladicek commented Sep 17, 2019

And yes, #3312 is (was?) most likely the same problem.

@edewit
Copy link
Contributor

edewit commented Sep 27, 2019

This is fixed right, I can't reproduce this

@ricardozanini
Copy link

I'm seeing this error on 0.22. I can't upgrade right now, but if you get here through Google, that's what saved my life for a simple demo using this thread as a reference:

application.properties:

quarkus.http.cors=true

The custom CORSFilter:

@Provider
public class CORSFilter implements ContainerResponseFilter {

    private static final Logger LOGGER = LoggerFactory.getLogger(CORSFilter.class);

    public CORSFilter() {}

    @Override
    public void filter(ContainerRequestContext requestContext, ContainerResponseContext responseContext) throws IOException {
        LOGGER.debug("Modifing response with CORSFIlter: {}", responseContext.getHeaders());
        MultivaluedMap<String, Object> headers = responseContext.getHeaders();
        headers.putSingle("Access-Control-Allow-Origin", "*");
        LOGGER.debug("Modified to add the required header: {}", responseContext.getHeaders());
    }
}

Then the POST method has the required header:

HTTP/1.1 200 OK
Connection: keep-alive
Access-Control-Allow-Origin: *
Content-Type: application/json
Content-Length: 121
Date: Fri, 04 Oct 2019 23:22:05 GMT

The OPTIONS response header is correct:

HTTP/1.1 200 OK
Connection: keep-alive
Access-Control-Allow-Origin: http://localhost:3000
Access-Control-Allow-Headers: content-type
Access-Control-Allow-Credentials: true
Transfer-Encoding: chunked
Access-Control-Allow-Methods: POST
Date: Fri, 04 Oct 2019 23:22:05 GMT

If you guys fixed this in the newer versions, that's great news, because it's a super common use case for front end developers that are keen to implement backend services with Quarkus.

@Ladicek
Copy link
Contributor

Ladicek commented Oct 8, 2019

I'm testing CORS using http://www.test-cors.org on Quarkus 0.23.2 and also the latest master branch, with this config file:

quarkus.http.cors=true
quarkus.http.cors.origins=http://www.test-cors.org
quarkus.http.cors.headers=accept,authorization,content-type,x-requested-with,x-foobar
quarkus.http.cors.methods=GET,POST,PUT

It seems to work just fine.

@ricardozanini
Copy link

@Ladicek cool! I'll try to upgrade after I give my presentation demo and will report the results here.

@manufunk
Copy link
Author

Hmm I just tested it again with 0.25.0 and for me the cors headers are not showing op on the response headers :( Could somebody verify that?

@gsmet
Copy link
Member

gsmet commented Oct 16, 2019

@Manu206 and you use the new quarkus.http.cors prefix?

@manufunk
Copy link
Author

This is my config:

# Cors
quarkus.http.cors=true
quarkus.http.cors.origins=http://localhost
quarkus.http.cors.headers=accept, origin, authorization, content-type, x-requested-with
quarkus.http.cors.methods=GET,POST,OPTIONS

@Ladicek
Copy link
Contributor

Ladicek commented Oct 16, 2019

One thing I'd like to highlight: if the request doesn't have the Origin header, the response won't have CORS headers either.

This is perfectly fine IMHO. (What's worse is that this doesn't necessarily play well with caching proxy servers. My understanding is that we should at least always include Vary: Origin if CORS is enabled.)

@manufunk
Copy link
Author

I also include the Origin header. Still no change. There is still something wrong.

@gsmet
Copy link
Member

gsmet commented Oct 23, 2019

@Manu206 any chance you could provide a sample application so that we could see what's not working correctly?

@gsmet
Copy link
Member

gsmet commented Oct 23, 2019

We really need to get to the bottom of this.

@machi1990
Copy link
Member

We really need to get to the bottom of this.

@gsmet I can check it once we have a sample producer app.

@sberyozkin
Copy link
Member

Can Stuart's fix help here ?

@alexander-dammeier
Copy link

@sberyozkin No, i still have this problem with Quarkus 0.28.1

my application.properties (copied from your documentation and sligthy modified) :

quarkus.http.cors=true
quarkus.http.cors.origins=*
quarkus.http.cors.headers=X-Custom
quarkus.http.cors.methods=GET, POST, PUT, DELETE, OPTIONS
quarkus.http.cors.exposed-headers=Content-Disposition
quarkus.http.cors.access-control-max-age=24H

but my response headers are:

HTTP/1.1 200 OK
Content-Length: 2
Content-Type: Application/json

I will test this a little bit on my system tomorrow.

@Ladicek
Copy link
Contributor

Ladicek commented Nov 6, 2019

quarkus.http.cors.origins=* is wrong, if you want all origins, you should just leave the property out. Please make sure that you set the Origin header in the request when testing.

@alexander-dammeier
Copy link

@Ladicek Thank you, I corrected it. Now CORS works correctly for me with Quarkus 1.0.0.CR1 (Wow, third release this week, you are ultra fast).
Can you please add a comment to the origins property in your documentation, that the default value allows all origins, maybe to the default value section?

Thank you very much for your help.

@Ladicek
Copy link
Contributor

Ladicek commented Nov 7, 2019

@alexander-dammeier Well the documentation already says that, in the "Description" column.

@sberyozkin sberyozkin added this to the 1.0.0.CR1 milestone Nov 7, 2019
@sberyozkin
Copy link
Member

Let me close it then :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working
Projects
None yet
Development

No branches or pull requests

8 participants