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

ASGI Streaming response contains extra content #1730

Closed
dmachi opened this issue Dec 13, 2019 · 6 comments · Fixed by #1876 or #1957
Closed

ASGI Streaming response contains extra content #1730

dmachi opened this issue Dec 13, 2019 · 6 comments · Fixed by #1876 or #1957
Labels

Comments

@dmachi
Copy link

dmachi commented Dec 13, 2019

I have some code that makes an http request (aiohttp) to an separate service to retrieve a file and then stream that file back. It is essentially acting as a proxy for this request. However, the streaming portion ends up wrapping some additional bytes of data around the body that I don't quite understand.

Here is a test case:

from sanic import Sanic
from sanic.response import file_stream
from pathlib import Path
import aiofiles
from aiofiles import os as async_os
from sanic.response import stream
import aiohttp

app = Sanic()

@app.route("/", methods=["GET"])
async def test(request):
  tf=__file__
  file_stat = await async_os.stat(tf)
  headers = {"Content-Length": file_stat.st_size,"Content-Type": "application/octet-stream"}
  return await file_stream(tf,headers=headers,chunked=False)

@app.route("/test2",methods=["GET"])
async def test2(request):
  session = aiohttp.ClientSession()
  resp = await session.get('http://localhost:%s/'%request.server_port)

  async def stream_file(response):
    while True:
      chunk = await resp.content.read()
      if not chunk:
        break
      await response.write(chunk)

  return stream(stream_file)

I run this code with 'uvicorn test:app --port 9000'

There are two endpoints / and /test2:

  • The / request simply returns a file (the actual code in this example). curl "http://localhost:9000/" verifies the results are correct
  • The /test2 call makes an aiohttp call to the / endpoint and then tries to stream the response. I expect the response to be the same as the / endpoint.

The actual response looks like:

$ curl http://localhost:9000/test2
33d
from sanic import Sanic
from sanic.response import file_stream
from pathlib import Path
...TRUNCATED MIDDLE OF OUTPUT
  return stream(stream_file)
0

That '33d' and '0' should not be there, but I don't see where they are coming from. Am I passing the stream along improperly?

The environment is OSX 10.15.1

One note is that I have applied the small snippet of code I described in #1653 in order to make the initial file stream work.

@dmachi
Copy link
Author

dmachi commented Dec 13, 2019

I figured out that the extra content is the number of bytes in the chunk.
It was coming from

        if self.chunked:
            await self.protocol.push_data(b"%x\r\n%b\r\n" % (len(data), data))
        else:
            await self.protocol.push_data(data)

I turned chunking off (return stream(stream_file,chunked=False) and it works. Its not clear to me if this actually a bug or I'm doing something wrong.

@stale
Copy link

stale bot commented Mar 12, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If this is incorrect, please respond with an update. Thank you for your contributions.

@stale stale bot added the stale label Mar 12, 2020
@Tronic
Copy link
Member

Tronic commented Mar 18, 2020

I cannot even run the testcase code on current Sanic master because it tries to send headers as data. This is because asgi.py calls StreamingHTTPResponse.stream, which does self.protocol.push_data(headers), and that cannot be right for ASGI. I suppose this would need fixing prior to 20.3 release.

However, this is one of the parts rewritten in #1791 (the streaming branch) and indeed the testcase is working correctly there, in both modes (chunked or not).

@stale stale bot removed the stale label Mar 18, 2020
@Tronic Tronic mentioned this issue Mar 18, 2020
@stale
Copy link

stale bot commented Jun 16, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If this is incorrect, please respond with an update. Thank you for your contributions.

@stale stale bot added the stale label Jun 16, 2020
@Tronic
Copy link
Member

Tronic commented Jun 17, 2020

Still broken in master!

@stale stale bot removed the stale label Jun 17, 2020
@stale
Copy link

stale bot commented Sep 17, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If this is incorrect, please respond with an update. Thank you for your contributions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants