Http::Headers#merge が元リクエストを変更しないように修正

ActionDispatch::Http::Headers#merge は 新しい Http::Headers インスタンスを返すと文書化されているにも関わらず、実際には元リクエストの env を変更していました。本 PR はその副作用を排除し、ドキュメントどおりの純粋な挙動に戻す変更を導入します。

背景

Http::Headers#merge が期待どおりに動作しない原因は、シャローコピーの罠 にあります。Request#headers は memoization により一度生成した Http::Headers オブジェクトをインスタンス変数 @headers に保持し、そのオブジェクトは元リクエストへの参照 @req を内部に持ちます。merge が @req.dup.headers でコピーを作成すると、Object#dup はインスタンス変数を参照としてコピーするだけで、@headers は新しいリクエストでも元のオブジェクトを指したままです。その結果、merge! が共有ヘッダーオブジェクトを変更し、元リクエストの env が書き換わる という副作用が発生していました。

この問題はテストやミドルウェアで request.headers を一度でも呼び出すと顕在化し、実運用で予期しないヘッダーの上書きにつながっていました。PR の説明でも「シャローコピーがフットガンになる」ことが指摘されており、根本的な修正が求められていました。

技術的な変更

元実装では merge が次のように書かれていました。

def merge(headers_or_env)
  headers = @req.dup.headers
  headers.merge!(headers_or_env)
  headers
end

@req.dup が浅いコピーであるため、headers は元リクエストと同一の Http::Headers オブジェクトを参照し、結果として merge! が元リクエストを改変していました。修正後は 新しいリクエストオブジェクト を明示的に生成し、その上で Headers を構築します。

def merge(headers_or_env)
  headers = Headers.new(@req.dup)
  headers.merge!(headers_or_env)
  headers
end

Headers.new(@req.dup) により、Headers コンストラクタは dup されたリクエスト を受取り、内部で再度 Http::Headers を生成します。これにより @headers が新しいリクエストに対して再度 memoize され、元リクエストへの参照が残らないため、merge! は安全に新しいヘッダーオブジェクトだけを操作します。さらに、同様の振る舞いを保証するテストが追加されました。

test "merge does not mutate the original request when headers are memoized" do
  request = ActionDispatch::Request.new(
    "CONTENT_TYPE" => "text/plain",
    "HTTP_REFERER" => "/some/page"
  )
  request.headers # memoize the Http::Headers instance on the request

  combined = request.headers.merge("HTTP_HOST" => "http://example.com")

  assert_equal "http://example.com", combined["HTTP_HOST"]
  assert_not request.headers.key?("HTTP_HOST")
  assert_nil request.get_header("HTTP_HOST")
end

このテストは ヘッダーが memo化されていても merge が元リクエストを汚染しないことを検証し、回帰防止の役割も果たします。

設計判断

修正方針は 最小限の侵入性 を保ちつつ期待通りの純粋性を実現することでした。Headers.new(@req.dup) への置き換えは、既存の API シグネチャや外部からの呼び出し方法を変更せずに問題を解決できるシンプルなアプローチです。代替案として Headers に直接 env を渡す形や、dup の深いコピー実装を導入する案も考えられましたが、前者は API の拡張で互換性リスクが高く、後者は Rack::Request の内部実装に依存するため保守コストが増大すると判断されました。

結果として、シャローコピーの影響範囲を限定 しつつ、merge の動作を文書どおりの「新しいインスタンス返却」に合わせました。テスト追加により、将来的な変更が同様の副作用を再び導入しないことが保証されます。

まとめ

本 PR は Http::Headers#merge が元リクエストを不意に変更していたバグを、Headers.new(@req.dup) に置き換えるだけのシンプルな修正で解消し、ドキュメントどおりの不変性を回復させました。追加されたテストにより、ヘッダーが memo化されている状態でも安全にマージできることが検証され、今後のメンテナンスでも安心して利用できるようになりました。