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.

Sign up for GitHub

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

Jump to bottom

Handle unknown 1XX status codes better in client mode #1353

Closed
Lukasa opened this issue Oct 31, 2016 · 6 comments
Closed

Handle unknown 1XX status codes better in client mode #1353

Lukasa opened this issue Oct 31, 2016 · 6 comments
Labels
bug client outdated

Comments

@Lukasa
Copy link

Lukasa commented Oct 31, 2016

Long story short

aiohttp's HTTP client does not tolerate non-100 or 101 status codes from the 1XX block in a sensible manner: it treats them as final responses, rather than as provisional ones.

Expected behaviour

When aiohttp receives a 1XX status code that it does not recognise, it should either surface these to a user that expects them by means of a callback, or it should ignore them and only show the user the eventual final status code. This is required by RFC 7231 Section 6.2 (Informational 1xx), which reads:

A client MUST be able to parse one or more 1xx responses received prior to a final response, even if the client does not expect one. A user agent MAY ignore unexpected 1xx responses.

Actual behaviour

aiohttp treats the 1XX status code as final. It parses the header block as though it were a response in the 2XX or higher range. In most cases these 1XX response codes will not contain a content length or transfer-encoding directive, which means that aiohttp will fall back to reading until connection close. This will cause any following final response to be interpreted as part of the body of the 1XX response, rather than as its own unique response.

Steps to reproduce

The following "server" can demonstrate the issue:

import socket
import time

document = b'''<!DOCTYPE html>
<html lang="en">
  <head>
    <meta charset="utf-8">
    <title>title</title>
    <link rel="stylesheet" href="/other/styles.css">
    <script src="/other/action.js"></script>
  </head>
  <body>
    <h1>Hello, world!</h1>
  </body>
</html>
'''

s = socket.socket()
s.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
s.bind(('localhost', 8080))
s.listen(5)

while True:
    new_socket, _ = s.accept()
    data = b''

    while not data.endswith(b'\r\n\r\n'):
        data += new_socket.recv(8192)

    new_socket.sendall(
        b'HTTP/1.1 103 Early Hints\r\n'
        b'Server: socketserver/1.0.0\r\n'
        b'Link: </other/styles.css>; rel=preload; as=style\r\n'
        b'Link: </other/action.js>; rel=preload; as=script\r\n'
        b'\r\n'
    )
    time.sleep(1)
    new_socket.sendall(
        b'HTTP/1.1 200 OK\r\n'
        b'Server: socketserver/1.0.0\r\n'
        b'Content-Type: text/html\r\n'
        b'Content-Length: %s\r\n'
        b'Link: </other/styles.css>; rel=preload; as=style\r\n'
        b'Link: </other/action.js>; rel=preload; as=script\r\n'
        b'Connection: close\r\n'
        b'\r\n' % len(document)
    )
    new_socket.sendall(document)
    new_socket.close()

If this server is run directly, the following client can be used to test it:

import aiohttp
import asyncio
import async_timeout


async def fetch(session, url):
    with async_timeout.timeout(10):
        async with session.get(url) as response:
            print("Status: {}".format(response.status))
            print("---")
            return await response.text()

async def main(loop):
    async with aiohttp.ClientSession(loop=loop) as session:
        html = await fetch(session, 'http://localhost:8080/')
        print(html)

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

This client will print

Status: 103

---
HTTP/1.1 200 OK
Server: socketserver/1.0.0
Content-Type: text/html
Content-Length: 256
Link: </other/styles.css>; rel=preload; as=style
Link: </other/action.js>; rel=preload; as=script
Connection: close

<!DOCTYPE html>
<html lang="en">
  <head>
    <meta charset="utf-8">
    <title>title</title>
    <link rel="stylesheet" href="/other/styles.css">
    <script src="/other/action.js"></script>
  </head>
  <body>
    <h1>Hello, world!</h1>
  </body>
</html>

This response is clearly wrong: the 200 header block is being treated as part of the 1XX response, rather than as a unique entity. Note that if Content-Length: 0 is added to the 1XX response, this will still parse incorrectly, but now it will be treated as having zero length, and so will ruin the framing of the HTTP request/responses.

Your environment

aiohttp version 1.0.5
Python 3.5.2
macOS 10.12.1
@asvetlov
Copy link
Member

asvetlov commented Nov 1, 2016

Yes, it's a bug. Thanks for report.
@Lukasa would you make a patch?

@Lukasa
Copy link
Author

Lukasa commented Nov 1, 2016

I certainly can, though I'm not sure how quickly I can get to it.

@fafhrd91
Copy link
Member

i do not think it is a bug. just limitations of current implementation.

simplest solution is to accept coroutine as expect100 parameter instead of boolean
then ClientResponse can call it with any 1XX message

@Lukasa
Copy link
Author

Lukasa commented Dec 27, 2016

@fafhrd91 It would not be a bug if aiohttp simply ignored the 1XX: that would be fine. Treating it as a final response, however, is a bug.

@fafhrd91
Copy link
Member

fixed in master

@hubo1016 hubo1016 mentioned this issue Apr 13, 2018
Closed
@lock
Copy link

lock bot commented Oct 29, 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 Oct 29, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
bug client outdated
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@asvetlov @fafhrd91 @Lukasa