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

Support for Set-Cookie response header from polling request #162

Closed
jackdreinhardt opened this issue Feb 7, 2020 · 10 comments
Closed

Support for Set-Cookie response header from polling request #162

jackdreinhardt opened this issue Feb 7, 2020 · 10 comments
Assignees
Labels

Comments

@jackdreinhardt
Copy link

Hi,

I am working on a project in which I need to set up a websocket connection to an express.js app running passport.js for authentication. I am running into an issue because the response headers from the initial polling request (e.g. the response from http://localhost:8080/socket.io/?EIO=3&transport=polling&t=N0TyWKS) contain a set-cookie header, namely io, that needs to be added to the cookie list in the websocket request.

The polling request is handled internally to the socket.io client connect function, so I am unable to add the io cookie using the existing custom headers functionality. I am unable to change the configuration of the server, so support from the client side would be ideal.

After taking a look at _connect_polling() in client.py, it seems like it would be pretty easy to scan the response headers for set-cookie and add the cookies to the existing headers. Something like this would suffice:

for c in  r.headers['set-cookie']:
    if not 'cookie' in headers:
        headers['cookie'] = c
    else:
        headers['cookie'] += c

Not sure if this is the best solution, but it seems like it would work to me.

@jackdreinhardt
Copy link
Author

Also, if there are other headers that behave like set-cookie, those could be handled in a similar fashion

@miguelgrinberg
Copy link
Owner

Are you using the latest version of this package?

@jackdreinhardt
Copy link
Author

I'm running the following:

python-engineio      3.11.2    
python-socketio      4.4.0

@miguelgrinberg
Copy link
Owner

Then I'm confused. Cookies send by the server in the HTTP transport are already passed on the websocket connection. Here is the relevant code that moves the cookies from HTTP to WS: https://github.com/miguelgrinberg/python-engineio/blob/master/engineio/client.py#L355-L360.

Why do you think this isn't working for you?

@jackdreinhardt
Copy link
Author

Seems like you're right. I directly looked at the sio.eio.http.cookies object and the cookie was set correctly. It must be going wrong when setting headers. I also need to add a cookie from outside sio.connect() using the extra headers, and this cookie does not get passed correctly to the session. Maybe an option to pass in a session would be desirable?

cookies = 'connect.sid=' + connectsid + ';'
sio.connect('http://localhost:8080', headers={'cookie': cookies})

print(sio.eio.http.cookies) # shows that the cookie set in polling was successful, but not the cookie set with the headers functionality
print(sio.eio.http.headers)

@miguelgrinberg
Copy link
Owner

miguelgrinberg commented Feb 7, 2020

Okay, so if I understand this the problem occurs when you have some cookies given as a header and some more that are set during the HTTP transport exchange. So it works fine with one type of cookie or the other, but not with both together?

That would mean that the websocket client gets confused if it gets cookies through the two arguments, and probably drops one of the sources of cookies.

@jackdreinhardt
Copy link
Author

Yes. However I have yet to test the case where cookies are only given as headers. I am looking into setting up a minimum working example where the server does not set any cookies.

@jackdreinhardt
Copy link
Author

jackdreinhardt commented Feb 7, 2020

This server does not send any Set-Cookie headers with the HTTP transport exchange. The client sets a cookie given as a header, but it does not appear in the sio.eio.http.cookies object. The server can be changed to send a Set-Cookie header by removing the { cookie: false} option from socketio.
Server:

var app = require('express')();
var http = require('http').createServer(app);
var io = require('socket.io')(http, { cookie: false });

io.on('connection', function(socket){
  console.log('a user connected');
});

http.listen(3000, function(){
  console.log('listening on *:3000');
});

Client:

import socketio

sio = socketio.Client(logger=True)
sio.connect('http://localhost:3000', headers={'Cookie': 'some-cookie'})
print(sio.eio.http.cookies)

@jackdreinhardt
Copy link
Author

I was able to work around this issue with the following code.

s = requests.Session()
payload = {
    'some-field': 'some-value'
}
s.post(URL, payload)

sio = socketio.Client(logger=True)
sio.eio.http = s

sio.connect(URL)
print(sio.eio.http.cookies) # contains cookies from the `s` and the HTTP transport response

@miguelgrinberg
Copy link
Owner

miguelgrinberg commented Mar 9, 2020

@jackdreinhardt if you have a moment and would like to test this fix I'd appreciate it.

The solution that I implemented combines any cookies given in headers, with cookies that are already in the cookie jar. Please install the master branch of this package to test. Thanks.

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

No branches or pull requests

2 participants