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

Add CORS header configuration for BRP RemoteHttpPlugin #15551

Open
villor opened this issue Sep 30, 2024 · 4 comments
Open

Add CORS header configuration for BRP RemoteHttpPlugin #15551

villor opened this issue Sep 30, 2024 · 4 comments
Labels
A-Dev-Tools Tools used to debug Bevy applications. C-Feature A new feature, making something new possible D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it!

Comments

@villor
Copy link
Contributor

villor commented Sep 30, 2024

What problem does this solve or what need does it fill?

To fully utilize the BRP functionality using web hosted tools, we need a way to set the Access-Control-Allow-Origin header in the responses.

Without being able to set that header, we would have to resort to reverse proxies to get around the issue.

What solution would you like?

Add a with_cors method to RemoteHttpPlugin. The method would ensure that Access-Control-Allow-Origin is added to all responses, with the origin provided as a param.

What alternative(s) have you considered?

We could add the option to set any headers during setup. But I don't know if that is really needed.

Additional context

See bevy_react_inspector which currently needs a reverse proxy to function.

@villor villor added C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled labels Sep 30, 2024
@pablo-lua pablo-lua added A-Dev-Tools Tools used to debug Bevy applications. D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it! and removed S-Needs-Triage This issue needs to be labelled labels Sep 30, 2024
@JMS55
Copy link
Contributor

JMS55 commented Oct 1, 2024

I'd rather a custom header in general vs specifying cors specifically.

@spacemen0
Copy link

I think with_cors probably make sense? Enabling cors might be the most common in terms of user cases.

@villor
Copy link
Contributor Author

villor commented Oct 1, 2024

Ideally, we would have both configurable headers and a with_cors for conveniency IMO.

I also believe cors will be the most common use case by far. So an initial solution could be to implement just that method.

@spacemen0
Copy link

I'm new to bevy but I think I can try working on this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Dev-Tools Tools used to debug Bevy applications. C-Feature A new feature, making something new possible D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it!
Projects
None yet
Development

No branches or pull requests

4 participants