Extract req/resp methods to new request.rb from server.rb

[MSP-Greg]

Description

server.rb is a very long file with several very long methods.

1. Extract default_server_port, fast_write, fetch_status_code, handle_request, normalize_env, possible_header_injection?, and str_early_hints from server.rb to request.rb

2. Extract req_env_post_parse and str_headers from handle_request

3. Updated comments/documentation

See https://msp-greg.github.io/puma/Puma/Request.html for the doc of the PR.

Your checklist for this pull request

• I have reviewed the guidelines for contributing to this repository.
• I have added an entry to History.md if this PR fixes a bug or adds a feature. If it doesn't need an entry to HISTORY.md, I have added [changelog skip] or [ci skip] to the pull request title.
• I have added 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.

[nateberkopec]

If this is going to be a composed object extraction, this file should be Puma::Server::Request and then live in lib/puma/server/request.rb.

[nateberkopec]

It looks like you've found a seam but haven't quite cut through all the way yet. I think you can go all the way to a composed object:

while true 
  case Request.new(client,buffer).handle

... and then in lib/puma/server/request.rb, everything except handle_request can be marked private.

This would greatly reduce the API surface of Server. Right now this moves methods around in different files, but with a composed object we could really make Server a lot smaller.

[MSP-Greg]

I did think about this. Two items:

1. It would be a breaking API change. handle_request and normalize_env are currently public methods of Server.

2. Coupling - the following Server instance variables are used in Request code:

@app              available
@early_hints      available
@events           available
@queue_requests   hidden
@thread_pool      hidden

We could add a couple of readers and pass the server instance into the class init? Also, re class init, I'm not sure if the buffer should/can be contained within Request, rather than be passed in.

I thought that the current layout would maintain the API, but make it easy to create a class when v6.0.0 was being considered.

But, re API changes, the earlier issue with the API changes I made affected parts of the API that could be used for Puma startup/setup. These changes are more 'runtime'. Possibly less likely to cause issues...

[nateberkopec]

OK, got it. I'll re-review this with a mind to "this is a temporary stopping point to an extraction"

[nateberkopec]

Ok, LGTM. We can revisit this for an extracted object when 6.0 rolls around.

[nateberkopec]

Wrong button, lol