diff --git a/alpn_test.go b/alpn_test.go index 0c4f7a1f..01f82756 100644 --- a/alpn_test.go +++ b/alpn_test.go @@ -15,7 +15,7 @@ import ( "testing" . "github.com/ooni/oohttp" - "github.com/ooni/oohttp/httptest" + httptest "github.com/ooni/oohttp/httptest" ) func TestNextProtoUpgrade(t *testing.T) { diff --git a/cgi/child.go b/cgi/child.go index 98211b7f..b7ec6c76 100644 --- a/cgi/child.go +++ b/cgi/child.go @@ -19,7 +19,7 @@ import ( "strconv" "strings" - "github.com/ooni/oohttp" + http "github.com/ooni/oohttp" ) // Request returns the HTTP request as represented in the current diff --git a/cgi/host.go b/cgi/host.go index 7a51bf81..bf61f10f 100644 --- a/cgi/host.go +++ b/cgi/host.go @@ -29,7 +29,7 @@ import ( "strconv" "strings" - "github.com/ooni/oohttp" + http "github.com/ooni/oohttp" "golang.org/x/net/http/httpguts" ) diff --git a/cgi/host_test.go b/cgi/host_test.go index 1fa6ab22..da72136c 100644 --- a/cgi/host_test.go +++ b/cgi/host_test.go @@ -21,8 +21,8 @@ import ( "testing" "time" - "github.com/ooni/oohttp" - "github.com/ooni/oohttp/httptest" + http "github.com/ooni/oohttp" + httptest "github.com/ooni/oohttp/httptest" ) func newRequest(httpreq string) *http.Request { diff --git a/cgi/integration_test.go b/cgi/integration_test.go index 2e6c2f86..39dc2217 100644 --- a/cgi/integration_test.go +++ b/cgi/integration_test.go @@ -9,17 +9,60 @@ package cgi import ( + "bytes" "errors" "fmt" "io" + "net/url" "os" + "strings" "testing" "time" - "github.com/ooni/oohttp" - "github.com/ooni/oohttp/httptest" + http "github.com/ooni/oohttp" + httptest "github.com/ooni/oohttp/httptest" + testenv "github.com/ooni/oohttp/internal/testenv" ) +// This test is a CGI host (testing host.go) that runs its own binary +// as a child process testing the other half of CGI (child.go). +func TestHostingOurselves(t *testing.T) { + testenv.MustHaveExec(t) + + h := &Handler{ + Path: os.Args[0], + Root: "/test.go", + Args: []string{"-test.run=TestBeChildCGIProcess"}, + } + expectedMap := map[string]string{ + "test": "Hello CGI-in-CGI", + "param-a": "b", + "param-foo": "bar", + "env-GATEWAY_INTERFACE": "CGI/1.1", + "env-HTTP_HOST": "example.com", + "env-PATH_INFO": "", + "env-QUERY_STRING": "foo=bar&a=b", + "env-REMOTE_ADDR": "1.2.3.4", + "env-REMOTE_HOST": "1.2.3.4", + "env-REMOTE_PORT": "1234", + "env-REQUEST_METHOD": "GET", + "env-REQUEST_URI": "/test.go?foo=bar&a=b", + "env-SCRIPT_FILENAME": os.Args[0], + "env-SCRIPT_NAME": "/test.go", + "env-SERVER_NAME": "example.com", + "env-SERVER_PORT": "80", + "env-SERVER_SOFTWARE": "go", + } + replay := runCgiTest(t, h, "GET /test.go?foo=bar&a=b HTTP/1.0\nHost: example.com\n\n", expectedMap) + + if expected, got := "text/plain; charset=utf-8", replay.Header().Get("Content-Type"); got != expected { + t.Errorf("got a Content-Type of %q; expected %q", got, expected) + } + if expected, got := "X-Test-Value", replay.Header().Get("X-Test-Header"); got != expected { + t.Errorf("got a X-Test-Header of %q; expected %q", got, expected) + } +} + type customWriterRecorder struct { w io.Writer *httptest.ResponseRecorder @@ -48,6 +91,109 @@ func (w *limitWriter) Write(p []byte) (n int, err error) { return } +// If there's an error copying the child's output to the parent, test +// that we kill the child. +func TestKillChildAfterCopyError(t *testing.T) { + testenv.MustHaveExec(t) + + h := &Handler{ + Path: os.Args[0], + Root: "/test.go", + Args: []string{"-test.run=TestBeChildCGIProcess"}, + } + req, _ := http.NewRequest("GET", "http://example.com/test.cgi?write-forever=1", nil) + rec := httptest.NewRecorder() + var out bytes.Buffer + const writeLen = 50 << 10 + rw := &customWriterRecorder{&limitWriter{&out, writeLen}, rec} + + h.ServeHTTP(rw, req) + if out.Len() != writeLen || out.Bytes()[0] != 'a' { + t.Errorf("unexpected output: %q", out.Bytes()) + } +} + +// Test that a child handler writing only headers works. +// golang.org/issue/7196 +func TestChildOnlyHeaders(t *testing.T) { + testenv.MustHaveExec(t) + + h := &Handler{ + Path: os.Args[0], + Root: "/test.go", + Args: []string{"-test.run=TestBeChildCGIProcess"}, + } + expectedMap := map[string]string{ + "_body": "", + } + replay := runCgiTest(t, h, "GET /test.go?no-body=1 HTTP/1.0\nHost: example.com\n\n", expectedMap) + if expected, got := "X-Test-Value", replay.Header().Get("X-Test-Header"); got != expected { + t.Errorf("got a X-Test-Header of %q; expected %q", got, expected) + } +} + +// Test that a child handler does not receive a nil Request Body. +// golang.org/issue/39190 +func TestNilRequestBody(t *testing.T) { + testenv.MustHaveExec(t) + + h := &Handler{ + Path: os.Args[0], + Root: "/test.go", + Args: []string{"-test.run=TestBeChildCGIProcess"}, + } + expectedMap := map[string]string{ + "nil-request-body": "false", + } + _ = runCgiTest(t, h, "POST /test.go?nil-request-body=1 HTTP/1.0\nHost: example.com\n\n", expectedMap) + _ = runCgiTest(t, h, "POST /test.go?nil-request-body=1 HTTP/1.0\nHost: example.com\nContent-Length: 0\n\n", expectedMap) +} + +func TestChildContentType(t *testing.T) { + testenv.MustHaveExec(t) + + h := &Handler{ + Path: os.Args[0], + Root: "/test.go", + Args: []string{"-test.run=TestBeChildCGIProcess"}, + } + var tests = []struct { + name string + body string + wantCT string + }{ + { + name: "no body", + wantCT: "text/plain; charset=utf-8", + }, + { + name: "html", + body: "test pageThis is a body", + wantCT: "text/html; charset=utf-8", + }, + { + name: "text", + body: strings.Repeat("gopher", 86), + wantCT: "text/plain; charset=utf-8", + }, + { + name: "jpg", + body: "\xFF\xD8\xFF" + strings.Repeat("B", 1024), + wantCT: "image/jpeg", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + expectedMap := map[string]string{"_body": tt.body} + req := fmt.Sprintf("GET /test.go?exact-body=%s HTTP/1.0\nHost: example.com\n\n", url.QueryEscape(tt.body)) + replay := runCgiTest(t, h, req, expectedMap) + if got := replay.Header().Get("Content-Type"); got != tt.wantCT { + t.Errorf("got a Content-Type of %q; expected it to start with %q", got, tt.wantCT) + } + }) + } +} + // golang.org/issue/7198 func Test500WithNoHeaders(t *testing.T) { want500Test(t, "/immediate-disconnect") } func Test500WithNoContentType(t *testing.T) { want500Test(t, "/no-content-type") } diff --git a/client_test.go b/client_test.go index 883c79ea..c95fa156 100644 --- a/client_test.go +++ b/client_test.go @@ -18,6 +18,7 @@ import ( "net" "net/url" "reflect" + "runtime" "strconv" "strings" "sync" @@ -26,8 +27,9 @@ import ( "time" . "github.com/ooni/oohttp" - "github.com/ooni/oohttp/cookiejar" - "github.com/ooni/oohttp/httptest" + cookiejar "github.com/ooni/oohttp/cookiejar" + httptest "github.com/ooni/oohttp/httptest" + testenv "github.com/ooni/oohttp/internal/testenv" ) var robotsTxtHandler = HandlerFunc(func(w ResponseWriter, r *Request) { @@ -1237,6 +1239,9 @@ func testClientTimeout(t *testing.T, mode testMode) { t.Logf("timeout before response received") continue } + if runtime.GOOS == "windows" && strings.HasPrefix(runtime.GOARCH, "arm") { + testenv.SkipFlaky(t, 43120) + } t.Fatal(err) } @@ -1260,6 +1265,9 @@ func testClientTimeout(t *testing.T, mode testMode) { t.Errorf("net.Error.Timeout = false; want true") } if got := ne.Error(); !strings.Contains(got, "(Client.Timeout") { + if runtime.GOOS == "windows" && strings.HasPrefix(runtime.GOARCH, "arm") { + testenv.SkipFlaky(t, 43120) + } t.Errorf("error string = %q; missing timeout substring", got) } @@ -1300,6 +1308,9 @@ func testClientTimeout_Headers(t *testing.T, mode testMode) { t.Error("net.Error.Timeout = false; want true") } if got := ne.Error(); !strings.Contains(got, "Client.Timeout exceeded") { + if runtime.GOOS == "windows" && strings.HasPrefix(runtime.GOARCH, "arm") { + testenv.SkipFlaky(t, 43120) + } t.Errorf("error string = %q; missing timeout substring", got) } } diff --git a/clientserver_test.go b/clientserver_test.go index 54a00a5c..3ddd7fb9 100644 --- a/clientserver_test.go +++ b/clientserver_test.go @@ -31,9 +31,9 @@ import ( "time" . "github.com/ooni/oohttp" - "github.com/ooni/oohttp/httptest" - "github.com/ooni/oohttp/httptrace" - "github.com/ooni/oohttp/httputil" + httptest "github.com/ooni/oohttp/httptest" + httptrace "github.com/ooni/oohttp/httptrace" + httputil "github.com/ooni/oohttp/httputil" ) type testMode string diff --git a/cookie.go b/cookie.go index ca4a8722..9472c824 100644 --- a/cookie.go +++ b/cookie.go @@ -14,7 +14,7 @@ import ( "strings" "time" - "github.com/ooni/oohttp/internal/ascii" + ascii "github.com/ooni/oohttp/internal/ascii" ) // A Cookie represents an HTTP cookie as sent in the Set-Cookie header of an diff --git a/cookiejar/example_test.go b/cookiejar/example_test.go index 7f2cf2bd..8438f132 100644 --- a/cookiejar/example_test.go +++ b/cookiejar/example_test.go @@ -9,9 +9,9 @@ import ( "log" "net/url" - "github.com/ooni/oohttp" - "github.com/ooni/oohttp/cookiejar" - "github.com/ooni/oohttp/httptest" + http "github.com/ooni/oohttp" + cookiejar "github.com/ooni/oohttp/cookiejar" + httptest "github.com/ooni/oohttp/httptest" ) func ExampleNew() { diff --git a/cookiejar/jar.go b/cookiejar/jar.go index cfee9704..320d7e2e 100644 --- a/cookiejar/jar.go +++ b/cookiejar/jar.go @@ -15,8 +15,8 @@ import ( "sync" "time" - "github.com/ooni/oohttp" - "github.com/ooni/oohttp/internal/ascii" + http "github.com/ooni/oohttp" + ascii "github.com/ooni/oohttp/internal/ascii" ) // PublicSuffixList provides the public suffix of a domain. For example: diff --git a/cookiejar/jar_test.go b/cookiejar/jar_test.go index 53598dc1..b26cb84e 100644 --- a/cookiejar/jar_test.go +++ b/cookiejar/jar_test.go @@ -12,7 +12,7 @@ import ( "testing" "time" - "github.com/ooni/oohttp" + http "github.com/ooni/oohttp" ) // tNow is the synthetic current time used as now during testing. diff --git a/cookiejar/punycode.go b/cookiejar/punycode.go index ee38bc01..ada6408a 100644 --- a/cookiejar/punycode.go +++ b/cookiejar/punycode.go @@ -11,7 +11,7 @@ import ( "strings" "unicode/utf8" - "github.com/ooni/oohttp/internal/ascii" + ascii "github.com/ooni/oohttp/internal/ascii" ) // These parameter values are specified in section 5. diff --git a/example_filesystem_test.go b/example_filesystem_test.go index 5ea9d14a..4cc9777c 100644 --- a/example_filesystem_test.go +++ b/example_filesystem_test.go @@ -9,7 +9,7 @@ import ( "log" "strings" - "github.com/ooni/oohttp" + http "github.com/ooni/oohttp" ) // containsDotFile reports whether name contains a path element starting with a period. diff --git a/example_handle_test.go b/example_handle_test.go index 5365176c..939f2abd 100644 --- a/example_handle_test.go +++ b/example_handle_test.go @@ -9,7 +9,7 @@ import ( "log" "sync" - "github.com/ooni/oohttp" + http "github.com/ooni/oohttp" ) type countHandler struct { diff --git a/example_test.go b/example_test.go index 67b1db0b..69d1a99c 100644 --- a/example_test.go +++ b/example_test.go @@ -12,7 +12,7 @@ import ( "os" "os/signal" - "github.com/ooni/oohttp" + http "github.com/ooni/oohttp" ) func ExampleHijacker() { diff --git a/fcgi/child.go b/fcgi/child.go index 4e514e10..1d6ef73f 100644 --- a/fcgi/child.go +++ b/fcgi/child.go @@ -16,8 +16,8 @@ import ( "strings" "time" - "github.com/ooni/oohttp" - "github.com/ooni/oohttp/cgi" + http "github.com/ooni/oohttp" + cgi "github.com/ooni/oohttp/cgi" ) // request holds the state for an in-progress request. As soon as it's complete, diff --git a/fcgi/fcgi_test.go b/fcgi/fcgi_test.go index 623deea5..20b825f1 100644 --- a/fcgi/fcgi_test.go +++ b/fcgi/fcgi_test.go @@ -12,7 +12,7 @@ import ( "testing" "time" - "github.com/ooni/oohttp" + http "github.com/ooni/oohttp" ) var sizeTests = []struct { diff --git a/fs.go b/fs.go index c21cef95..2aa9c801 100644 --- a/fs.go +++ b/fs.go @@ -23,7 +23,7 @@ import ( "strings" "time" - "github.com/ooni/oohttp/internal/safefilepath" + safefilepath "github.com/ooni/oohttp/internal/safefilepath" ) // A Dir implements FileSystem using the native file system restricted to a diff --git a/fs_test.go b/fs_test.go index baa0705d..4736a7d2 100644 --- a/fs_test.go +++ b/fs_test.go @@ -27,7 +27,7 @@ import ( "time" . "github.com/ooni/oohttp" - "github.com/ooni/oohttp/httptest" + httptest "github.com/ooni/oohttp/httptest" ) const ( diff --git a/h2_bundle.go b/h2_bundle.go index bf6b0421..1d9720f2 100644 --- a/h2_bundle.go +++ b/h2_bundle.go @@ -47,7 +47,7 @@ import ( "sync/atomic" "time" - "github.com/ooni/oohttp/httptrace" + httptrace "github.com/ooni/oohttp/httptrace" "golang.org/x/net/http/httpguts" "golang.org/x/net/http2/hpack" "golang.org/x/net/idna" diff --git a/header.go b/header.go index 5f453803..2ca391f2 100644 --- a/header.go +++ b/header.go @@ -12,8 +12,8 @@ import ( "sync" "time" - "github.com/ooni/oohttp/httptrace" - "github.com/ooni/oohttp/internal/ascii" + httptrace "github.com/ooni/oohttp/httptrace" + ascii "github.com/ooni/oohttp/internal/ascii" "golang.org/x/net/http/httpguts" ) diff --git a/header_test.go b/header_test.go index 0121dcfd..06c04152 100644 --- a/header_test.go +++ b/header_test.go @@ -7,9 +7,12 @@ package http import ( "bytes" "reflect" + "runtime" "strings" "testing" "time" + + fakerace "github.com/ooni/oohttp/internal/fakerace" ) var headerWriteTests = []struct { @@ -214,6 +217,26 @@ func BenchmarkHeaderWriteSubset(b *testing.B) { } } +func TestHeaderWriteSubsetAllocs(t *testing.T) { + t.Skip("test disabled in the github.com/ooni/oohttp fork") + if testing.Short() { + t.Skip("skipping alloc test in short mode") + } + if fakerace.Enabled { + t.Skip("skipping test under race detector") + } + if runtime.GOMAXPROCS(0) > 1 { + t.Skip("skipping; GOMAXPROCS>1") + } + n := testing.AllocsPerRun(100, func() { + buf.Reset() + testHeader.WriteSubset(&buf, nil) + }) + if n > 0 { + t.Errorf("allocs = %g; want 0", n) + } +} + // Issue 34878: test that every call to // cloneOrMakeHeader never returns a nil Header. func TestCloneOrMakeHeader(t *testing.T) { diff --git a/http_test.go b/http_test.go index 61dccee8..9f4b50e6 100644 --- a/http_test.go +++ b/http_test.go @@ -7,9 +7,17 @@ package http import ( + "bytes" + "io/fs" "net/url" + "os" + "os/exec" "reflect" + "regexp" + "strings" "testing" + + testenv "github.com/ooni/oohttp/internal/testenv" ) func TestForeachHeaderElement(t *testing.T) { @@ -41,6 +49,68 @@ func TestForeachHeaderElement(t *testing.T) { } } +// Test that cmd/go doesn't link in the HTTP server. +// +// This catches accidental dependencies between the HTTP transport and +// server code. +func TestCmdGoNoHTTPServer(t *testing.T) { + t.Skip("test disabled in the github.com/ooni/oohttp fork") + t.Parallel() + goBin := testenv.GoToolPath(t) + out, err := exec.Command(goBin, "tool", "nm", goBin).CombinedOutput() + if err != nil { + t.Fatalf("go tool nm: %v: %s", err, out) + } + wantSym := map[string]bool{ + // Verify these exist: (sanity checking this test) + "net/http.(*Client).do": true, + "net/http.(*Transport).RoundTrip": true, + + // Verify these don't exist: + "net/http.http2Server": false, + "net/http.(*Server).Serve": false, + "net/http.(*ServeMux).ServeHTTP": false, + "net/http.DefaultServeMux": false, + } + for sym, want := range wantSym { + got := bytes.Contains(out, []byte(sym)) + if !want && got { + t.Errorf("cmd/go unexpectedly links in HTTP server code; found symbol %q in cmd/go", sym) + } + if want && !got { + t.Errorf("expected to find symbol %q in cmd/go; not found", sym) + } + } +} + +// Tests that the nethttpomithttp2 build tag doesn't rot too much, +// even if there's not a regular builder on it. +func TestOmitHTTP2(t *testing.T) { + t.Skip("test disabled in the github.com/ooni/oohttp fork") + if testing.Short() { + t.Skip("skipping in short mode") + } + t.Parallel() + goTool := testenv.GoToolPath(t) + out, err := exec.Command(goTool, "test", "-short", "-tags=nethttpomithttp2", "net/http").CombinedOutput() + if err != nil { + t.Fatalf("go test -short failed: %v, %s", err, out) + } +} + +// Tests that the nethttpomithttp2 build tag at least type checks +// in short mode. +// The TestOmitHTTP2 test above actually runs tests (in long mode). +func TestOmitHTTP2Vet(t *testing.T) { + t.Skip("test disabled in the github.com/ooni/oohttp fork") + t.Parallel() + goTool := testenv.GoToolPath(t) + out, err := exec.Command(goTool, "vet", "-tags=nethttpomithttp2", "net/http").CombinedOutput() + if err != nil { + t.Fatalf("go vet failed: %v, %s", err, out) + } +} + var valuesCount int func BenchmarkCopyValues(b *testing.B) { @@ -81,3 +151,45 @@ var forbiddenStringsFunctions = map[string]bool{ "Fields": true, "TrimSpace": true, } + +// TestNoUnicodeStrings checks that nothing in net/http uses the Unicode-aware +// strings and bytes package functions. HTTP is mostly ASCII based, and doing +// Unicode-aware case folding or space stripping can introduce vulnerabilities. +func TestNoUnicodeStrings(t *testing.T) { + if !testenv.HasSrc() { + t.Skip("source code not available") + } + + re := regexp.MustCompile(`(strings|bytes).([A-Za-z]+)`) + if err := fs.WalkDir(os.DirFS("."), ".", func(path string, d fs.DirEntry, err error) error { + if err != nil { + t.Fatal(err) + } + + if path == "internal/ascii" { + return fs.SkipDir + } + if !strings.HasSuffix(path, ".go") || + strings.HasSuffix(path, "_test.go") || + path == "h2_bundle.go" || d.IsDir() { + return nil + } + + contents, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + for lineNum, line := range strings.Split(string(contents), "\n") { + for _, match := range re.FindAllStringSubmatch(line, -1) { + if !forbiddenStringsFunctions[match[2]] { + continue + } + t.Errorf("disallowed call to %s at %s:%d", match[0], path, lineNum+1) + } + } + + return nil + }); err != nil { + t.Fatal(err) + } +} diff --git a/httptest/example_test.go b/httptest/example_test.go index 8b4ed68b..ea5fb23c 100644 --- a/httptest/example_test.go +++ b/httptest/example_test.go @@ -9,8 +9,8 @@ import ( "io" "log" - "github.com/ooni/oohttp" - "github.com/ooni/oohttp/httptest" + http "github.com/ooni/oohttp" + httptest "github.com/ooni/oohttp/httptest" ) func ExampleResponseRecorder() { diff --git a/httptest/httptest.go b/httptest/httptest.go index a4459477..c5fe33ca 100644 --- a/httptest/httptest.go +++ b/httptest/httptest.go @@ -12,7 +12,7 @@ import ( "io" "strings" - "github.com/ooni/oohttp" + http "github.com/ooni/oohttp" ) // NewRequest returns a new incoming server Request, suitable diff --git a/httptest/httptest_test.go b/httptest/httptest_test.go index df856f3c..d24e23b9 100644 --- a/httptest/httptest_test.go +++ b/httptest/httptest_test.go @@ -12,7 +12,7 @@ import ( "strings" "testing" - "github.com/ooni/oohttp" + http "github.com/ooni/oohttp" ) func TestNewRequest(t *testing.T) { diff --git a/httptest/recorder.go b/httptest/recorder.go index 0dd79e9e..d8dbc6f2 100644 --- a/httptest/recorder.go +++ b/httptest/recorder.go @@ -12,7 +12,7 @@ import ( "strconv" "strings" - "github.com/ooni/oohttp" + http "github.com/ooni/oohttp" "golang.org/x/net/http/httpguts" ) diff --git a/httptest/recorder_test.go b/httptest/recorder_test.go index 89fb1a35..5239dba3 100644 --- a/httptest/recorder_test.go +++ b/httptest/recorder_test.go @@ -9,7 +9,7 @@ import ( "io" "testing" - "github.com/ooni/oohttp" + http "github.com/ooni/oohttp" ) func TestRecorder(t *testing.T) { diff --git a/httptest/server.go b/httptest/server.go index 71ed67f6..81893460 100644 --- a/httptest/server.go +++ b/httptest/server.go @@ -18,8 +18,8 @@ import ( "sync" "time" - "github.com/ooni/oohttp" - "github.com/ooni/oohttp/internal/testcert" + http "github.com/ooni/oohttp" + testcert "github.com/ooni/oohttp/internal/testcert" ) // A Server is an HTTP server listening on a system-chosen port on the diff --git a/httptrace/example_test.go b/httptrace/example_test.go index e6f9e000..217814b7 100644 --- a/httptrace/example_test.go +++ b/httptrace/example_test.go @@ -8,8 +8,8 @@ import ( "fmt" "log" - "github.com/ooni/oohttp" - "github.com/ooni/oohttp/httptrace" + http "github.com/ooni/oohttp" + httptrace "github.com/ooni/oohttp/httptrace" ) func Example() { diff --git a/httputil/dump.go b/httputil/dump.go index 6b24fc05..11ebc5f5 100644 --- a/httputil/dump.go +++ b/httputil/dump.go @@ -15,7 +15,7 @@ import ( "strings" "time" - "github.com/ooni/oohttp" + http "github.com/ooni/oohttp" ) // drainBody reads all of b to memory and then returns two equivalent diff --git a/httputil/dump_test.go b/httputil/dump_test.go index fad0855c..80ad2129 100644 --- a/httputil/dump_test.go +++ b/httputil/dump_test.go @@ -18,7 +18,7 @@ import ( "testing" "time" - "github.com/ooni/oohttp" + http "github.com/ooni/oohttp" ) type eofReader struct{} diff --git a/httputil/example_test.go b/httputil/example_test.go index b3c0ed99..16bf5082 100644 --- a/httputil/example_test.go +++ b/httputil/example_test.go @@ -11,9 +11,9 @@ import ( "net/url" "strings" - "github.com/ooni/oohttp" - "github.com/ooni/oohttp/httptest" - "github.com/ooni/oohttp/httputil" + http "github.com/ooni/oohttp" + httptest "github.com/ooni/oohttp/httptest" + httputil "github.com/ooni/oohttp/httputil" ) func ExampleDumpRequest() { diff --git a/httputil/httputil.go b/httputil/httputil.go index 100e9aee..d94aefa4 100644 --- a/httputil/httputil.go +++ b/httputil/httputil.go @@ -9,7 +9,7 @@ package httputil import ( "io" - "github.com/ooni/oohttp/internal" + internal "github.com/ooni/oohttp/internal" ) // NewChunkedReader returns a new chunkedReader that translates the data read from r diff --git a/httputil/persist.go b/httputil/persist.go index f9b8fea8..dc482285 100644 --- a/httputil/persist.go +++ b/httputil/persist.go @@ -12,7 +12,7 @@ import ( "net/textproto" "sync" - "github.com/ooni/oohttp" + http "github.com/ooni/oohttp" ) var ( diff --git a/httputil/reverseproxy.go b/httputil/reverseproxy.go index 7ba3e0b5..3e239f60 100644 --- a/httputil/reverseproxy.go +++ b/httputil/reverseproxy.go @@ -21,8 +21,8 @@ import ( "time" http "github.com/ooni/oohttp" - "github.com/ooni/oohttp/httptrace" - "github.com/ooni/oohttp/internal/ascii" + httptrace "github.com/ooni/oohttp/httptrace" + ascii "github.com/ooni/oohttp/internal/ascii" "golang.org/x/net/http/httpguts" ) diff --git a/httputil/reverseproxy_test.go b/httputil/reverseproxy_test.go index aeff7117..29e0e8c6 100644 --- a/httputil/reverseproxy_test.go +++ b/httputil/reverseproxy_test.go @@ -26,9 +26,9 @@ import ( "time" http "github.com/ooni/oohttp" - "github.com/ooni/oohttp/httptest" - "github.com/ooni/oohttp/httptrace" - "github.com/ooni/oohttp/internal/ascii" + httptest "github.com/ooni/oohttp/httptest" + httptrace "github.com/ooni/oohttp/httptrace" + ascii "github.com/ooni/oohttp/internal/ascii" ) const fakeHopHeader = "X-Fake-Hop-Header-For-Test" diff --git a/internal/fakerace/fakerace.go b/internal/fakerace/fakerace.go new file mode 100644 index 00000000..323a5f97 --- /dev/null +++ b/internal/fakerace/fakerace.go @@ -0,0 +1,4 @@ +// Package fakerace fakes out the internal/race package. +package fakerace + +const Enabled = false diff --git a/internal/testenv/testenv.go b/internal/testenv/testenv.go new file mode 100644 index 00000000..32df8d8a --- /dev/null +++ b/internal/testenv/testenv.go @@ -0,0 +1,30 @@ +// Package testenv emulates some testenv properties to reduce +// the amount of deleted line with respect to upstream. +package testenv + +import "testing" + +// MustHaveExec always skips the current test. +func MustHaveExec(t testing.TB) { + t.Skip("testenv.MustHaveExec is not enabled in this fork") +} + +// SkipFlay skips a flaky test. +func SkipFlaky(t testing.TB, issue int) { + t.Skip("testenv.SkipFlaky: skipping flaky test", issue) +} + +// HasSrc always returns false. +func HasSrc() bool { + return false +} + +// GoToolPath returns the Go tool path. +func GoToolPath(t testing.TB) string { + return "go" +} + +// Builder always returns the empty string. +func Builder() string { + return "" +} diff --git a/main_test.go b/main_test.go index c17c8649..fe4e05a9 100644 --- a/main_test.go +++ b/main_test.go @@ -15,7 +15,7 @@ import ( "testing" "time" - "github.com/ooni/oohttp" + http "github.com/ooni/oohttp" ) var quietLog = log.New(io.Discard, "", 0) diff --git a/request.go b/request.go index 8e5b1b5f..167e0163 100644 --- a/request.go +++ b/request.go @@ -24,8 +24,8 @@ import ( "strings" "sync" - "github.com/ooni/oohttp/httptrace" - "github.com/ooni/oohttp/internal/ascii" + httptrace "github.com/ooni/oohttp/httptrace" + ascii "github.com/ooni/oohttp/internal/ascii" "golang.org/x/net/http/httpguts" "golang.org/x/net/idna" ) diff --git a/serve_test.go b/serve_test.go index 50c7b15f..8dae1005 100644 --- a/serve_test.go +++ b/serve_test.go @@ -24,6 +24,7 @@ import ( "net/url" "os" "os/exec" + "path/filepath" "reflect" "regexp" "runtime" @@ -37,11 +38,12 @@ import ( "time" . "github.com/ooni/oohttp" - "github.com/ooni/oohttp/httptest" - "github.com/ooni/oohttp/httptrace" - "github.com/ooni/oohttp/httputil" - "github.com/ooni/oohttp/internal" - "github.com/ooni/oohttp/internal/testcert" + httptest "github.com/ooni/oohttp/httptest" + httptrace "github.com/ooni/oohttp/httptrace" + httputil "github.com/ooni/oohttp/httputil" + internal "github.com/ooni/oohttp/internal" + testcert "github.com/ooni/oohttp/internal/testcert" + testenv "github.com/ooni/oohttp/internal/testenv" ) type dummyAddr string @@ -1887,6 +1889,43 @@ func TestServerUnreadRequestBodyLittle(t *testing.T) { <-done } +// Over a ~256KB (maxPostHandlerReadBytes) threshold, the server +// should ignore client request bodies that a handler didn't read +// and close the connection. +func TestServerUnreadRequestBodyLarge(t *testing.T) { + t.Skip("test disabled in the github.com/ooni/oohttp fork") + setParallel(t) + if testing.Short() && testenv.Builder() == "" { + t.Log("skipping in short mode") + } + conn := new(testConn) + body := strings.Repeat("x", 1<<20) + conn.readBuf.Write([]byte(fmt.Sprintf( + "POST / HTTP/1.1\r\n"+ + "Host: test\r\n"+ + "Content-Length: %d\r\n"+ + "\r\n", len(body)))) + conn.readBuf.Write([]byte(body)) + conn.closec = make(chan bool, 1) + + ls := &oneConnListener{conn} + go Serve(ls, HandlerFunc(func(rw ResponseWriter, req *Request) { + if conn.readBuf.Len() < len(body)/2 { + t.Errorf("on request, read buffer length is %d; expected about 1MB", conn.readBuf.Len()) + } + rw.WriteHeader(200) + rw.(Flusher).Flush() + if conn.readBuf.Len() < len(body)/2 { + t.Errorf("post-WriteHeader, read buffer length is %d; expected about 1MB", conn.readBuf.Len()) + } + })) + <-conn.closec + + if res := conn.writeBuf.String(); !strings.Contains(res, "Connection: close") { + t.Errorf("Expected a Connection: close header; got response: %s", res) + } +} + type handlerBodyCloseTest struct { bodySize int bodyChunked bool @@ -1988,6 +2027,17 @@ var handlerBodyCloseTests = [...]handlerBodyCloseTest{ }, } +func TestHandlerBodyClose(t *testing.T) { + t.Skip("test disabled in the github.com/ooni/oohttp fork") + setParallel(t) + if testing.Short() && testenv.Builder() == "" { + t.Skip("skipping in -short mode") + } + for i, tt := range handlerBodyCloseTests { + testHandlerBodyClose(t, i, tt) + } +} + func testHandlerBodyClose(t *testing.T, i int, tt handlerBodyCloseTest) { conn := new(testConn) body := strings.Repeat("x", tt.bodySize) @@ -5814,6 +5864,10 @@ func TestServerDuplicateBackgroundRead(t *testing.T) { run(t, testServerDuplicateBackgroundRead, []testMode{http1Mode}) } func testServerDuplicateBackgroundRead(t *testing.T, mode testMode) { + if runtime.GOOS == "netbsd" && runtime.GOARCH == "arm" { + testenv.SkipFlaky(t, 24826) + } + goroutines := 5 requests := 2000 if testing.Short() { @@ -6306,6 +6360,10 @@ func testTimeoutHandlerSuperfluousLogs(t *testing.T, mode testMode) { t.Skip("skipping in short mode") } + pc, curFile, _, _ := runtime.Caller(0) + curFileBaseName := filepath.Base(curFile) + testFuncName := runtime.FuncForPC(pc).Name() + timeoutMsg := "timed out here!" tests := []struct { @@ -6382,11 +6440,14 @@ func testTimeoutHandlerSuperfluousLogs(t *testing.T, mode testMode) { t.Fatalf("Server logs count mismatch\ngot %d, want %d\n\nGot\n%s\n", g, w, blob) } - _ = <-lastLine + lastSpuriousLine := <-lastLine + firstSpuriousLine := lastSpuriousLine - 3 // Now ensure that the regexes match exactly. // "http: superfluous response.WriteHeader call from .func\d.\d (:lastSpuriousLine-[1, 3]" - for _, logEntry := range logEntries { - pat := `^http: superfluous response.WriteHeader call from github.com/ooni/oohttp.relevantCaller \(server.go:` + for i, logEntry := range logEntries { + wantLine := firstSpuriousLine + i + pat := fmt.Sprintf("^http: superfluous response.WriteHeader call from %s.func\\d+.\\d+ \\(%s:%d\\)$", + testFuncName, curFileBaseName, wantLine) re := regexp.MustCompile(pat) if !re.MatchString(logEntry) { t.Errorf("Log entry mismatch\n\t%s\ndoes not match\n\t%s", logEntry, pat) diff --git a/transfer.go b/transfer.go index cc7d98d0..036839ef 100644 --- a/transfer.go +++ b/transfer.go @@ -18,9 +18,9 @@ import ( "sync" "time" - "github.com/ooni/oohttp/httptrace" - "github.com/ooni/oohttp/internal" - "github.com/ooni/oohttp/internal/ascii" + httptrace "github.com/ooni/oohttp/httptrace" + internal "github.com/ooni/oohttp/internal" + ascii "github.com/ooni/oohttp/internal/ascii" "golang.org/x/net/http/httpguts" ) diff --git a/transport.go b/transport.go index b422a919..559f85cf 100644 --- a/transport.go +++ b/transport.go @@ -28,8 +28,8 @@ import ( "sync/atomic" "time" - "github.com/ooni/oohttp/httptrace" - "github.com/ooni/oohttp/internal/ascii" + httptrace "github.com/ooni/oohttp/httptrace" + ascii "github.com/ooni/oohttp/internal/ascii" "golang.org/x/net/http/httpguts" "golang.org/x/net/http/httpproxy" ) diff --git a/transport_internal_test.go b/transport_internal_test.go index 7fd7aef9..463aafca 100644 --- a/transport_internal_test.go +++ b/transport_internal_test.go @@ -16,7 +16,7 @@ import ( "strings" "testing" - "github.com/ooni/oohttp/internal/testcert" + testcert "github.com/ooni/oohttp/internal/testcert" ) // Issue 15446: incorrect wrapping of errors when server closes an idle connection. diff --git a/transport_test.go b/transport_test.go index a1f48e73..94082b0b 100644 --- a/transport_test.go +++ b/transport_test.go @@ -39,10 +39,10 @@ import ( "time" . "github.com/ooni/oohttp" - "github.com/ooni/oohttp/httptest" - "github.com/ooni/oohttp/httptrace" - "github.com/ooni/oohttp/httputil" - "github.com/ooni/oohttp/internal/testcert" + httptest "github.com/ooni/oohttp/httptest" + httptrace "github.com/ooni/oohttp/httptrace" + httputil "github.com/ooni/oohttp/httputil" + testcert "github.com/ooni/oohttp/internal/testcert" "golang.org/x/net/http/httpguts" ) diff --git a/triv.go b/triv.go index d5b562dd..fa6a249d 100644 --- a/triv.go +++ b/triv.go @@ -18,7 +18,7 @@ import ( "strings" "sync" - "github.com/ooni/oohttp" + http "github.com/ooni/oohttp" ) // hello world, the web server