Keyboard shortcuts

Press or to navigate between chapters

Press S or / to search in the book

Press ? to show this help

Press Esc to hide this help

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

このコミットは、Go言語の標準ライブラリ net/http パッケージにおける Request.ParseForm() メソッドの挙動を改善するものです。具体的には、未知の Content-Type ヘッダを持つリクエストボディのパース時にエラーを返さず、そのタイプを無視するように変更されました。また、同時にGo言語特有の「シャドーイングされたエラー変数」のバグも修正されています。

コミット

commit 32d7a7364f10b652c36e1515623586d0db82ef20
Author: Roger Peppe <rogpeppe@gmail.com>
Date:   Thu Jan 26 16:50:56 2012 +0000

    net/http: make ParseForm ignore unknown content types.
    Also fix a shadowed error variable bug.
    
    R=golang-dev, rsc
    CC=golang-dev
    https://golang.org/cl/5573072

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

https://github.com/golang/go/commit/32d7a7364f10b652c36e1515623586d0db82ef20

元コミット内容

このコミットは、net/http パッケージの Request.ParseForm() 関数が、未知の Content-Type を持つリクエストボディを処理する際に、エラーを発生させるのではなく、その Content-Type を無視するように変更します。さらに、ParseForm() 内で発生していた、エラー変数がシャドーイング(隠蔽)されてしまうバグも修正しています。

変更の背景

未知のContent-Typeの扱い

net/http パッケージの Request.ParseForm() メソッドは、HTTPリクエストのボディからフォームデータをパースするために使用されます。これには通常、application/x-www-form-urlencodedmultipart/form-data といった Content-Type が想定されます。しかし、これら以外の未知の Content-Type が指定された場合、以前の ParseForm() はエラーを返していました。

この挙動は、柔軟性に欠けるという問題がありました。例えば、特定の Content-Type を処理するカスタムハンドラが後続で存在する場合、ParseForm() が先にエラーを返してしまうと、そのカスタムハンドラに処理が渡る機会が失われます。Webアプリケーションフレームワークなどでは、リクエストのパースを複数の段階で行うことがあり、ParseForm() が既知のタイプ以外でエラーを返さない方が、より柔軟な処理フローを構築できます。この変更により、ParseForm() は既知のフォームデータタイプのみを処理し、それ以外は単に無視するようになり、他のハンドラやミドルウェアが独自に処理する余地が生まれます。

シャドーイングされたエラー変数バグ

Go言語では、変数のスコープに関する重要な概念として「シャドーイング(Shadowing)」があります。これは、内側のスコープで外側のスコープと同じ名前の変数を再宣言すると、内側のスコープでは新しい変数が使われ、外側の変数が一時的に「隠される」現象を指します。

このコミットで修正されたバグは、ParseForm() 関数内で mime.ParseMediaType の呼び出し時に ct, _, err := mime.ParseMediaType(ct):= (ショート変数宣言) を使用していたことに起因します。ParseForm() 関数自体は func (r *Request) ParseForm() (err error) のように、戻り値として err という名前のエラー変数を既に宣言しています。しかし、mime.ParseMediaType の呼び出し箇所が if ブロック内にあったため、そこで err := ... とすると、if ブロックのローカルスコープで新しい err 変数が宣言され、関数の戻り値として宣言された err をシャドーイングしていました。

これにより、mime.ParseMediaType がエラーを返した場合でも、そのエラーはローカルスコープの err に格納され、関数の最終的な戻り値である外側の err には伝播されませんでした。結果として、mime.ParseMediaType で発生したエラーが呼び出し元に正しく報告されないという潜在的なバグが存在していました。

前提知識の解説

Go言語におけるエラーハンドリングとシャドーイング

Go言語では、エラーは多値戻り値の最後の値として返されるのが一般的です。例えば、value, err := someFunc() のように使用します。 変数の宣言には var キーワードを使用するか、:= (ショート変数宣言) を使用します。

  • var err errorerrerror 型として宣言します。
  • err = someFunc():既存の err 変数に値を代入します。
  • err := someFunc()err を宣言し、同時に値を代入します。もし同じスコープに err が既に存在する場合、これはコンパイルエラーになります。しかし、異なるスコープ(例:if ブロック内)で同じ名前の変数を := で宣言すると、それは新しいローカル変数を宣言することになり、外側のスコープの変数をシャドーイングします。これが今回のバグの原因でした。

HTTPのContent-Typeヘッダ

Content-Type ヘッダは、HTTPリクエストまたはレスポンスのボディに含まれるデータのメディアタイプ(MIMEタイプ)を示すために使用されます。これにより、受信側はボディのデータをどのように解釈すればよいかを判断できます。 一般的なフォームデータに関連する Content-Type には以下があります。

  • application/x-www-form-urlencoded: HTMLフォームのデフォルトのエンコーディング。キーと値のペアが & で区切られ、= で結合されます。特殊文字はURLエンコードされます。
  • multipart/form-data: ファイルアップロードなど、複数のパートからなるデータを送信する際に使用されます。各パートは独自の Content-TypeContent-Disposition を持ちます。

net/http パッケージの Request.ParseForm() メソッド

net/http パッケージはGo言語の標準ライブラリであり、HTTPクライアントとサーバーの実装を提供します。 Request.ParseForm() メソッドは、HTTPリクエストのURLクエリパラメータとボディ(application/x-www-form-urlencoded または multipart/form-data の場合)からフォームデータをパースし、Request.Form フィールドに格納します。このメソッドは冪等であり、複数回呼び出されても一度しかパース処理は行われません。

mime.ParseMediaType

mime パッケージはMIMEメディアタイプをパースするための機能を提供します。 mime.ParseMediaType(v string) 関数は、Content-Type ヘッダのようなメディアタイプ文字列をパースし、タイプ(例: text/plain)とパラメータ(例: charset=utf-8)のマップを返します。

技術的詳細

ParseForm の挙動変更

以前の Request.ParseForm() メソッドは、Content-Type ヘッダが application/x-www-form-urlencoded または multipart/form-data のいずれでもない場合に、&badStringError{"unknown Content-Type", ct} というエラーを返していました。

このコミットでは、src/pkg/net/http/request.goParseForm メソッド内の switch ステートメントから、default ケースが削除されました。これにより、未知の Content-Type が指定された場合でも、ParseForm() はエラーを返さずに処理を続行します。これは、ParseForm() が既知のフォームデータタイプのみを処理し、それ以外のタイプは他のハンドラに任せるという設計思想への変更を意味します。

シャドーイングバグの修正

src/pkg/net/http/request.go の以下の行が変更されました。

--- a/src/pkg/net/http/request.go
+++ b/src/pkg/net/http/request.go
@@ -606,7 +606,7 @@ func (r *Request) ParseForm() (err error) {
 			return errors.New("missing form body")
 		}
 		ct := r.Header.Get("Content-Type")
-		ct, _, err := mime.ParseMediaType(ct)
+		ct, _, err = mime.ParseMediaType(ct)
 		switch {
 		case ct == "application/x-www-form-urlencoded":
 			var reader io.Reader = r.Body

元のコードでは ct, _, err := mime.ParseMediaType(ct) となっており、if ブロックの内部で新しい err 変数を宣言していました。これにより、関数の戻り値として宣言されている err 変数がシャドーイングされ、mime.ParseMediaType がエラーを返しても、そのエラーが関数の呼び出し元に伝わらないという問題がありました。

修正後のコードでは ct, _, err = mime.ParseMediaType(ct) となり、:== に変更されています。これにより、既存の err 変数(関数の戻り値として宣言されているもの)に mime.ParseMediaType の結果が代入されるようになり、エラーが正しく伝播するようになりました。

テストコードの変更

src/pkg/net/http/request_test.go では、これらの変更を反映するためにテストが更新されました。

  • parseContentTypeTest 構造体に shouldError bool フィールドが追加され、各テストケースがエラーを期待するかどうかを明示的に指定できるようになりました。
  • parseContentTypeTests 配列のテストケースが更新され、text/plainapplication/unknown のような未知の Content-Type に対しては shouldError: false が設定されました。これは、これらのタイプがもはやエラーを発生させないという新しい挙動を反映しています。
  • テスト関数 TestParseFormBadContentTypeTestParseFormUnknownContentType にリネームされ、テストロジックも switch ステートメントを使用して、shouldError フラグに基づいてエラーの有無を適切に検証するように変更されました。

これらのテストの変更は、ParseForm() の新しい挙動(未知の Content-Type を無視する)と、シャドーイングバグ修正後のエラー伝播の正確性を保証するために不可欠です。

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

src/pkg/net/http/request.go

--- a/src/pkg/net/http/request.go
+++ b/src/pkg/net/http/request.go
@@ -606,7 +606,7 @@ func (r *Request) ParseForm() (err error) {
 			return errors.New("missing form body")
 		}
 		ct := r.Header.Get("Content-Type")
-		ct, _, err := mime.ParseMediaType(ct)
+		ct, _, err = mime.ParseMediaType(ct)
 		switch {
 		case ct == "application/x-www-form-urlencoded":
 			var reader io.Reader = r.Body
@@ -646,8 +646,6 @@ func (r *Request) ParseForm() (err error) {
 			// Clean this up and write more tests.
 			// request_test.go contains the start of this,
 			// in TestRequestMultipartCallOrder.
-\t\tdefault:\
-\t\t\treturn &badStringError{"unknown Content-Type", ct}\
 \t\t}\
 \t}\
 \treturn err

src/pkg/net/http/request_test.go

--- a/src/pkg/net/http/request_test.go
+++ b/src/pkg/net/http/request_test.go
@@ -46,19 +46,19 @@ func TestPostQuery(t *testing.T) {
 
 type stringMap map[string][]string
 type parseContentTypeTest struct {
+\tshouldError bool
 \tcontentType stringMap
 }
 
 var parseContentTypeTests = []parseContentTypeTest{
-\t{contentType: stringMap{"Content-Type": {"text/plain"}}},\
-\t{contentType: stringMap{}}, // Non-existent keys are not placed. The value nil is illegal.\
-\t{contentType: stringMap{"Content-Type": {"text/plain; boundary="}}},\
-\t{\
-\t\tcontentType: stringMap{"Content-Type": {"application/unknown"}},\
-\t},\
+\t{false, stringMap{"Content-Type": {"text/plain"}}},\
+\t// Non-existent keys are not placed. The value nil is illegal.\
+\t{true, stringMap{}},\
+\t{true, stringMap{"Content-Type": {"text/plain; boundary="}}},\
+\t{false, stringMap{"Content-Type": {"application/unknown"}}},\
 }\
 
-func TestParseFormBadContentType(t *testing.T) {
+func TestParseFormUnknownContentType(t *testing.T) {
 \tfor i, test := range parseContentTypeTests {
 \t\treq := &Request{
 \t\t\tMethod: "POST",
@@ -66,8 +66,11 @@ func TestParseFormBadContentType(t *testing.T) {
 \t\t\tBody:   ioutil.NopCloser(bytes.NewBufferString("body")),\
 \t\t}\
 \t\terr := req.ParseForm()\
-\t\tif err == nil {\
+\t\tswitch {\
+\t\tcase err == nil && test.shouldError:\
 \t\t\tt.Errorf("test %d should have returned error", i)\
+\t\tcase err != nil && !test.shouldError:\
+\t\t\tt.Errorf("test %d should not have returned error, got %v", i, err)\
 \t\t}\
 \t}\
 }\

コアとなるコードの解説

src/pkg/net/http/request.go の変更点

  1. シャドーイングバグの修正:

    -		ct, _, err := mime.ParseMediaType(ct)
    +		ct, _, err = mime.ParseMediaType(ct)
    

    この変更は、ParseForm 関数の戻り値として既に宣言されている err 変数を、mime.ParseMediaType の結果で更新するようにします。元のコードでは := を使用していたため、if ブロックのローカルスコープで新しい err 変数が宣言され、外側の err をシャドーイングしていました。これにより、mime.ParseMediaType がエラーを返しても、そのエラーが関数の呼び出し元に正しく伝播しないというバグがありました。= に変更することで、既存の err 変数に代入が行われ、エラーが正しく伝播するようになります。

  2. 未知のContent-Typeを無視する挙動への変更:

    -		default:
    -			return &badStringError{"unknown Content-Type", ct}
    

    switch ステートメント内の default ケースが削除されました。以前は、Content-Typeapplication/x-www-form-urlencoded または multipart/form-data のいずれでもない場合に、この default ケースが実行され、"unknown Content-Type" エラーを返していました。この行が削除されたことで、ParseForm() は未知の Content-Type を持つリクエストボディに対してはエラーを返さず、単にそのタイプを無視するようになります。これにより、より柔軟なリクエスト処理が可能になります。

src/pkg/net/http/request_test.go の変更点

  1. テスト構造体の変更:

    type parseContentTypeTest struct {
    +	shouldError bool
     	contentType stringMap
     }
    

    parseContentTypeTest 構造体に shouldError フィールドが追加されました。これは、各テストケースが ParseForm() からエラーが返されることを期待するかどうかを示すブール値です。これにより、テストの意図がより明確になり、新しい挙動(未知のタイプでエラーを返さない)を正確にテストできるようになります。

  2. テストケースの更新:

    var parseContentTypeTests = []parseContentTypeTest{
    -	{contentType: stringMap{"Content-Type": {"text/plain"}}},\
    -	{contentType: stringMap{}}, // Non-existent keys are not placed. The value nil is illegal.\
    -	{contentType: stringMap{"Content-Type": {"text/plain; boundary="}}},\
    -	{\
    -		contentType: stringMap{"Content-Type": {"application/unknown"}},\
    -	},\
    +	{false, stringMap{"Content-Type": {"text/plain"}}},\
    +	// Non-existent keys are not placed. The value nil is illegal.\
    +	{true, stringMap{}},\
    +	{true, stringMap{"Content-Type": {"text/plain; boundary="}}},\
    +	{false, stringMap{"Content-Type": {"application/unknown"}}},\
     }
    

    parseContentTypeTests 配列の各テストケースが shouldError フィールドを含むように更新されました。

    • text/plainapplication/unknown のような未知の Content-Type は、ParseForm() がエラーを返さなくなったため、shouldError: false となっています。
    • Content-Type が空の場合や、text/plain; boundary= のようにパース自体が不正な場合は、引き続きエラーを期待するため shouldError: true となっています。
  3. テスト関数のリネームとロジックの変更:

    -func TestParseFormBadContentType(t *testing.T) {
    +func TestParseFormUnknownContentType(t *testing.T) {
     	for i, test := range parseContentTypeTests {
     		req := &Request{
     			Method: "POST",
     			Body:   ioutil.NopCloser(bytes.NewBufferString("body")),
     		}
     		err := req.ParseForm()
    -		if err == nil {
    +		switch {
    +		case err == nil && test.shouldError:
     			t.Errorf("test %d should have returned error", i)
    +		case err != nil && !test.shouldError:
    +			t.Errorf("test %d should not have returned error, got %v", i, err)
     		}
     	}
     }
    
    • テスト関数名が TestParseFormBadContentType から TestParseFormUnknownContentType に変更され、テストの目的がより正確に反映されました。
    • テストの検証ロジックが if err == nil から switch ステートメントに変更されました。これにより、test.shouldError の値に基づいて、エラーが返されるべきか、返されるべきではないかを正確にチェックできるようになりました。
      • err == nil && test.shouldError: エラーが返されるべきなのに返されなかった場合。
      • err != nil && !test.shouldError: エラーが返されるべきではないのに返された場合。 この変更により、ParseForm() の新しい挙動(未知の Content-Type を無視する)と、シャドーイングバグ修正後のエラー伝播の正確性が、より堅牢に検証されるようになりました。

関連リンク

参考にした情報源リンク