Skip to content

Conversation

@MSP-Greg
Copy link
Member

@MSP-Greg MSP-Greg commented Sep 9, 2022

Description

Closes #2931

Your checklist for this pull request

  • I have reviewed the guidelines for contributing to this repository.
  • I have added (or updated) appropriate tests if this PR fixes a bug or adds a feature.
  • My pull request is 100 lines added/removed or less so that it can be easily reviewed.
  • If this PR doesn't need tests (docs change), I added [ci skip] to the title of the PR.
  • If this closes any issues, I have added "Closes #issue" to the PR description or my commit messages.
  • I have updated the documentation accordingly.
  • All new and existing tests passed, including Rubocop.


def send_http_and_read(req)
send_http(req).read
send_http(req).sysread 10_240
Copy link
Member

Choose a reason for hiding this comment

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

🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

read is evil. I was working on this, and with read, the new test took 20 sec. With sysread, it was what it should be.

But, then the Windows tests (only Ruby 3.1 and later), weren't getting the full response. But, using PR #2896, no problem.

Some of the issue is using the same Process for the server and client...

Copy link
Member

Choose a reason for hiding this comment

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

I see 👍 Lets refactor the magic number into a constant somwhere then.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done. Added comment also, as the value is dependent on the tests. I've used looping code for responses with unknown length...

Copy link
Member

@nateberkopec nateberkopec left a comment

Choose a reason for hiding this comment

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

Just left some thoughts.

@nateberkopec nateberkopec merged commit e2ef83b into puma:master Sep 9, 2022
@MSP-Greg MSP-Greg deleted the 00-rack-conform branch September 11, 2022 17:24
JoeDupuis pushed a commit to JoeDupuis/puma that referenced this pull request May 28, 2023
* request.rb - allow header array value

* test_response_header.rb - add test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Does Puma support Array headers?

2 participants