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

set defaults for settings, and respect max_frame_size in sending #7

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kahuang
Copy link

@kahuang kahuang commented Feb 21, 2019

responses to clients

@kahuang
Copy link
Author

kahuang commented Feb 21, 2019

I'm using this http2 server as a same-process testing server for a tornado based http2 client. For large body values, the server wasn't respecting the max frame size, causing errors on the client side:

File "build/bdist.linux-x86_64/egg/h2/frame_buffer.py", line 150, in next self._validate_frame_length(length) File "build/bdist.linux-x86_64/egg/h2/frame_buffer.py", line 81, in _validate_frame_length (length, self.max_frame_size) FrameTooLargeError: Received overlong frame: length 65535, max 16384

This PR solves that issue by ensuring that 1. We have our settings initialized as the defaults and 2. We respect the MAX_FRAME_SIZE setting

Copy link
Owner

@bdarnell bdarnell 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 the contribution! Can you add a test that runs into this problem? Probably a version of ServerTest.test_large_response that writes its payload all at once instead of in small chunks.

tornado_http2/connection.py Outdated Show resolved Hide resolved
@kahuang
Copy link
Author

kahuang commented Feb 25, 2019

I've added a test that writes the payload all at once, instead of in chunks.

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