[インデックス 16207] ファイルの概要

このコミットは、Go言語の標準ライブラリである net/http パッケージにおける http.Redirect 関数が特定の条件下でパニックを引き起こす可能性があったバグを修正するものです。具体的には、urlStr 引数が空文字列の場合に発生するパニックを解消し、より堅牢なリダイレクト処理を実現しています。

コミット

commit 8b29f158add964b788cea61ad608f042238f52ba
Author: Brad Fitzpatrick <bradfitz@golang.org>
Date:   Sat Apr 20 17:20:14 2013 -0700

    net/http: fix a panic in Redirect
    
    R=golang-dev, dsymonds, r
    CC=golang-dev
    https://golang.org/cl/8721045

GitHub上でのコミットページへのリンク

https://github.com/golang/go/commit/8b29f158add964b788cea61ad608f042238f52ba

元コミット内容

net/http: fix a panic in Redirect

このコミットは、http.Redirect 関数におけるパニックを修正することを目的としています。

変更の背景

Go言語の net/http パッケージは、HTTPクライアントとサーバーの実装を提供します。http.Redirect 関数は、HTTPリクエストを別のURLにリダイレクトするために使用されます。この関数は、指定されたURLとHTTPステータスコード（例: 301 Moved Permanently, 302 Found, 303 See Other, 307 Temporary Redirect, 308 Permanent Redirect）を使用して、クライアントにリダイレクトを指示するレスポンスを書き込みます。

このコミットが作成された時点では、http.Redirect 関数内でURLの末尾のスラッシュを処理するロジックに脆弱性がありました。具体的には、urlStr 引数が空文字列の場合、urlStr[len(urlStr)-1] のように文字列の最後の文字にアクセスしようとすると、len(urlStr) が0であるため、インデックスが負になり、ランタイムパニック（panic: runtime error: index out of range）が発生する可能性がありました。これは、不正な入力であってもアプリケーションがクラッシュすべきではないという原則に反するものでした。

このバグは、特に開発者が意図せず空のURLを http.Redirect に渡してしまった場合や、動的に生成されるURLが何らかの理由で空になってしまった場合に、サーバーアプリケーションが予期せず停止する原因となり得ました。そのため、このパニックを修正し、より堅牢なエラーハンドリングを実現することが求められました。

前提知識の解説

• net/http パッケージ: Go言語の標準ライブラリで、HTTPクライアントとサーバーの機能を提供します。Webアプリケーション開発において中心的な役割を果たします。
• http.Redirect 関数: func Redirect(w ResponseWriter, r *Request, urlStr string, code int) というシグネチャを持つ関数で、HTTPリダイレクトレスポンスを生成し、クライアントに送信します。urlStr はリダイレクト先のURL、code はHTTPステータスコードです。
• path.Clean: path パッケージの関数で、URLパスを正規化します。例えば、a/./b は a/b に、a/../b は b に、//a は /a になります。
• strings.HasSuffix: strings パッケージの関数で、ある文字列が特定のサフィックス（末尾の文字列）で終わるかどうかを判定します。例えば、strings.HasSuffix("foo/", "/") は true を返します。
• パニック (Panic): Go言語におけるランタイムエラーの一種です。通常、プログラムが回復不能な状態に陥った場合に発生し、プログラムの実行を停止させます。パニックは、配列の範囲外アクセス、nilポインタのデリファレンスなど、プログラマーの論理的な誤りによって引き起こされることが多いです。
• HTTPステータスコード: HTTPレスポンスの最初の行に含まれる3桁の数字で、リクエストの結果を示します。リダイレクトに関連するコードには、301 (Moved Permanently), 302 (Found), 303 (See Other), 307 (Temporary Redirect), 308 (Permanent Redirect) などがあります。

技術的詳細

このコミットの主要な変更点は、net/http/server.go ファイル内の Redirect 関数におけるURLの末尾スラッシュ処理の改善です。

修正前のコードでは、urlStr の末尾がスラッシュであるかを判定するために urlStr[len(urlStr)-1] == '/' という直接的なインデックスアクセスを使用していました。この方法は、urlStr が空文字列 ("") の場合、len(urlStr) が 0 となり、urlStr[-1] のような無効なインデックスアクセスが発生し、パニックを引き起こしていました。

修正後のコードでは、この脆弱なインデックスアクセスを strings.HasSuffix(urlStr, "/") に置き換えています。strings.HasSuffix 関数は、文字列が空である場合でも安全に false を返すため、パニックを回避できます。

同様に、path.Clean 適用後に末尾スラッシュが失われたかどうかをチェックする部分も、urlStr[len(urlStr)-1] != '/' から !strings.HasSuffix(urlStr, "/") に変更されています。これにより、path.Clean の結果が空文字列になった場合でも安全性が保たれます。

また、テストコード (src/pkg/net/http/client_test.go と src/pkg/net/http/server_test.go) も更新されています。 client_test.go では、TestRedirects 関数が TestClientRedirects にリネームされています。 server_test.go では、TestServerRedirect という新しいテストケースが追加されています。このテストは、http.Redirect 関数に意図的に不正なパス（先頭スラッシュがないパス）を持つリクエストを渡し、パニックが発生しないこと、そして期待されるHTTPステータスコード（304 Not Modified）が返されることを確認しています。このテストは、まさに修正されたパニックのシナリオを再現し、修正が正しく機能していることを検証するためのものです。 さらに、テストヘルパーとして使用されていた codeSaver 構造体とその関連メソッドが削除され、代わりに httptest.NewRecorder() を使用するように変更されています。これは、Goのテストフレームワークにおけるより標準的で推奨される方法への移行を示しています。

コアとなるコードの変更箇所

src/pkg/net/http/server.go

--- a/src/pkg/net/http/server.go
+++ b/src/pkg/net/http/server.go
@@ -1222,9 +1222,9 @@ func Redirect(w ResponseWriter, r *Request, urlStr string, code int) {
 			}
 
 			// clean up but preserve trailing slash
-			trailing := urlStr[len(urlStr)-1] == '/'
+			trailing := strings.HasSuffix(urlStr, "/")
 			urlStr = path.Clean(urlStr)
-			if trailing && urlStr[len(urlStr)-1] != '/' {
+			if trailing && !strings.HasSuffix(urlStr, "/") {
 				urlStr += "/"
 			}
 			urlStr += query

src/pkg/net/http/server_test.go

--- a/src/pkg/net/http/server_test.go
+++ b/src/pkg/net/http/server_test.go
@@ -2,9 +2,11 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.\n
-package http
+package http_test
 
 import (
+\t. "net/http"
+\t"net/http/httptest"
 	"net/url"
 	"testing"
 )
@@ -76,20 +78,27 @@ func TestServeMuxHandler(t *testing.T) {
 			},
 		}
 		h, pattern := mux.Handler(r)
-		cs := &codeSaver{h: Header{}}
-		h.ServeHTTP(cs, r)
-		if pattern != tt.pattern || cs.code != tt.code {
-			t.Errorf("%s %s %s = %d, %q, want %d, %q", tt.method, tt.host, tt.path, cs.code, pattern, tt.code, tt.pattern)
+		rr := httptest.NewRecorder()
+		h.ServeHTTP(rr, r)
+		if pattern != tt.pattern || rr.Code != tt.code {
+			t.Errorf("%s %s %s = %d, %q, want %d, %q", tt.method, tt.host, tt.path, rr.Code, pattern, tt.code, tt.pattern)
 		}
 	}
 }
 
-// A codeSaver is a ResponseWriter that saves the code passed to WriteHeader.
-type codeSaver struct {
-	h    Header
-	code int
+func TestServerRedirect(t *testing.T) {
+	// This used to crash. It's not valid input (bad path), but it
+	// shouldn't crash.
+	rr := httptest.NewRecorder()
+	req := &Request{
+		Method: "GET",
+		URL: &url.URL{
+			Scheme: "http",
+			Path:   "not-empty-but-no-leading-slash", // bogus
+		},
+	}
+	Redirect(rr, req, "", 304)
+	if rr.Code != 304 {
+		t.Errorf("Code = %d; want 304", rr.Code)
+	}
 }
--
-func (cs *codeSaver) Header() Header              { return cs.h }
-func (cs *codeSaver) Write(p []byte) (int, error) { return len(p), nil }
-func (cs *codeSaver) WriteHeader(code int)        { cs.code = code }

コアとなるコードの解説

src/pkg/net/http/server.go の変更

Redirect 関数内の以下の行が変更されました。

1. trailing := urlStr[len(urlStr)-1] == '/' この行は、リダイレクト先のURL urlStr が末尾にスラッシュを持っているかを判定していました。修正前は urlStr の最後の文字に直接アクセスしていましたが、urlStr が空文字列の場合にインデックスエラーが発生する可能性がありました。 これを trailing := strings.HasSuffix(urlStr, "/") に変更することで、strings.HasSuffix 関数が文字列の末尾が指定されたサフィックスで終わるかを安全にチェックするようになりました。空文字列に対してもパニックを起こさず false を返します。

2. if trailing && urlStr[len(urlStr)-1] != '/' { この条件は、path.Clean によってURLが正規化された後、もし元のURLに末尾スラッシュがあり、かつ正規化によってそのスラッシュが失われた場合に、再度スラッシュを追加するためのものでした。ここでも同様に、urlStr[len(urlStr)-1] の直接アクセスが問題となる可能性がありました。 これを if trailing && !strings.HasSuffix(urlStr, "/") { に変更することで、path.Clean 後の urlStr が空文字列になった場合でも安全に処理できるようになりました。

これらの変更により、http.Redirect 関数は、urlStr が空文字列であるような不正な入力に対してもパニックを起こさず、より堅牢に動作するようになりました。

src/pkg/net/http/server_test.go の変更

• パッケージ名の変更: package http から package http_test に変更されました。これは、テストコードがテスト対象のパッケージとは別のパッケージとしてコンパイルされる「外部テスト」の慣習に従うものです。これにより、テストコードはパッケージの公開されたAPIのみを使用するようになり、より現実的な使用シナリオをシミュレートできます。
• インポートの追加: . net/http と net/http/httptest がインポートされました。. net/http は、http パッケージの公開された識別子を修飾なしで直接使用できるようにするためのものです（例: Redirect ではなく Redirect）。httptest は、HTTPテスト用のユーティリティを提供します。
• TestServerRedirect の追加: この新しいテスト関数は、http.Redirect 関数が不正な入力（先頭スラッシュのないパス）を受け取った際にパニックを起こさないことを検証します。 httptest.NewRecorder() を使用して http.ResponseWriter のモックを作成し、http.Request を手動で構築しています。特に req.URL.Path に "not-empty-but-no-leading-slash" という「不正だが空ではない」パスを設定している点が重要です。 Redirect(rr, req, "", 304) を呼び出し、リダイレクト先のURLを空文字列に、ステータスコードを304に指定しています。 最後に、rr.Code が期待される304であることを確認し、パニックが発生しないことを間接的に検証しています。このテストは、修正されたバグの具体的なシナリオをカバーしています。
• codeSaver の削除と httptest.NewRecorder への移行: 以前のテストでは、codeSaver というカスタムの http.ResponseWriter 実装を使用して、WriteHeader に渡されたステータスコードを保存していました。 このコミットでは、TestServeMuxHandler 関数内で codeSaver の使用を httptest.NewRecorder() に置き換えています。httptest.NewRecorder() は、http.ResponseWriter インターフェースを実装しており、レスポンスのヘッダー、ボディ、ステータスコードを記録する機能を提供します。これにより、テストコードがより簡潔になり、Goの標準的なテストユーティリティを活用できるようになりました。

これらの変更は、net/http パッケージの堅牢性を高め、テストカバレッジを向上させる上で重要な役割を果たしています。

関連リンク

• Go言語の net/http パッケージのドキュメント: https://pkg.go.dev/net/http
• Go言語の strings パッケージのドキュメント: https://pkg.go.dev/strings
• Go言語の path パッケージのドキュメント: https://pkg.go.dev/path
• Go言語の httptest パッケージのドキュメント: https://pkg.go.dev/net/http/httptest

参考にした情報源リンク

• Go言語の公式ドキュメント
• Go言語のソースコード (特に net/http パッケージ)
• Go言語のコミット履歴
• Go言語のコードレビューシステム (Gerrit) のCL (Change-list) 8721045: https://golang.org/cl/8721045 (コミットメッセージに記載されているリンク)
• 一般的なHTTPリダイレクトに関する情報 (MDN Web Docsなど)
• Go言語におけるパニックとエラーハンドリングに関する情報