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

aiohttp eroneous, incorrect, Client error "invalid constant string" #3641

Closed
nhumrich opened this issue Mar 7, 2019 · 7 comments
Closed
Labels
bug needs-info Issue is lacking sufficient information and will be closed if not provided Stale

Comments

@nhumrich
Copy link

nhumrich commented Mar 7, 2019

Long story short

aiohttp is returning 400, message='invalid constant string' when nothing is actually wrong with the request or response.

Expected behaviour

I get the response object, which should be a 204

Actual behaviour

when I send a request, and get a 204 back, aiohttp throws this stack trace:

Traceback (most recent call last):
  File "/home/nhumrich/.virtualenvs/aviary/lib/python3.7/site-packages/aiohttp/client_reqrep.py", line 757, in start
    message, payload = await self._protocol.read()
  File "/home/nhumrich/.virtualenvs/aviary/lib/python3.7/site-packages/aiohttp/streams.py", line 543, in read
    await self._waiter
  File "/home/nhumrich/.virtualenvs/aviary/lib/python3.7/site-packages/aiohttp/client_proto.py", line 195, in data_received
    messages, upgraded, tail = self._parser.feed_data(data)
  File "aiohttp/_http_parser.pyx", line 523, in aiohttp._http_parser.HttpParser.feed_data
aiohttp.http_exceptions.BadHttpMessage: 400, message='invalid constant string'

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/nhumrich/cnpy/aviary/aviary/service.py", line 53, in wrapped
    resp = await _function(session)
  File "/home/nhumrich/cnpy/aviary/aviary/canaries/billing.py", line 131, in create_invoice
    resp3 = await session.delete(f'/api/invoices/{new_invoice_id}')
  File "/home/nhumrich/cnpy/aviary/aviary/session.py", line 32, in delete
    return await self.send_request('DELETE', path, headers=headers, vars=vars)
  File "/home/nhumrich/cnpy/aviary/aviary/session.py", line 45, in send_request
    async with self._session.request(method, url, json=body, headers=headers) as resp:
  File "/home/nhumrich/.virtualenvs/aviary/lib/python3.7/site-packages/aiohttp/client.py", line 855, in __aenter__
    self._resp = await self._coro
  File "/home/nhumrich/.virtualenvs/aviary/lib/python3.7/site-packages/aiohttp/client.py", line 391, in _request
    await resp.start(conn)
  File "/home/nhumrich/.virtualenvs/aviary/lib/python3.7/site-packages/aiohttp/client_reqrep.py", line 762, in start
    message=exc.message, headers=exc.headers) from exc
aiohttp.client_exceptions.ClientResponseError: 400, message='invalid constant string'

I have tried to debug this as much as possible, but got stuck, and this is as far as I got. I noticed that in
client_proto.py line 195 (as seen in the stack trace) that data is set to

b'HTTP/1.1 204 No Content\r\nDate: Thu, 07 Mar 2019 00:30:24 GMT\r\nContent-Length: 0\r\nConnection: keep-alive\r\nX-Via: heimdall\r\nServer: canopy-invoice\r\nAccess-Control-Allow-Origin: https://sub.example.domain.com\r\nAccess-Control-Allow-Credentials: true\r\nx-heimdall-target: rabbit\r\nX-Via: canopy-proxy-n\r\nX-Env: integ\r\nStrict-Transport-Security: max-age=31536000; includeSubDomains\r\n\r\nnull'

As you can see, there is a weird null at the end, even though content-length is 0. Wondering if this was a weird server bug, I tried to see what requests did and if I use requests-toolbelt to dump the raw response data, doing the exact same thing, the response has no null at the end:

bytearray(> HTTP/1.1 204 No Content\r\n> Date: Thu, 07 Mar 2019 00:00:27 GMT\r\n> Content-Length: 0\r\n> Connection: keep-alive\r\n> X-Via: heimdall\r\n> X-Via: canopy-proxy-n\r\n> Server: canopy-invoice\r\n> Access-Control-Allow-Origin: https://sub.example.com\r\n> Access-Control-Allow-Credentials: true\r\n> x-heimdall-target: rabbit\r\n> X-Env: integ\r\n> Strict-Transport-Security: max-age=31536000; includeSubDomains\r\n> \r\n')

If I call the same endpoint in the browser, there is no body shown.
Unfortunately, this is a private endpoint that requires authentication, so I cant send a url to reproduce.
But using that string about for data, you can reproduce the error. I have attempted to replace the data, and remove null, and if I remove the null at the end manually, (by editing aiohttp code locall, or using the debugger), everything works perfectly. The issue is that null is getting at the end somehow, and I am not sure how. I tried to put the debugger up higher in the stack, near read() in client_reqrep.py, but ended up getting very different errors, that seam to be race conditions with the headers when I attach a debugger.

Anyways, any thoughts as how the null is getting there?

This is actually a two-fold bug. First, the error makes it look like its a client error by sending a 400. I spent a significant amount of time thinking that the error was coming from the server because of the 400 status code. It would be great if this error made it more clear that the "server response" was invalid, and not my request.

But second, why is the null there, when no other client has it? Can this issue be solved?

Your environment

aiohttp==3.4.4
python==3.7.2
os==linux 4.20.5
using the client only.

any followup questions?

@aio-libs-bot
Copy link

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

Possibly related issues are #3242 (Error), #1753 (aiohttp.errors is gone), #3251 (Aiohttp Client request limit), #2624 (aiohttp client throws http errors for the following redirect), and #2635 (Unclosed client session Error in aiohttp.request).

@dimaqq
Copy link
Contributor

dimaqq commented Mar 7, 2019

@nhumrich can you explain what produces this response?
Is it some server?
Is it hard-coded?
Is there perhaps a tcpdump / wireshark trace?

You assumption on the importance of the source of null is correct. Anyone who's going to work on it must know whence it comes.

For example, curl gives out a warning when there's extra data after Content-Length: 0 and http pipelining is off. curl however accepts the response and doesn't raise the exception.

requests swallows the unexpected body without exception. Perhaps it's a quirk introduced specifically to work around bad servers.

Chrome also swallows the unexpected body without error.

Thus, if heimdall proxy or the origin server actually sent the 4 bytes null, it could easily go unnoticed, as common tools handle such badness.

@webknjaz
Copy link
Member

webknjaz commented Mar 7, 2019

@dimaqq I believe it's produced by http-parser on the client side.

@dimaqq
Copy link
Contributor

dimaqq commented Mar 7, 2019

http-parser doesn't compile for me against Python-3.7.2, neither on linux nor mac.

@webknjaz
Copy link
Member

webknjaz commented Mar 7, 2019

You probably don't have build deps then. Also, we ship OS-specific wheels so you don't have to compile anything during install time.

@nhumrich
Copy link
Author

nhumrich commented Mar 7, 2019

For example, curl gives out a warning when there's extra data after Content-Length: 0

ok, good call, here is the result of curl:

  • Excess found in a non pipelined read: excess = 4 url = /api/invoices/149828 (zero-length body)

So, you might be right, the server probably is sending something. That helps. Thanks!

I would still like to keep this issue open, to change the error, and how it is presented to the user. Because 400 invalid constant string in no way represents the real error, which is a parsing error.

Also, perhaps, aiohttp could also act like most clients, and simply not read the body when the content-length is 0.

damb pushed a commit to EIDA/eidaws that referenced this issue May 7, 2021
- Prevent us from storing errorneous status code in response code stats

References: aio-libs/aiohttp#3641
damb pushed a commit to EIDA/eidaws that referenced this issue May 7, 2021
- Prevent us from storing errorneous status code in response code stats

References: aio-libs/aiohttp#3641
damb pushed a commit to EIDA/eidaws that referenced this issue May 7, 2021
- Prevent us from storing errorneous status code in response code stats

References: aio-libs/aiohttp#3641
@Dreamsorcerer
Copy link
Member

Parsing etc. has changed substantially since this issue was opened. Can you retest if the problem still exists?

@Dreamsorcerer Dreamsorcerer added the needs-info Issue is lacking sufficient information and will be closed if not provided label Aug 11, 2024
@github-actions github-actions bot added the Stale label Sep 11, 2024
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Sep 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug needs-info Issue is lacking sufficient information and will be closed if not provided Stale
Projects
None yet
Development

No branches or pull requests

5 participants