adafruit / Adafruit_CircuitPython_Requests

Requests-like interface for web interfacing
MIT License
51 stars 36 forks source link

Fix duplicate header issue #149

Closed justmobilize closed 6 months ago

justmobilize commented 6 months ago

When helping a member in the discord channel #help-with-circuitpython, I had made the mistake of thinking that this library didn't automatically deal with turning a dict into a urlencoded string. I had them convert it by hand and try and it worked.

As it turns out - it does, but also sends the header Content-Type: application/x-www-form-urlencoded, which some systems (Spotify as one) can't handle getting the header twice.

This would fix issue https://github.com/adafruit/Adafruit_CircuitPython_Requests/issues/148

Test:

from os import getenv
import adafruit_requests
import socketpool
import ssl
import wifi

wifi_ssid = getenv("CIRCUITPY_WIFI_SSID")
wifi_password = getenv("CIRCUITPY_WIFI_PASSWORD")
client_id = getenv("SPOTIFY_CLIENT_ID")
client_secret = getenv("SPOTIFY_CLIENT_SECRET")

ssl_context=ssl.create_default_context()
pool = socketpool.SocketPool(wifi.radio)
wifi.radio.connect(wifi_ssid, wifi_password)

requests = adafruit_requests.Session(pool, ssl_context)

data = {"grant_type": "client_credentials", "client_id": client_id, "client_secret": client_secret}
headers = {"Content-Type": "application/x-www-form-urlencoded"}
url = "https://accounts.spotify.com/api/token"
r = requests.post(url, data=data, headers=headers)
print(r.status_code)
print(r.text)

Using the main branch of Adafruit_CircuitPython_Requests, this will return a 400 because it sends Content-Type twice, using this branch will succeed

justmobilize commented 6 months ago

This may also resolve: https://github.com/adafruit/Adafruit_CircuitPython_Requests/issues/40

justmobilize commented 6 months ago

This looks good to me. Thanks for the fix @justmobilize!

I confirmed the duplicated Content-Type header issue with a simple http server and wireshark to view the raw TCP data stream. I tried a few other things before resorting to wireshark, but flask and a few of the other basic server examples I tried made it difficult for me to see the raw request. Many of them treat headers as dictionary-like so it seems to resolve duplicates without much trouble.

I confirmed the fix does successfully get rid of that duplicated header.

All testing was was done on a QTPy ESP32-S2 8.2.9

Thanks for testing! And happy to do this (and I hope a bunch more)

FoamyGuy commented 6 months ago

Looks like all requested changes have been addressed. Merging this one. Thanks again for all of your work on this @justmobilize!