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

Correctly handle headers of different types #152

Merged
merged 1 commit into from
Feb 3, 2024

Conversation

justmobilize
Copy link
Collaborator

@justmobilize justmobilize commented Jan 26, 2024

This stems from this issue in Adafruit_CircuitPython_AdafruitIO that caused this issue to be re-opened.

If this path looks good I will add tests for the changes.

Comment on lines +540 to +542
raise AttributeError(
f"Header part ({value}) from {key} must be of type str or bytes, not {type(value)}"
)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in CPython requests this is:

        raise InvalidHeader(
            f"Invalid leading whitespace, reserved character(s), or return "
            f"character(s) in header {header_kind}: {header_part!r}"
        )

An AttributeError seems similar enough and didn't know if creating a new Exception would be wanted

@@ -555,9 +567,14 @@ def _send_as_bytes(self, socket: SocketType, data: str):
return self._send(socket, bytes(data, "utf-8"))

def _send_header(self, socket, header, value):
if value is None:
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In CPython requests they have a method merge_setting

That they use in a bunch of places, but it creates new dict objects, and thought this would be simpler in memory usage

@justmobilize justmobilize marked this pull request as ready for review January 26, 2024 19:54
Copy link
Contributor

@FoamyGuy FoamyGuy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. Agree with both of the points you mentioned in other comments.

Thanks for the fix @justmobilize!

I tested this successfully with a Feather ESP32S3 TFT with:

  • requests simpletest
  • adafruit io simpletest
  • adafruit io http simpletest

All appear to function as intended with this version.

@FoamyGuy FoamyGuy merged commit 2afa03d into adafruit:main Feb 3, 2024
1 check passed
@justmobilize justmobilize deleted the fix-null-headers branch February 3, 2024 19:12
@brentru
Copy link
Member

brentru commented Feb 5, 2024

@FoamyGuy Can we release this library, is it picked up by Adabot?

@FoamyGuy
Copy link
Contributor

FoamyGuy commented Feb 5, 2024

Yep, new release is made now.

adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Feb 6, 2024
Updating https://github.com/adafruit/Adafruit_CircuitPython_DS18X20 to 1.4.0 from 1.3.19:
  > Merge pull request adafruit/Adafruit_CircuitPython_DS18X20#29 from ilikecake/main

Updating https://github.com/adafruit/Adafruit_CircuitPython_FocalTouch to 1.5.2 from 1.5.1:
  > Merge pull request adafruit/Adafruit_CircuitPython_FocalTouch#32 from ilikecake/main

Updating https://github.com/adafruit/Adafruit_CircuitPython_Seesaw to 1.16.2 from 1.16.1:
  > Merge pull request adafruit/Adafruit_CircuitPython_seesaw#129 from scirelli/issue_128

Updating https://github.com/adafruit/Adafruit_CircuitPython_Display_Button to 1.9.1 from 1.9.0:
  > Merge pull request adafruit/Adafruit_CircuitPython_Display_Button#44 from DJDevon3/DJDevon3_Working_Branch

Updating https://github.com/adafruit/Adafruit_CircuitPython_DisplayIO_Layout to 2.1.0 from 2.0.1:
  > Merge pull request adafruit/Adafruit_CircuitPython_DisplayIO_Layout#92 from FoamyGuy/gridlayout_cell_contains

Updating https://github.com/adafruit/Adafruit_CircuitPython_HTTPServer to 4.5.3 from 4.5.1:
  > Merge pull request adafruit/Adafruit_CircuitPython_HTTPServer#79 from michalpokusa/cpython-fix
  > Merge pull request adafruit/Adafruit_CircuitPython_HTTPServer#78 from tedder/ted/cpython_port5k

Updating https://github.com/adafruit/Adafruit_CircuitPython_LED_Animation to 2.9.0 from 2.8.0:
  > Merge pull request adafruit/Adafruit_CircuitPython_LED_Animation#116 from tylerwinfield/main

Updating https://github.com/adafruit/Adafruit_CircuitPython_PyCamera to 0.0.9 from 0.0.8:
  > Merge pull request adafruit/Adafruit_CircuitPython_PyCamera#23 from adafruit/blendmode
  > Merge pull request adafruit/Adafruit_CircuitPython_PyCamera#22 from adafruit/timelapse
  > Merge pull request adafruit/Adafruit_CircuitPython_PyCamera#21 from adafruit/fix_rl

Updating https://github.com/adafruit/Adafruit_CircuitPython_Requests to 2.0.5 from 2.0.4:
  > Merge pull request adafruit/Adafruit_CircuitPython_Requests#152 from justmobilize/fix-null-headers

Updating https://github.com/adafruit/Adafruit_CircuitPython_Bundle/circuitpython_library_list.md to NA from NA:
  > Updated download stats for the libraries
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants