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 default headers to WebSocket
s implementations
#1606
Conversation
adopted change suggested. Co-authored-by: Marcelo Trylesinski <marcelotryle@gmail.com>
as per suggestion, moved the limitation to --no-server-header area
Added logic to add server header support for websockets and also to pass default headers
Added logic to add server header support for websockets and also to pass default headers
* added logic to accept default headers in websocket * added default_headers to class init * fix * feat(websockets): added server header support for websockets Added logic to add server header support for websockets and also to pass default headers * feat(websockets-wsproto): added server header support for websockets Added logic to add server header support for websockets and also to pass default headers Co-authored-by: Irfanuddin <irfanuddin@knowledgelens.com>
# Conflicts: # uvicorn/protocols/websockets/websockets_impl.py # uvicorn/protocols/websockets/wsproto_impl.py
…sockets" This reverts commit 62c6c7e
--no-date-header is also not compatible with Websockets
--no-date-header is also not compatible with Websockets
Suggestion removes duplication check on header names to keep it consistent with http implementations. Co-authored-by: Marcelo Trylesinski <marcelotryle@gmail.com>
Co-authored-by: Marcelo Trylesinski <marcelotryle@gmail.com> Co-authored-by: Irfanuddin <irfanuddinshafi@gmail.com>
Merged suggestions given by Marcelo in #1736 |
@pytest.mark.anyio | ||
@pytest.mark.parametrize("ws_protocol_cls", ONLY_WS_PROTOCOL) | ||
@pytest.mark.parametrize("http_protocol_cls", HTTP_PROTOCOLS) | ||
async def test_no_date_header(ws_protocol_cls, http_protocol_cls): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not 100% true. I forgot to mention... The Date
headers from the websockets
cannot be removed... 🤔
Should we do something about it on the tests? It's on purpose... 🤔
It's a question, I'm still thinking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do what on tests? There is no way to remove date header in websockets
I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean, I know, but the tests make us believe that we forgot about Date
headers... 🤔
Co-authored-by: Marcelo Trylesinski <marcelotryle@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was a long way, but we got it. 🙏
Just a remark for myself: the Date
headers remains on the websockets
implementation.
WebSocket
s implementations
Co-authored-by: Marcelo Trylesinski <marcelotryle@gmail.com> Co-authored-by: Irfanuddin <irfanuddin@knowledgelens.com> Co-authored-by: Irfanuddin <irfanuddinshafi@gmail.com>
Co-authored-by: Marcelo Trylesinski <marcelotryle@gmail.com> Co-authored-by: Irfanuddin <irfanuddin@knowledgelens.com> Co-authored-by: Irfanuddin <irfanuddinshafi@gmail.com>
Refer #1581 for detailed discussion.
Feature Description:
This feature fixes the issue that discussed in #1574
Currently it implements support for
no-server-header
forwsproto
implementations.