Proposed solution for Issue #104: multiple cookies

[MarkTsengTW]

Patch so that the _parse_headers function handles multiple cookies by appending them, separated by a comma and space. This mimics the behaviour of Python requests when it comes to response.headers['set-cookie'] and response.headers.items().

Currently this function just writes over any previous cookie, leaving only the last one. That doesn't allow it to work with some APIs, e.g. the Growatt solar API that I've been working on.

Thrilled to be submitting my first public pull request. Thanks to @askpatrickw and @tekktrik for their encouragement.

Hope I've sent it to the right place this time!

[MarkTsengTW]

Hi @tekktrik I've made the change requested (thanks for feedback) and the second commit is showing up in this pull request. Just now it came up with Build CI / test (pull_request) not successful due to being cancelled, but not sure if this is a problem. Happy to do any further work needed!

[tekktrik]

Weird about the CI, but I re-ran it and it's all clear!

[MarkTsengTW]

Woohoo! Thanks for your support throughout the process.

[tekktrik]

My pleasure! Congrats on your first pull request!

[tekktrik]

Do you have a test site that provides multiple cookies? I want to test this out, as well as check that it freezes into the firmware for some boards okay (like the MagTag). What site where you using when you noticed this issue?

[MarkTsengTW]

@tekktrik I have been working on https://server-api.growatt.com/, which has cookies for JSESSIONID and SERVERID. (My JSESSIONID cookie was getting overwritten by the SERVERID, making it impossible to do anything after logging in.)

Hopefully you can register an account and sign in without actually owning a Growatt solar inverter. The code to use the API is here: https://github.com/indykoning/PyPi_GrowattServer. All you need to do is the initial post in the login function and you will see the cookies in the response.

I've been adapting the above code for the Matrix Portal. My current script is an embarrassing pile of spaghetti that only prints to the serial console, but I can share a version privately if it helps.

[askpatrickw]

Should this have a test added as well?

[tekktrik]

Just chiming in to say that this is still on my radar! I will likely create a local server with FastAPI or something too test!

@askpatrickw - it may be worth it, if we can rely on a site to provide multiple cookies consistently. Is it worth asking Adafruit to set up an endpoint for this?

[tekktrik]

Alright, managed to deploy a simple heroku server to spit out cookies and confirmed that it does behave as expected!

[FoamyGuy]

adafruit.com seems to return two set-cookie headers:

Although it's also somewhat large of a response for CircuitPython, might cause other issues. I would guess lots of public APIs will end up having multiple. I'll check a few that I've used recently.

[FoamyGuy]

Another potential option is to include a FastAPI or Flask based script that implements a basic server that returns multiple headers to a GET request. That could be included in the repo with instructions to run it on a PC and then make the request to the PCs IP address.

[tekktrik]

I could include the test script I used. I did have an issue running the FastAPI on localhost but I could have just messed something up that got fixed when I deployed to heroku. I can confirm that and push it to the examples folder for this PR.

[tekktrik]

Looks like a call to https://www.wikipedia,org returns two cookies!

[tekktrik]

@FoamyGuy @MarkTsengTW @askpatrickw Okay, turns out using www.adafruit.com is perfect. I had trouble adding an automated test, so I instead added an example to this PR on how to handle (and manipulate) multiple cookies. Let me know if there's anything else.

[tekktrik]

New example was tested using my MagTag, forgot to mention.

[FoamyGuy]

I modified the example to work on PyPortal and when I run it I'm only seeing a single cookie outputted:

Fetching multiple cookies from https://www.adafruit.com
Number of cookies: 1

Cookies received:
----------------------------------------
wishlist=deleted; expires=Thu, 01-Jan-1970 00:00:01 GMT; Max-Age=0; path=/; domain=.adafruit.com; Secure
----------------------------------------

However when I test in a browser on the PC I'm seeing two set-cookie headers, one for wishlist, and one for cart_count.

Is the cart_count one getting lost somehow in this use case? or is the server returning different amounts based on the thing that is making the request or something?

@tekktrik what does it output for you when you run on the MagTag?

[tekktrik]

Interestingly enough, I get THREE cookies:

Cookies received:
----------------------------------------
zenid=XXXXXXX expires=Sat, 30-Apr-2022 22:10:40 GMT; Max-Age=1209600; path=/; domain=.adafruit.com; secure; HttpOnly; Secure
----------------------------------------
cart_count=0; expires=Sat, 30-Apr-2022 22:10:40 GMT; Max-Age=1209600; path=/; domain=.adafruit.com; Secure
----------------------------------------
wishlist=deleted; expires=Thu, 01-Jan-1970 00:00:01 GMT; Max-Age=0; path=/; domain=.adafruit.com; Secure
----------------------------------------

[tekktrik]

Maybe www.wikipedia.org will work better?

[FoamyGuy]

I tried switching over to https://www.widipledia.org and I see a single cookie with that URL as well when I request it from the pyportal. Strangely I see no set-cookie headers when I load it in firefox on my PC.

I'll try an esp32-S2 based device today to see if it is a difference between that and esp32spi. I can try to get a flask server set up as well to test locally in an environment where I can be 100% certain of the amount of cookies.

[tekktrik]

If you want to use my heroku website I deployed, it's https://enigmatic-refuge-56866.herokuapp.com/

/get-cookie returns one cookie. /get-cookies returns two.

[FoamyGuy]

@tekktrik thanks, I'm giving that one a try. Does it require POST requests specifically or some method other than GET? when I load it in a browser I'm getting this: and I don't see any set-cookie headers returned with it:

[tekktrik]

Whoops, yeah should have mentioned they're POST!

[FoamyGuy]

Ah, I am seeing a single cookie from the "cookies" endpoint on the pyportal:

I'm guessing there must be some difference between esp32spi and builtin-wifi contexts that could be causing this.

[tekktrik]

It looks like you're only getting the second cookie, which is the previous behavior. Is your requests library in the CIRCUITPY folder? If it's in the lib folder it won't take precedence over the built-in.

[FoamyGuy]

Doh! that was the problem 🤦. Can't believe I did that after just discussing during deep dive on friday.

I put the modified library in the root and this is indeed working correctly for adafruit.com and your heroku app.

If we were making our own API I think making response['headers'] a List instead of a comma separated string would be nice to avoid needing the "day of week" check logic. But if this behavior matches CPython requests library then that is how we want to keep it.

Changes look good to me. Thanks @MarkTsengTW for this enhancement and @tekktrik for help testing.

[MarkTsengTW]

Yay! Thanks @FoamyGuy and @tekktrik. I learned a lot from reading your discussion about testing. So happy to see this merged!