add port back to host arguments (sends correct host-header, fixes redirects)#218
add port back to host arguments (sends correct host-header, fixes redirects)#218bablokb wants to merge 3 commits into
Conversation
|
Can someone check this please? It seems the tests are confused by the additional port information. The PR could be changed to only include the port if the port is not a standard port, but this would bloat the code without functional benefits. |
|
@bablokb you'll need to update all the tests. Random example: Adafruit_CircuitPython_Requests/tests/reuse_test.py Lines 175 to 182 in 82a656a If we sent the port on all of these, then we'll need to update all the tests. I also don't think a single if statement for a non-standard port would add that much bloat |
|
@justmobilize I changed the code: the hostname is passed as is (except to the socket-constructor), so the tests don't fail since they don't see the port anymore. |
|
@bablokb is it possible to add a test that hits that block? |
|
For someone who knows how to write and verify tests, certainly. But this is currently beyond my reach. From what the code does, it would be sufficient to pass a hostname:port combination, e.g. www.adafruit.com:80 or www.adadruit.com:443. Although these are the default ports, they get passed since the port is provided explicitly. |
The current version of
request()strips the port from the host-part of the url. Subsequently,_send_request()sends a wrong host-header. Also when relative redirects are processed, the missing original port will redirect to a wrong url.