Skip to content

Add, test support for chunk extensions #46

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

Merged
merged 3 commits into from
Nov 10, 2020
Merged

Add, test support for chunk extensions #46

merged 3 commits into from
Nov 10, 2020

Conversation

jepler
Copy link
Contributor

@jepler jepler commented Oct 31, 2020

The chunk line is read up to the terminating "\r\n", then any extensions are ignored by only using the string up to the first ";" (if any) when determining the length of the chunk.

Resources:
https://tools.ietf.org/html/rfc7230#section-4.1.1
https://www.boost.org/doc/libs/1_74_0/libs/beast/doc/html/beast/using_http/chunked_encoding.html

Thanks to @anecdata for asking on Discord why ";" was getting special treatment, leading to this investigation. We don't know of a specific URL or service that is affected by this bug.

The chunk line is read up to the terminating "\r\n", then any extensions are
ignored by only using the string up to the first ";" (if any) when determining
the length of the chunk.

Resources:
https://tools.ietf.org/html/rfc7230#section-4.1.1
https://www.boost.org/doc/libs/1_74_0/libs/beast/doc/html/beast/using_http/chunked_encoding.html
@tannewt tannewt self-requested a review November 2, 2020 23:56
Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

Thanks for finding this issue!

print(c)
sock = mocket.Mocket(headers + c)
pool.socket.return_value = sock
for extra in (b"", b";blahblah; blah"):
Copy link
Member

Choose a reason for hiding this comment

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

Mind copying and pasting this into a new test function? That will lead to a clearer failure if/when it fails. Right now it's lumped into the basic case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've updated the test, let me know if it's still not what you're looking for.

@tannewt
Copy link
Member

tannewt commented Nov 6, 2020

@jepler Are you going to follow up on this?

Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

Works for me! Black is unhappy though.

@jepler jepler requested a review from tannewt November 10, 2020 03:20
Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

Thank you!

@tannewt tannewt merged commit ba76312 into master Nov 10, 2020
adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Nov 14, 2020
Updating https://github.com/adafruit/Adafruit_CircuitPython_IL0373 to 1.3.2 from 1.3.1:
  > Merge pull request adafruit/Adafruit_CircuitPython_IL0373#14 from tannewt/tune_grayscale

Updating https://github.com/adafruit/Adafruit_CircuitPython_LIS3MDL to 1.1.6 from 1.1.5:
  > Merge pull request adafruit/Adafruit_CircuitPython_LIS3MDL#14 from Breefield/patch-1

Updating https://github.com/adafruit/Adafruit_CircuitPython_MONSTERM4SK to 0.1.1 from 0.1.0:
  > Merge pull request adafruit/Adafruit_CircuitPython_MONSTERM4SK#5 from FoamyGuy/fix_refrences_to_old_repo_name

Updating https://github.com/adafruit/Adafruit_CircuitPython_AdafruitIO to 4.3.0 from 4.2.2:
  > Merge pull request adafruit/Adafruit_CircuitPython_AdafruitIO#50 from brentru/fix-throttling-message-parsing

Updating https://github.com/adafruit/Adafruit_CircuitPython_MagTag to 1.0.1 from 1.0.0:
  > Merge pull request adafruit/Adafruit_CircuitPython_MagTag#2 from ladyada/main

Updating https://github.com/adafruit/Adafruit_CircuitPython_MatrixPortal to 1.9.4 from 1.9.3:
  > Merge pull request adafruit/Adafruit_CircuitPython_MatrixPortal#50 from makermelissa/master

Updating https://github.com/adafruit/Adafruit_CircuitPython_Requests to 1.8.0 from 1.7.5:
  > Merge pull request adafruit/Adafruit_CircuitPython_Requests#46 from adafruit/jepler-patch-1

Updating https://github.com/adafruit/Adafruit_CircuitPython_Slideshow to 1.3.1 from 1.3.0:
  > Merge pull request adafruit/Adafruit_CircuitPython_Slideshow#24 from FoamyGuy/alignment_example

Updating https://github.com/adafruit/Adafruit_CircuitPython_Bundle/circuitpython_library_list.md to NA from NA:
  > Added the following libraries: Adafruit_CircuitPython_MagTag
@evaherrada evaherrada deleted the jepler-patch-1 branch June 21, 2021 13:58
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.

2 participants