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

このコミットは、Go言語の標準ライブラリである net/http パッケージにおけるリクエストのフォーム解析に関するテストの追加と修正を行っています。特に、Request オブジェクトの MultipartReader メソッドと ParseMultipartForm メソッドの呼び出し順序に関する挙動を検証するためのテストが強化されています。

コミット

commit 991e9a83314216040592e3f303d737cb2dc087ac
Author: Matthew Cottingham <mattcottingham@gmail.com>
Date:   Mon Jan 6 10:36:04 2014 -0800

    net/http: Add more call order tests for request form parsing.
    
    Adds tests for branches handling call ordering which
    were shown to be untested by the cover tool.
    
    This is part of the refactoring of form parsing discussed
    in CL 44040043. These tests may need to be changed later but
    should help lock in the current behaviour.
    
    R=golang-codereviews, dave, bradfitz
    CC=golang-codereviews
    https://golang.org/cl/46750043

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

https://github.com/golang/go/commit/991e9a83314216040592e3f303d737cb2dc087ac

元コミット内容

このコミットは、net/http パッケージにおいて、リクエストのフォーム解析、特にマルチパートフォームの処理におけるメソッド呼び出し順序に関するテストを追加するものです。既存のコードカバレッジツールによって、これらの呼び出し順序を扱うブランチがテストされていないことが示されたため、そのカバレッジを向上させる目的があります。

これは、以前の変更セット (CL 44040043) で議論されたフォーム解析のリファクタリングの一部として位置づけられています。追加されたテストは、現在の挙動を固定化するのに役立ちますが、将来のリファクタリングによって変更される可能性も示唆されています。

変更の背景

HTTPリクエストにおいて、クライアントから送信されるデータは様々な形式を取り得ます。特に、HTMLフォームからのデータ送信やファイルアップロードには、application/x-www-form-urlencoded や multipart/form-data といったMIMEタイプが使用されます。Goの net/http パッケージは、これらのリクエストボディを解析し、プログラムから簡単にアクセスできるようにする機能を提供しています。

Request オブジェクトには、フォームデータを解析するためのいくつかのメソッドがあります。例えば、ParseForm()、ParseMultipartForm()、MultipartReader() などです。これらのメソッドは、リクエストボディのストリームを読み取り、解析するため、一度読み取られたデータは再利用できないという特性があります。そのため、これらのメソッドが呼び出される順序は、予期せぬエラーやデータ喪失を引き起こす可能性があります。

このコミットの背景には、コードカバレッジツールの分析によって、特定の呼び出し順序のシナリオがテストされていないことが判明したという事実があります。特に、MultipartReader() と ParseMultipartForm() のような、リクエストボディの読み取りに競合する可能性のあるメソッドの組み合わせが対象でした。これらの未テストのブランチは、将来の変更によって意図しないバグが導入されるリスクを抱えていました。したがって、現在の挙動を明確にし、将来のリファクタリングの際の安全性を確保するために、これらのテストを追加する必要がありました。

前提知識の解説

HTTPリクエストのボディとMIMEタイプ

HTTPリクエストには、GETリクエストのようにURLパラメータでデータを渡す方法と、POSTリクエストのようにリクエストボディにデータを格納して渡す方法があります。リクエストボディにデータを格納する場合、そのデータの形式を示すために Content-Type ヘッダが使用されます。

• application/x-www-form-urlencoded: 最も一般的なフォームデータ形式。キーと値のペアが key1=value1&key2=value2 のようにエンコードされ、URLエンコードされます。
• multipart/form-data: ファイルアップロードなど、複数の異なる種類のデータを一つのリクエストで送信する場合に使用されます。各データパートは境界文字列 (boundary string) で区切られ、それぞれが独自の Content-Type ヘッダを持つことができます。

Go net/http パッケージの Request オブジェクト

Goの net/http パッケージは、HTTPクライアントとサーバーを実装するための基本的な機能を提供します。サーバー側では、受信したHTTPリクエストは http.Request 型のオブジェクトとして表現されます。この Request オブジェクトには、リクエストメソッド、URL、ヘッダ、そしてリクエストボディなどの情報が含まれています。

Request オブジェクトには、リクエストボディからフォームデータを解析するための便利なメソッドがいくつか用意されています。

• ParseForm(): application/x-www-form-urlencoded または multipart/form-data のいずれかの形式で送信されたフォームデータを解析し、Request.Form フィールドに格納します。このメソッドは、Request.PostForm も設定します。
• ParseMultipartForm(maxMemory int64): multipart/form-data 形式のリクエストボディを解析し、ファイルデータを含むフォームデータをメモリにロードします。maxMemory は、メモリに保持する最大バイト数を指定し、これを超える部分は一時ファイルに書き込まれます。解析されたデータは Request.MultipartForm フィールドに格納されます。
• MultipartReader(): multipart/form-data 形式のリクエストボディを直接読み取るための multipart.Reader を返します。これは、ParseMultipartForm のようにデータをメモリにロードするのではなく、ストリームとして処理したい場合に有用です。

ストリーム処理の特性と呼び出し順序の重要性

HTTPリクエストボディは、通常、ストリームとして扱われます。これは、データが一度にすべてメモリにロードされるのではなく、必要に応じて少しずつ読み込まれることを意味します。ParseForm()、ParseMultipartForm()、MultipartReader() といったメソッドは、このストリームからデータを読み取ります。

ストリーム処理の重要な特性として、「一度読み取られたデータは、通常、再度読み取ることができない」という点があります。例えば、MultipartReader() を呼び出してリクエストボディの読み取りを開始した場合、そのストリームは進んでしまいます。その後に ParseMultipartForm() を呼び出そうとしても、すでにデータが読み取られているため、期待通りに動作しないか、エラーを発生させる可能性があります。

このようなメソッド間の相互作用と呼び出し順序の依存性は、プログラムの堅牢性を確保するために非常に重要です。開発者がこれらのメソッドを誤った順序で呼び出した場合に、明確なエラーを返すか、少なくとも予期せぬ挙動をしないように設計されている必要があります。このコミットは、まさにこの「呼び出し順序」に起因する潜在的な問題を特定し、テストによってその挙動を保証しようとしています。

コードカバレッジツール

コードカバレッジツールは、テストがソースコードのどの部分を実行したかを測定するツールです。これにより、テストが不足しているコードパス（ブランチ、ステートメントなど）を特定できます。このコミットの背景で「cover toolによって未テストであることが示された」とあるのは、Goの標準的なカバレッジツール（go test -cover など）が、特定の呼び出し順序のシナリオがテストスイートによって実行されていないことを指摘したことを意味します。

技術的詳細

このコミットの技術的な核心は、net/http パッケージの Request オブジェクトにおけるフォーム解析関連メソッドの「状態管理」と「エラーハンドリング」のテストにあります。

Request オブジェクトは、リクエストボディの解析状態を内部的に管理しています。例えば、ParseMultipartForm や MultipartReader が呼び出されると、リクエストボディの読み取りが開始され、その状態が変更されます。もし、これらのメソッドが互いに排他的な操作である場合（つまり、一度どちらかが呼び出されたら、もう一方は呼び出せない、またはエラーを返す必要がある場合）、その排他性を保証するためのロジックと、それに伴う適切なエラーメッセージが重要になります。

コミットの変更点を見ると、request.go の parsePostForm 関数内のコメントが更新されています。これは、以前のコメントが TestRequestMultipartCallOrder という特定のテストケースに言及していたのに対し、より一般的な TestParseMultipartFormOrder and others という表現に変更されたことを示しています。これは、単一のテストケースだけでなく、複数のテストケースで呼び出し順序の検証が行われるようになったことを示唆しています。

request_test.go では、以下の新しいテストケースが追加または修正されています。

1. TestParseMultipartFormOrder:

   • このテストは、まず req.MultipartReader() を呼び出し、その後に req.ParseMultipartForm() を呼び出した場合にエラーが発生することを検証します。
   • MultipartReader() が呼び出されると、リクエストボディのストリームが multipart.Reader によって消費され始めます。この状態で ParseMultipartForm() を呼び出すと、期待されるデータがストリームから読み取れないため、エラーとなるべきです。このテストは、このエラー挙動を保証します。

2. TestMultipartReaderOrder:

   • このテストは、まず req.ParseMultipartForm() を呼び出し、その後に req.MultipartReader() を呼び出した場合にエラーが発生することを検証します。
   • ParseMultipartForm() が呼び出されると、リクエストボディ全体（または指定されたメモリ制限まで）が解析され、内部的に消費されます。この状態で MultipartReader() を呼び出しても、読み取るべきストリームがすでに消費されているため、エラーとなるべきです。このテストは、このエラー挙動を保証します。

3. TestFormFileOrder:

   • このテストは、まず req.MultipartReader() を呼び出し、その後に req.FormFile("") を呼び出した場合にエラーが発生することを検証します。
   • FormFile メソッドは、ParseMultipartForm が呼び出された後に、解析されたマルチパートフォームデータから特定のファイルを取得するために使用されます。しかし、MultipartReader() が呼び出された場合、ParseMultipartForm は呼び出されていないため、Request.MultipartForm フィールドは設定されていません。この状態で FormFile を呼び出すと、適切なエラー（例えば、フォームが解析されていないことによるエラー）が発生するべきです。このテストは、この挙動を保証します。

これらのテストは、net/http パッケージの Request オブジェクトが、フォーム解析メソッドの呼び出し順序に関して、明確で予測可能なエラーセマンティクスを持つことを保証します。これにより、開発者がこれらのメソッドを誤って使用した場合に、早期に問題を検出できるようになります。

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

このコミットによるコードの変更は、主に2つのファイルにわたります。

1. src/pkg/net/http/request.go

   • parsePostForm 関数内のコメントが修正されています。

2. src/pkg/net/http/request_test.go

   • 既存のテスト関数 TestRequestMultipartCallOrder が削除され、より詳細な新しいテスト関数 TestParseMultipartFormOrder、TestMultipartReaderOrder、TestFormFileOrder が追加されています。

--- a/src/pkg/net/http/request.go
+++ b/src/pkg/net/http/request.go
@@ -708,7 +708,7 @@ func parsePostForm(r *Request) (vs url.Values, err error) {
 		// orders to call too many functions here.
 		// Clean this up and write more tests.
 		// request_test.go contains the start of this,
-		// in TestRequestMultipartCallOrder.
+		// in TestParseMultipartFormOrder and others.
 	}
 	return
 }
--- a/src/pkg/net/http/request_test.go
+++ b/src/pkg/net/http/request_test.go
@@ -199,15 +199,39 @@ func TestEmptyMultipartRequest(t *testing.T) {
 	testMissingFile(t, req)
 }
 
-func TestRequestMultipartCallOrder(t *testing.T) {
+// Test that ParseMultipartForm errors if called
+// after MultipartReader on the same request.
+func TestParseMultipartFormOrder(t *testing.T) {
 	req := newTestMultipartRequest(t)
-	_, err := req.MultipartReader()
-	if err != nil {
+	if _, err := req.MultipartReader(); err != nil {
 		t.Fatalf("MultipartReader: %v", err)
 	}
-	err = req.ParseMultipartForm(1024)
-	if err == nil {
-		t.Errorf("expected an error from ParseMultipartForm after call to MultipartReader")
+	if err := req.ParseMultipartForm(1024); err == nil {
+		t.Fatal("expected an error from ParseMultipartForm after call to MultipartReader")
+	}
+}
+
+// Test that MultipartReader errors if called
+// after ParseMultipartForm on the same request.
+func TestMultipartReaderOrder(t *testing.T) {
+	req := newTestMultipartRequest(t)
+	if err := req.ParseMultipartForm(25); err != nil {
+		t.Fatalf("ParseMultipartForm: %v", err)
+	}
+	if _, err := req.MultipartReader(); err == nil {
+		t.Fatal("expected an error from MultipartReader after call to ParseMultipartForm")
+	}
+}
+
+// Test that FormFile errors if called after
+// MultipartReader on the same request.
+func TestFormFileOrder(t *testing.T) {
+	req := newTestMultipartRequest(t)
+	if _, err := req.MultipartReader(); err != nil {
+		t.Fatalf("MultipartReader: %v", err)
+	}
+	if _, _, err := req.FormFile(""); err == nil {
+		t.Fatal("expected an error from FormFile after call to MultipartReader")
 	}
 }
 

コアとなるコードの解説

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

parsePostForm 関数内のコメントの変更は、コードの機能的な変更ではなく、ドキュメンテーションの改善です。以前は特定のテストケース TestRequestMultipartCallOrder に言及していましたが、新しいテストが追加されたことで、より一般的な表現 TestParseMultipartFormOrder and others に変更されました。これは、フォーム解析の呼び出し順序に関するテストが複数存在し、より包括的になったことを示唆しています。

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

このファイルでは、既存の TestRequestMultipartCallOrder テストが削除され、その代わりに以下の3つの新しいテストが追加されています。

1. TestParseMultipartFormOrder:

   • このテストは、req.MultipartReader() を呼び出してリクエストボディの読み取りを開始した後、req.ParseMultipartForm(1024) を呼び出すシナリオをテストします。
   • MultipartReader() が呼び出されると、リクエストボディのストリームが消費され始めます。この状態で ParseMultipartForm() を呼び出すと、すでにデータが読み取られているため、ParseMultipartForm はエラーを返すことが期待されます。テストは if err := req.ParseMultipartForm(1024); err == nil でエラーが返されない場合に t.Fatal を呼び出し、テストを失敗させます。

2. TestMultipartReaderOrder:

   • このテストは、req.ParseMultipartForm(25) を呼び出してリクエストボディを解析した後、req.MultipartReader() を呼び出すシナリオをテストします。
   • ParseMultipartForm() が呼び出されると、リクエストボディ全体が解析され、内部的に消費されます。この状態で MultipartReader() を呼び出すと、読み取るべきストリームがすでに消費されているため、MultipartReader はエラーを返すことが期待されます。テストは if _, err := req.MultipartReader(); err == nil でエラーが返されない場合に t.Fatal を呼び出し、テストを失敗させます。

3. TestFormFileOrder:

   • このテストは、req.MultipartReader() を呼び出してリクエストボディの読み取りを開始した後、req.FormFile("") を呼び出すシナリオをテストします。
   • FormFile メソッドは、ParseMultipartForm が呼び出されて Request.MultipartForm が設定された後に機能します。MultipartReader() が呼び出されただけでは Request.MultipartForm は設定されないため、FormFile はエラーを返すことが期待されます。テストは if _, _, err := req.FormFile(""); err == nil でエラーが返されない場合に t.Fatal を呼び出し、テストを失敗させます。

これらのテストは、net/http パッケージの Request オブジェクトが、フォーム解析関連のメソッドの呼び出し順序に関して、堅牢なエラーハンドリングと予測可能な挙動を提供することを保証します。これにより、開発者がこれらのメソッドを誤って使用した場合に、早期に問題を検出できるようになります。

関連リンク

• Go net/http パッケージのドキュメント: https://pkg.go.dev/net/http
• Go mime/multipart パッケージのドキュメント: https://pkg.go.dev/mime/multipart
• Go net/url パッケージのドキュメント: https://pkg.go.dev/net/url

参考にした情報源リンク

• Goのコミット履歴 (GitHub): https://github.com/golang/go/commits/master
• Goのコードレビューシステム (Gerrit): https://go.dev/cl/ (コミットメッセージに記載されている https://golang.org/cl/46750043 は、このGerritシステムへのリンクです。)
• Goのコードカバレッジツールに関する情報: https://go.dev/blog/cover