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

Simple cookie use in ClientSession produces malformed HTTP request header in v1.2.0 #1566

Closed
nugget opened this issue Jan 30, 2017 · 14 comments
Labels

Comments

@nugget
Copy link

nugget commented Jan 30, 2017

Long story short

I have client code which uses simple cookies in http requests. It works fine on aiohttp 1.1.6 machines, but not on aiohttp 1.2.0 machines.

Expected behaviour

A ClientSession is invoked with simple cookies (not CookieJar) in order to pass cookies to a server as part of the request. Here is an actual, valid request on a 1.1.6 host:

GET / HTTP/1.1
Accept-Encoding: gzip, deflate
Accept: */*
Host: server.example.com
User-Agent: Python/3.5 aiohttp/1.1.6
Cookie: cows_are=cool
Content-Length: 0

Actual behaviour

The HTTP request header produced by aiohttp 1.2.0 is malformed, erroneously including the string "Set-Cookie:". This is from a 1.2.0 host using the same client script:

GET / HTTP/1.1
Accept-Encoding: gzip, deflate
Accept: */*
Host: server.example.com
User-Agent: Python/3.4 aiohttp/1.2.0
Cookie: cows_are="Set-Cookie: cows_are=cool"
Content-Length: 0

Steps to reproduce

I ran nc -l 80 on my test server so I could inspect the HTTP request being generated, then I ran this script on both versions of aiohttp:

#!/usr/bin/env python3

import aiohttp
import asyncio
import async_timeout

@asyncio.coroutine
def fetch(session, url):
    with aiohttp.Timeout(10):
        response = yield from session.get(url)
        return (yield from response.text())

@asyncio.coroutine
def main(loop):
    cookies = {'cows_are': 'cool'}
    with aiohttp.ClientSession(loop=loop, cookies=cookies) as session:
        html = yield from fetch(session, 'http://server.example.com/')
        print(html)

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

Your environment

Working host:

Non-working host:

I don't believe that the Python version difference is a factor, but I have not tested for that.

@fafhrd91
Copy link
Member

fafhrd91 commented Feb 1, 2017

i can not reproduce this problem with python 3.5.2

@fafhrd91
Copy link
Member

fafhrd91 commented Feb 1, 2017

@nugget could you test with python3.4.3 + aiohttp 1.2.0 and python3.5.2 + 1.2.0

@pvizeli
Copy link

pvizeli commented Feb 1, 2017

I have the same issue on my raspberry. But I can't update to python3.4.3 since pyton3.4.2 ist the default. But I don't see the issue on my windows developer machine with python 3.5.2. I hope that will help to find the issue.

@fafhrd91
Copy link
Member

fafhrd91 commented Feb 1, 2017

ok, i will back port cookies module. but it will be removed with python3.4 support around fall.

@fafhrd91
Copy link
Member

fafhrd91 commented Feb 1, 2017

@pvizeli could you test master?

@fafhrd91
Copy link
Member

fafhrd91 commented Feb 1, 2017

this issue should be fixed in master

@fafhrd91 fafhrd91 closed this as completed Feb 1, 2017
@pvizeli
Copy link

pvizeli commented Feb 2, 2017

Looks good @fafhrd91 thanks a lot 👍 We need support for 3.4.2 while a lot of people run home-assistant on raspberry pi.

@PuckStar
Copy link

PuckStar commented Feb 5, 2017

@fafhrd91 yes please also for 3.4.2 !

@fafhrd91
Copy link
Member

fafhrd91 commented Feb 5, 2017

@pvizeli @PuckStar what's your timeframe? When will you be able to upgrade python? I am thinking about dropping python3.4 around Jan 2018

@PuckStar
Copy link

PuckStar commented Feb 7, 2017

@fafhrd91 I'm sorry I have no idea. I'm not a developer for Home Assistant.

@fafhrd91
Copy link
Member

fafhrd91 commented Feb 7, 2017

That's fine. In any case aiohttp will support python3.4.2 until 01/2018

@pvizeli
Copy link

pvizeli commented Feb 8, 2017

@fafhrd91 sorry for late message. That should be okay. You make amazing work 👍

@balloob
Copy link
Contributor

balloob commented Feb 9, 2017

01/2018 is great! The Home Assistant min Python version is whatever ships with Debian. It currently is 3.4.2 but the next release of Debian called Stretch is around the corner and ships Python 3.5.3.

@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.
Labels
Projects
None yet
Development

No branches or pull requests

5 participants