WebSocket header HTTP_SEC_WEBSOCKET_PROTOCOL value changed to 'streamlit, PLACEHOLDER_AUTH_TOKEN' which gives connection error

pymarc commented 5 months ago

Checklist

[X] I have searched the existing issues for similar issues.
[X] I added a very descriptive title to this issue.
[X] I have provided sufficient information below to help reproduce this issue.

Summary

After this change https://github.com/streamlit/streamlit/commit/fd1037ba61 in the file https://github.com/streamlit/streamlit/blame/1.29.0/frontend/app/src/connection/WebsocketConnection.tsx#L435 the request header Sec-Websocket-Protocol from the webpage client has no longer the value 'streamlit' but from Streamlit version 1.29.0 the value is 'streamlit, PLACEHOLDER_AUTH_TOKEN'.

I use a Streamlit app where the server side of the app is in a docker and the communication between the webpage client and the server goes through a proxy based on Flask and uwsgi.

That changed header prevented the webpage client from connecting to the server ( URL /_stcore/stream failed).

My fix was to replace the value 'streamlit, PLACEHOLDER_AUTH_TOKEN' with 'streamlit' in my proxy layer before establishing the websocket handshake, so I did a patch for that.

Maybe it's a good idea for you guys that you rethink the solution? Maybe it can be more robust?

Reproducible Code Example

No response

Steps To Reproduce

No response

Expected Behavior

No response

Current Behavior

No response

Is this a regression?

[X] Yes, this used to work in a previous version.

Debug info

Streamlit version: 1.29.0 and higher
Python version: 3.9
Operating System: Ubuntu 20.04
Browser: Microsoft Edge for Business (Version 125.0.2535.51)

Additional Information

No response

github-actions[bot] commented 5 months ago

If this issue affects you, please react with a 👍 (thumbs up emoji) to the initial post.

Your feedback helps us prioritize which bugs to investigate and address first.

kmcgrady commented 5 months ago

Hey @pymarc ! Thanks for filing the issue. I will have to follow up with @vdonato who is more knowledgable of the code. According the comment, an empty string is an invalid value to provide. See https://github.com/streamlit/streamlit/pull/7608 for some context.

I do know we are doing some more work with regards to auth and headers, which might be of interest in providing a more robust solution. Tagging @sfc-gh-jcarroll and @kajarenc who's been thinking in this space.

kmcgrady commented 5 months ago

Following up. I've confirmed with @vdonato that this is expected behavior, and so I am going to close the ticket. The protocol should allow for a comma separated list, so while semantically it may not make sense, the contents should comply with the protocol.

pymarc commented 5 months ago

OK, I accept your answer. However, please be informed that uwsgi does not like the header value and refuses to establish a connection because of it.

streamlit / streamlit