リクエスト処理コードをclient_env.rbに集約
client.rbとrequest.rbに分散していたリクエストのenv処理コードをclient_env.rbに集約し、コードの責務を明確化した変更です。これにより、リクエスト検証ロジックが一箇所にまとまり、テストも容易になりました。
背景
Pumaでは、リクエストのenv(環境変数ハッシュ)を処理するコードがclient.rbとrequest.rbの2つのファイルに分散していました。#3540で報告されたバグ(http_content_length_limitを超えた際のエラーメッセージが不適切)の修正にあたり、この構造が問題を複雑にしていることが明らかになりました。
リクエストの検証とenvの正規化は密接に関連する処理であり、これらが別々のファイルに存在することで、コードの追跡や修正が困難になっていました。特に、http_content_length_limitのような制約の検証ロジックが適切な場所に配置されていませんでした。
技術的な変更
新たにlib/puma/client_env.rbを作成し、ClientEnvモジュールとしてenv処理コードを集約しました。
主要な移動コード:
module ClientEnv
include Puma::Const
def normalize_env
if host = @env[HTTP_HOST]
if colon = host.rindex("]:") # IPV6 with port
@env[SERVER_NAME] = host[0, colon+1]
@env[SERVER_PORT] = host[colon+2, host.bytesize]
elsif !host.start_with?("[") && colon = host.index(":") # not hostname or IPV4 with port
@env[SERVER_NAME] = host[0, colon]
@env[SERVER_PORT] = host[colon+1, host.bytesize]
else
@env[SERVER_NAME] = host
@env[SERVER_PORT] = default_server_port
end
else
@env[SERVER_NAME] = LOCALHOST
@env[SERVER_PORT] = default_server_port
end
# ...
end
end
Clientクラスはこのモジュールをインクルードし、env処理の実装を継承します。request.rbはresponse.rbに名前変更され、レスポンス生成に関する処理のみを保持するようになりました。
責務の明確化:
-
Clientクラス: ソケットラッパーとしての役割を保持 -
ClientEnvモジュール:envの検証・正規化ロジックを集約 -
Responseモジュール: レスポンス生成のみを担当(旧Requestから改名)
テストコードも再編成され、test_request_single.rbとtest_request_invalid_multiple.rbが拡充されました。これにより、サーバーを起動せずにClientクラス単体でリクエスト処理のテストが可能になっています。
設計判断
Moduleパターンの採用により、既存のClientクラスへの影響を最小限に抑えています。include ClientEnvにより、Clientのインターフェースを変更することなく、内部実装の再編成を実現しました。
PR内のコメントでは、http_content_length_limitの検証タイミングも見直されています。従来はrequest.rb内のhandle_requestで検証していましたが、これをClientの責務に移すことで、リクエスト解析の早い段階でエラーを検出できるようになりました。これは#3540の根本原因(エラーメッセージの不整合)を解消する設計判断です。
テストファイルの統合も重要な判断です。test_normalize.rbを削除し、test_request_single.rbに統合することで、テストコードの重複を排除し、保守性を向上させています。テストがClientクラスの責務に沿って整理されたことで、将来の変更時にも影響範囲が把握しやすくなりました。
まとめ
本PRは、リクエスト処理コードの責務を再定義し、client_env.rbに集約することで、コードベースの保守性を向上させた変更です。envの検証・正規化ロジックを一箇所にまとめることで、#3540のようなバグの修正が容易になり、今後の機能追加もより明確な構造の上で行えるようになりました。