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

No CORS headers are set on the client javascript file #3552

Closed
1 of 2 tasks
aeons opened this issue Feb 14, 2020 · 12 comments
Closed
1 of 2 tasks

No CORS headers are set on the client javascript file #3552

aeons opened this issue Feb 14, 2020 · 12 comments
Labels
enhancement New feature or request good first issue Good issue for new contributors to get started with
Milestone

Comments

@aeons
Copy link

aeons commented Feb 14, 2020

You want to:

  • report a bug
  • request a feature

Current behaviour

When serving the client via settings the serveClient option, the client is not served with the same CORS headers as the actual socket.io server.

Steps to reproduce (if the current behaviour is a bug)

Start a server with serveClient: true and request /socket.io/socket.io.js with an Origin header set.

Observe that the response does not have any Access-Control-Allow-Origin header set.

Expected behaviour

That the response has the same CORS headers as the normal operation.

Other information (e.g. stacktraces, related issues, suggestions how to fix)

This function (and the corresponding serveMap) should do the same origin checking as this code.

@eltharynd
Copy link

eltharynd commented Feb 20, 2020

Is it me or CORS don't have to be "set" on client side... you have to tell the server which cors will be accepted.

Here's my implementation to allow all in typescript

this.app.use(cors({
    origin: '*',
    optionsSuccessStatus: 200,
}))

the browser is responsible for indicating the origin domain and you have no control over that for security reasons...

I might be mistaken..

Also, even if, this should probably go on the socket.io-client github :)

@aeons
Copy link
Author

aeons commented Feb 20, 2020

The Access-Control-Allow-Origin header is a response header from the server, indicating which origins are allowed to use the resource. Browsers respect this header.

If you set the serveClient option in socket.io, it serves the javascript of the client from the socket.io server. However, the function(s) responsible for doing this, does not set the Access-Control-Allow-Origin header, which means that if you try to request the client javascript in a cross-origin environment, the request fails.

The correct headers are applied to the actual socket.io websocket connection, so I'm suggesting that the same logic is applied to the served client sources.

@Apollon77
Copy link

I also stumble over the same issue and yes CORS needs to be set by the server also from my knowledge ;-) Can you do a PR? ;-)

@Keepcase
Copy link

I was able to fix my CORS issues using the handlePreflightRequest referenced in socketio/engine.io#279 (comment) but I couldn't use the latest changes implemented for engine.io v4 in socket.io (2.3.0).

@Apollon77
Copy link

@Keepcase sorry but I do not understand it ... were yiu able to fix it or not? ;-) Or do you mean that you know how/where to fix but in engine.io and noch in socket.io?! Or what?
What would be needed to be patched?

@Keepcase
Copy link

Keepcase commented Jun 22, 2020

My apologies @Apollon77, let me clarify. Perhaps I'm misunderstanding the issue but I was suggesting to try the change that I referenced on the socket.io server to see if that fixed the issue @aeons was running into.

I recently ran into issues with CORS and the referenced comment fixed it for my use case. I was getting errors on the browser similar to socketio/engine.io#574 but I fixed it with the code referenced in socketio/engine.io#574 (comment) which I implemented on the socket.io server.

I was just mentioning that the latest changes made in socketio/engine.io#279 (comment) referencing cors was not working for socket.io (2.3.0).

@darrachequesne darrachequesne added enhancement New feature or request good first issue Good issue for new contributors to get started with labels Nov 25, 2021
@bilalsha
Copy link

Can I work on it @darrachequesne

@darrachequesne
Copy link
Member

@bilalsha sure!

@pushkarsingh019
Copy link

Has the issue been resolved, if not, I would like to work on it.

@Apollon77
Copy link

I assume it is still there :-(

@hritik-dubey
Copy link

@darrachequesne issue is still there ?

darrachequesne added a commit that referenced this issue Jun 20, 2023
The version of the `cors` package matches the one used by `engine.io`.

Related: #3552
@darrachequesne darrachequesne added this to the 4.7.0 milestone Jun 20, 2023
@darrachequesne
Copy link
Member

This should be fixed by 63f181c, included in version 4.7.0.

Please reopen if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good issue for new contributors to get started with
Projects
None yet
Development

No branches or pull requests

8 participants