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

StreamReader.readchunk returns false-negative indicator end_of_HTTP_chunk #3361

Closed
mnacharov opened this issue Oct 24, 2018 · 7 comments
Closed

Comments

@mnacharov
Copy link
Contributor

mnacharov commented Oct 24, 2018

Long story short

readchunk function returns False after full chunk received.
It causes unexpected delays and complicated chunk parser in chunk-dependent protocols (In our case: "Watch keys" in etcd HTTP API)

Expected behaviour

end_of_HTTP_chunk indicator equals True when full chunk received.

Actual behaviour

indicator equals False when tail of chunk ('\r\n') comes separately or with next chunk

Steps to reproduce

Simple web-server to reproduce the issue:

import socket
import time


def main():
    ls = socket.socket()
    ls.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
    ls.bind(('0.0.0.0', 1234))
    ls.listen(1)
    print('server is up')
    while True:
        s, addr = ls.accept()
        s.setsockopt(socket.SOL_TCP, socket.TCP_NODELAY, 1)
        # read request
        request = b''
        while True:
            data = s.recv(65536)
            if not data:
                print('err')
                return
            request += data
            if b'\r\n\r\n' in request:
                break
        print('read complete')
        # send headers
        s.send(b'\r\n'.join([
            b'HTTP/1.0 200 OK',
            b'transfer-encoding: chunked',
            b'connection: close',
            b'',
            b'',
        ]))
        # send first part of the chunk
        s.send(b'3\r\n***')
        time.sleep(0.1)
        # end of chunk
        s.send(b'\r\n')
        # time.sleep(0.1)   # uncomment to send end of chunk separately
        # next chunk
        s.send(b'5\r\n@@@@@\r\n')
        s.send(b'0\r\n\r\n')
        s.close()

main()

Client side:

import asyncio
import aiohttp


async def main():
    async with aiohttp.ClientSession() as session:
        async with session.get(url='http://localhost:1234/', timeout=None) as resp:
            buffer = b""
            async for raw_data, end_of_http_chunk in resp.content.iter_chunks():
                print(raw_data, end_of_http_chunk)
                buffer += raw_data
                if not end_of_http_chunk:
                    continue
                print('full chunk received', buffer)
                buffer = b""
            print('stop iteration', buffer)

loop = asyncio.get_event_loop()
loop.run_until_complete(main())

Expected result:

b'***' True
full chunk received b'***'
b'@@@@@' True
full chunk received b'@@@@@'
stop iteration b''

Actual:

b'***' False
b'@@@@@' True
full chunk received b'***@@@@@'
stop iteration b''

Environment

client side issue
aiohttp: 3.2.0, 3.4.4, master
OS: linux

@aio-libs-bot
Copy link

GitMate.io thinks the contributor most likely able to help you is @asvetlov.

Possibly related issues are #2112 (StreamReader.iter_chunks() raises IndexError), #313 (Can StreamReader replace FlowControlStreamReader ???), and #959 (StreamReader.read(n) to read n bytes or until eof).

@mnacharov
Copy link
Contributor Author

Working on solution with @socketpair

@socketpair
Copy link
Contributor

@mnach please show expected results and actual

@mnacharov
Copy link
Contributor Author

@mnach please show expected results and actual

thx, updated

@socketpair
Copy link
Contributor

socketpair commented Oct 24, 2018

@mnach Please make pull request, I know, you have a patch.

@mnacharov
Copy link
Contributor Author

@socketpair How did you know? )
I corrected a patch, so please take a look

@asvetlov
Copy link
Member

asvetlov commented Dec 5, 2018

Fixed by #3362

@lock
Copy link

lock bot commented Dec 5, 2019

This thread has been automatically locked since there has not been
any recent activity after it was closed. Please open a new issue for
related bugs.

If you feel like there's important points made in this discussion,
please include those exceprts into that new issue.

@lock lock bot added the outdated label Dec 5, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Dec 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants