Skip to content

Conversation

@MSP-Greg
Copy link
Member

@MSP-Greg MSP-Greg commented Oct 26, 2022

Description

  1. At present, some code paths will not close the app body. Add code to always hold a reference to the app body, so when code calling lowlevel_error is executed, the reference to the app body is maintained.

  2. test_rack_server.rb - add test_hijack_body_close - test for above

  3. Small refactor of test_puma_server.rb. Takes too damn long.

  4. test_puma_server.rb - add test_lowlevel_error_body_close to verify body close call when lowlevel_error

Closes #2999.

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.

miharekar added a commit to miharekar/visualizer that referenced this pull request Oct 26, 2022
@MSP-Greg MSP-Greg marked this pull request as ready for review October 28, 2022 21:05
@MSP-Greg
Copy link
Member Author

@MSP-Greg
Copy link
Member Author

MSP-Greg commented Nov 2, 2022

One person stated that this PR fixed the issue re #2999, see comment.

Re #3004, as mentioned, testing with hotwired/turbo-rails, I may write a few more tests.

This will cause merge conflicts with #3004, so I'd like to merge this.

Thoughts?

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.

Puma 6.0.0 does not close the response bodies of hijacked requests

2 participants