httpie / cli

🥧 HTTPie CLI — modern, user-friendly command-line HTTP client for the API era. JSON support, colors, sessions, downloads, plugins & more.
https://httpie.io
BSD 3-Clause "New" or "Revised" License
33.56k stars 3.67k forks source link

Checking headers to determine auto-streaming #1383

Closed egleston closed 2 years ago

egleston commented 2 years ago

The header-check for auto-streaming should parse out the type/sub-type from optional parameters. FastAPI/Starlette modifies the Content-Type header for SSE responses (actually, for all text/* media) by adding the charset as an "optional parameter": Content-Type: text/event-stream; charset=utf-8

codecov-commenter commented 2 years ago

Codecov Report

Merging #1383 (fbcff30) into master (4d7d6b6) will decrease coverage by 2.69%. The diff coverage is 91.32%.

@@            Coverage Diff             @@
##           master    #1383      +/-   ##
==========================================
- Coverage   97.28%   94.59%   -2.70%     
==========================================
  Files          67      109      +42     
  Lines        4235     7655    +3420     
==========================================
+ Hits         4120     7241    +3121     
- Misses        115      414     +299     
Impacted Files Coverage Δ
httpie/output/ui/man_pages.py 0.00% <0.00%> (ø)
httpie/output/ui/rich_utils.py 0.00% <0.00%> (ø)
tests/test_binary.py 100.00% <ø> (ø)
tests/test_ssl.py 92.59% <ø> (-2.35%) :arrow_down:
tests/test_stream.py 100.00% <ø> (ø)
tests/test_tokens.py 100.00% <ø> (ø)
tests/test_update_warnings.py 99.14% <ø> (ø)
tests/test_uploads.py 96.70% <ø> (-3.30%) :arrow_down:
tests/test_xml.py 97.56% <ø> (-0.06%) :arrow_down:
tests/utils/__init__.py 92.37% <ø> (+3.69%) :arrow_up:
... and 101 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 5e76ebc...fbcff30. Read the comment docs.

isidentical commented 2 years ago

Overall, it looks amazing. Thanks @egleston for the PR (and finding the bug). Beside those 2 minor comments, I wouldn't touch to the HTTP server code but just rather change the test case (which should be easily adaptable):

diff --git a/tests/test_stream.py b/tests/test_stream.py
index a8950d2..8df2af2 100644
--- a/tests/test_stream.py
+++ b/tests/test_stream.py
@@ -124,6 +124,10 @@ def test_redirected_stream(httpbin):
         ['Accept:text/event-stream'],
         3
     ),
+    (
+        ['Accept:text/event-stream; charset=utf-8'],
+        3
+    ),
     (
         ['Accept:text/plain'],
         1
@@ -132,7 +136,7 @@ def test_redirected_stream(httpbin):
 def test_auto_streaming(http_server, extras, expected):
     env = MockEnvironment()
     env.stdout.write = Mock()
     http(http_server + '/drip', *extras, env=env)
     assert len([
         call_arg
         for call_arg in env.stdout.write.call_args_list

and add a changelog entry!