-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
file_uploader headers on https deployed streamlit app #10049
Comments
Hello @Test-028 , thank you for opening this issue! I don't fully understand the problem exact problem you facing.
Or as an alternative, I think you should be able to disable CORS protection on the Streamlit level and delegate that check to the proxy Please let us know if you are able to fix the issue on the infrastructure level or if you have any other suggestions/comments related to this issue. |
Hello @kajarenc , thank you for your reply ! My exact problem , is that , with the setup that streamlit suggest for deploying HTTPS application, the file_uploader handler add this If you look at other endpoints present on streamlit , the logic when we enable Cors protection is to simply reject CORS request, and no Why not simply getting rid of the possibility of doing cross origin request on it , like it is done for other endpoints in streamlit when enableCors is on: that is no cross origin request -> No CORS headers. Currently if my understanding is correct, in a regular setup you will have Acces-Control-Allow-Origin that has the same value as your origin (so not really needed I guess) , and with people with the same situation as me , a weird |
Yeah, I see; thank you for the explanation We thinking of improving this experience, especially decoupling CORS and CSRF stremlit configs, and this is also could be point of improvement! just to check, when we talk about upload endpoint , we mean yeah, for now the workaround with configuring a rule on proxy layer to remove undesired headers should work |
@kajarenc Exactly I'm speaking about this endpoint. I think it received a different treatment for no obvious reason, its set-header method should have been like the other ones. And certainly a lot of users are impacted without noticing it , since at the end they are doing the request from the same origin, but they have this weird Access-Control-Allow-origin set to localhost for example, which can be a potential important security issue I guess. When you look at everything, you think that by having |
Checklist
Summary
When using
file_uploader
component on a streamlit application that has been deployed behind a load balancer that performs https termination everything works fine.Let's suppose that my application is accessible through
https://myapp.com
enableCors
andenableXsrfProtection
are both on.Then due to this block in the
upload_file_request_handler.py
file:If I let my configuration unchanged everything works fine , but we will have
Access-Control-Allow-Origin
with localhost as value which is something I don't want to have.If I put
myapp.com
asbrowser.serverAddress
, I'll get http://myapp.com (and not https://myapp.com) in theAccess-Control-Allow-Origin
headerand if i enter
https://myapp.com
asbrowser.serverAddress
it will behttp://https://myapp.com
in theAccess-Control-Allow-Origin
header.If we look at
server_util.get_url
we can understand this behavior , as I said in the beginning I perform https termination before the application and therefore I don't set anyserver.sslCertFile
that will make theget_url
adding https as protocol instead of http.Finally I don't even understand why we set this Cors header in the file_uploader, in any of the scenario everything works fine (except that we have weird
Access-Control-Allow-Origin
: the whole point of my issue) , because I guess when using the file_uploader, requests are made from the same origin and aren't cross origin requests.Reproducible Code Example
No response
Steps To Reproduce
No response
Expected Behavior
No response
Current Behavior
No response
Is this a regression?
Debug info
Additional Information
No response
The text was updated successfully, but these errors were encountered: