diff --git a/internal/testingx/oonibackendwithlogin.go b/internal/testingx/oonibackendwithlogin.go new file mode 100644 index 000000000..169e80c68 --- /dev/null +++ b/internal/testingx/oonibackendwithlogin.go @@ -0,0 +1,289 @@ +package testingx + +// +// Code for testing the OONI backend login flow. +// + +import ( + "errors" + "fmt" + "io" + "net/http" + "sync" + "time" + + "github.com/google/uuid" + "github.com/ooni/probe-cli/v3/internal/model" + "github.com/ooni/probe-cli/v3/internal/must" + "github.com/ooni/probe-cli/v3/internal/runtimex" +) + +// OONIBackendWithLoginFlowUserRecord is a user record used by [OONIBackendWithLoginFlow]. +type OONIBackendWithLoginFlowUserRecord struct { + Expire time.Time + Password string + Token string +} + +// OONIBackendWithLoginFlow is an [http.Handler] that implements the register and +// loging workflow and serves psiphon and tor config. +// +// The zero value is ready to use. +// +// This struct methods panics for several errors. Only use for testing purposes! +type OONIBackendWithLoginFlow struct { + // logins maps the existing login names to the corresponding record. + logins map[string]*OONIBackendWithLoginFlowUserRecord + + // mu provides mutual exclusion. + mu sync.Mutex + + // psiphonConfig is the serialized psiphon config to send to authenticated clients. + psiphonConfig []byte + + // tokens maps a token to a user record. + tokens map[string]*OONIBackendWithLoginFlowUserRecord + + // torTargets is the serialized tor config to send to authenticated clients. + torTargets []byte +} + +// SetPsiphonConfig sets psiphon configuration to use. +// +// This method is safe to call concurrently with incoming HTTP requests. +func (h *OONIBackendWithLoginFlow) SetPsiphonConfig(config []byte) { + defer h.mu.Unlock() + h.mu.Lock() + h.psiphonConfig = config +} + +// SetTorTargets sets tor targets to use. +// +// This method is safe to call concurrently with incoming HTTP requests. +func (h *OONIBackendWithLoginFlow) SetTorTargets(config []byte) { + defer h.mu.Unlock() + h.mu.Lock() + h.torTargets = config +} + +// DoWithLockedUserRecord performs an action with the given user record. The action will +// run while we're holding the [*OONIBackendWithLoginFlow] mutex. +func (h *OONIBackendWithLoginFlow) DoWithLockedUserRecord( + username string, fx func(rec *OONIBackendWithLoginFlowUserRecord) error) error { + defer h.mu.Unlock() + h.mu.Lock() + rec := h.logins[username] + if rec == nil { + return errors.New("no such record") + } + return fx(rec) +} + +// NewMux constructs an [*http.ServeMux] configured with the correct routing. +func (h *OONIBackendWithLoginFlow) NewMux() *http.ServeMux { + mux := http.NewServeMux() + mux.Handle("/api/v1/register", h.handleRegister()) + mux.Handle("/api/v1/login", h.handleLogin()) + mux.Handle("/api/v1/test-list/psiphon-config", h.withAuthentication(h.handlePsiphonConfig())) + mux.Handle("/api/v1/test-list/tor-targets", h.withAuthentication(h.handleTorTargets())) + return mux +} + +func (h *OONIBackendWithLoginFlow) handleRegister() http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // make sure the method is OK + if r.Method != http.MethodPost { + w.WriteHeader(501) + return + } + + // read the raw request body + rawreqbody := runtimex.Try1(io.ReadAll(r.Body)) + + // unmarshal the request + var request model.OOAPIRegisterRequest + must.UnmarshalJSON(rawreqbody, &request) + + // lock the users database + h.mu.Lock() + + // make sure the map is usable + if h.logins == nil { + h.logins = make(map[string]*OONIBackendWithLoginFlowUserRecord) + } + + // create new login + userID := uuid.Must(uuid.NewRandom()).String() + + // save login + h.logins[userID] = &OONIBackendWithLoginFlowUserRecord{ + Expire: time.Time{}, + Password: request.Password, + Token: "", + } + + // unlock the users database + h.mu.Unlock() + + // prepare response + response := &model.OOAPIRegisterResponse{ + ClientID: userID, + } + + // send response + w.Write(must.MarshalJSON(response)) + }) +} + +func (h *OONIBackendWithLoginFlow) handleLogin() http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // make sure the method is OK + if r.Method != http.MethodPost { + w.WriteHeader(501) + return + } + + // read the raw request body + rawreqbody := runtimex.Try1(io.ReadAll(r.Body)) + + // unmarshal the request + var request model.OOAPILoginCredentials + must.UnmarshalJSON(rawreqbody, &request) + + // lock the users database + h.mu.Lock() + + // attempt to access user record + record := h.logins[request.Username] + + // handle the case where the user does not exist + if record == nil { + // unlock the users database + h.mu.Unlock() + + // return 401 + w.WriteHeader(http.StatusUnauthorized) + return + } + + // handle the case where the password is invalid + if request.Password != record.Password { + // unlock the users database + h.mu.Unlock() + + // return 401 + w.WriteHeader(http.StatusUnauthorized) + return + } + + // create token + token := uuid.Must(uuid.NewRandom()).String() + + // create expiry date + expirydate := time.Now().Add(10 * time.Minute) + + // update record + record.Token = token + record.Expire = expirydate + + // create the token bearer header + bearer := fmt.Sprintf("Bearer %s", token) + + // make sure the tokens map is okay + if h.tokens == nil { + h.tokens = make(map[string]*OONIBackendWithLoginFlowUserRecord) + } + + // update the tokens map + h.tokens[bearer] = record + + // unlock the users database + h.mu.Unlock() + + // prepare response + response := &model.OOAPILoginAuth{ + Expire: expirydate, + Token: token, + } + + // send response + w.Write(must.MarshalJSON(response)) + }) +} + +func (h *OONIBackendWithLoginFlow) handlePsiphonConfig() http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // make sure the method is OK + if r.Method != http.MethodGet { + w.WriteHeader(501) + return + } + + // we must lock because of SetPsiphonConfig + h.mu.Lock() + w.Write(h.psiphonConfig) + h.mu.Unlock() + }) +} + +func (h *OONIBackendWithLoginFlow) handleTorTargets() http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // make sure the method is OK + if r.Method != http.MethodGet { + w.WriteHeader(501) + return + } + + // make sure the client has provided the right query string + cc := r.URL.Query().Get("country_code") + if cc == "" { + w.WriteHeader(http.StatusBadRequest) + return + } + + // we must lock because of SetTorTargets + h.mu.Lock() + w.Write(h.torTargets) + h.mu.Unlock() + }) + +} + +func (h *OONIBackendWithLoginFlow) withAuthentication(child http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // get the authorization header + authorization := r.Header.Get("Authorization") + + // lock the users database + h.mu.Lock() + + // check whether we have state + record := h.tokens[authorization] + + // handle the case of nonexisting state + if record == nil { + // unlock the users database + h.mu.Unlock() + + // return 401 + w.WriteHeader(http.StatusUnauthorized) + return + } + + // handle the case of expired state + if time.Until(record.Expire) <= 0 { + // unlock the users database + h.mu.Unlock() + + // return 401 + w.WriteHeader(http.StatusUnauthorized) + return + } + + // unlock the users database + h.mu.Unlock() + + // defer to the child handler + child.ServeHTTP(w, r) + }) +} diff --git a/internal/testingx/oonibackendwithlogin_test.go b/internal/testingx/oonibackendwithlogin_test.go new file mode 100644 index 000000000..7648f0177 --- /dev/null +++ b/internal/testingx/oonibackendwithlogin_test.go @@ -0,0 +1,501 @@ +package testingx + +import ( + "bytes" + "fmt" + "io" + "net/http" + "testing" + "time" + + "github.com/google/go-cmp/cmp" + "github.com/google/uuid" + "github.com/ooni/probe-cli/v3/internal/model" + "github.com/ooni/probe-cli/v3/internal/must" + "github.com/ooni/probe-cli/v3/internal/runtimex" + "github.com/ooni/probe-cli/v3/internal/urlx" +) + +func TestOONIBackendWithLoginFlow(t *testing.T) { + // create state + state := &OONIBackendWithLoginFlow{} + + // create local testing server + server := MustNewHTTPServer(state.NewMux()) + defer server.Close() + + // create a fake filler + ff := &FakeFiller{} + + t.Run("it may be that there's no user record", func(t *testing.T) { + err := state.DoWithLockedUserRecord("foobar", func(rec *OONIBackendWithLoginFlowUserRecord) error { + panic("should not be called") + }) + if err == nil || err.Error() != "no such record" { + t.Fatal("unexpected error", err) + } + }) + + t.Run("attempt login with invalid method", func(t *testing.T) { + // create HTTP request + req := runtimex.Try1(http.NewRequest( + "GET", + runtimex.Try1(urlx.ResolveReference(server.URL, "/api/v1/login", "")), + nil, + )) + + // perform the round trip + resp, err := http.DefaultClient.Do(req) + + // we do not expect an error + if err != nil { + t.Fatal(err) + } + + // make sure we eventually close the body + defer resp.Body.Close() + + // we expect to see not implemented + if resp.StatusCode != http.StatusNotImplemented { + t.Fatal("unexpected status code", resp.StatusCode) + } + }) + + t.Run("attempt login with invalid credentials", func(t *testing.T) { + // create fake login request + request := &model.OOAPILoginCredentials{} + + // fill it with random data + ff.Fill(&request) + + // create HTTP request + req := runtimex.Try1(http.NewRequest( + "POST", + runtimex.Try1(urlx.ResolveReference(server.URL, "/api/v1/login", "")), + bytes.NewReader(must.MarshalJSON(request)), + )) + + // perform the round trip + resp, err := http.DefaultClient.Do(req) + + // we do not expect an error + if err != nil { + t.Fatal(err) + } + + // make sure we eventually close the body + defer resp.Body.Close() + + // we expect to be unauthorized + if resp.StatusCode != http.StatusUnauthorized { + t.Fatal("unexpected status code", resp.StatusCode) + } + }) + + t.Run("attempt register with invalid method", func(t *testing.T) { + // create HTTP request + req := runtimex.Try1(http.NewRequest( + "GET", + runtimex.Try1(urlx.ResolveReference(server.URL, "/api/v1/register", "")), + nil, + )) + + // perform the round trip + resp, err := http.DefaultClient.Do(req) + + // we do not expect an error + if err != nil { + t.Fatal(err) + } + + // make sure we eventually close the body + defer resp.Body.Close() + + // we expect to see not implemented + if resp.StatusCode != http.StatusNotImplemented { + t.Fatal("unexpected status code", resp.StatusCode) + } + }) + + // registerflow attempts to register and returns the username and password + registerflow := func(t *testing.T) (string, string) { + // create register request + // + // we ignore the metadata because we're testing + request := &model.OOAPIRegisterRequest{ + OOAPIProbeMetadata: model.OOAPIProbeMetadata{}, + Password: uuid.Must(uuid.NewRandom()).String(), + } + + // create HTTP request + req := runtimex.Try1(http.NewRequest( + "POST", + runtimex.Try1(urlx.ResolveReference(server.URL, "/api/v1/register", "")), + bytes.NewReader(must.MarshalJSON(request)), + )) + + // perform the round trip + resp, err := http.DefaultClient.Do(req) + + // we do not expect an error + if err != nil { + t.Fatal(err) + } + + // make sure we eventually close the body + defer resp.Body.Close() + + // we expect to be authorized + if resp.StatusCode != http.StatusOK { + t.Fatal("unexpected status code", resp.StatusCode) + } + + // read response body + rawrespbody := runtimex.Try1(io.ReadAll(resp.Body)) + + // parse the response body + var response model.OOAPIRegisterResponse + must.UnmarshalJSON(rawrespbody, &response) + + // return username and password + return response.ClientID, request.Password + } + + t.Run("successful register", func(t *testing.T) { + _, _ = registerflow(t) + }) + + loginrequest := func(username, password string) *http.Response { + // create login request + request := &model.OOAPILoginCredentials{ + Username: username, + Password: password, + } + + // create HTTP request + req := runtimex.Try1(http.NewRequest( + "POST", + runtimex.Try1(urlx.ResolveReference(server.URL, "/api/v1/login", "")), + bytes.NewReader(must.MarshalJSON(request)), + )) + + // perform the round trip + resp, err := http.DefaultClient.Do(req) + + // we do not expect an error + if err != nil { + t.Fatal(err) + } + + return resp + } + + loginflow := func(username, password string) (string, time.Time) { + // get the response + resp := loginrequest(username, password) + + // make sure we eventually close the body + defer resp.Body.Close() + + // we expect to be authorized + if resp.StatusCode != http.StatusOK { + t.Fatal("unexpected status code", resp.StatusCode) + } + + // read response body + rawrespbody := runtimex.Try1(io.ReadAll(resp.Body)) + + // parse the response body + var response model.OOAPILoginAuth + must.UnmarshalJSON(rawrespbody, &response) + + // return token and expiry date + return response.Token, response.Expire + } + + t.Run("successful login", func(t *testing.T) { + _, _ = loginflow(registerflow(t)) + }) + + t.Run("login with invalid password", func(t *testing.T) { + // obtain the credentials + username, _ := registerflow(t) + + // obtain the response using a completely different password + resp := loginrequest(username, "antani") + + // make sure we eventually close the body + defer resp.Body.Close() + + // we expect to see 401 + if resp.StatusCode != http.StatusUnauthorized { + t.Fatal("unexpected status code", resp.StatusCode) + } + }) + + t.Run("get psiphon config with invalid method", func(t *testing.T) { + // obtain the token + token, _ := loginflow(registerflow(t)) + + // create HTTP request + req := runtimex.Try1(http.NewRequest( + "DELETE", + runtimex.Try1(urlx.ResolveReference(server.URL, "/api/v1/test-list/psiphon-config", "")), + nil, + )) + + // create the authorization token + req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", token)) + + // perform the round trip + resp, err := http.DefaultClient.Do(req) + + // we do not expect an error + if err != nil { + t.Fatal(err) + } + + // make sure we eventually close the body + defer resp.Body.Close() + + // we expect to see not implemented + if resp.StatusCode != http.StatusNotImplemented { + t.Fatal("unexpected status code", resp.StatusCode) + } + }) + + t.Run("get tor targets with invalid method", func(t *testing.T) { + // obtain the token + token, _ := loginflow(registerflow(t)) + + // create HTTP request + req := runtimex.Try1(http.NewRequest( + "DELETE", + runtimex.Try1(urlx.ResolveReference(server.URL, "/api/v1/test-list/tor-targets", "")), + nil, + )) + + // create the authorization token + req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", token)) + + // perform the round trip + resp, err := http.DefaultClient.Do(req) + + // we do not expect an error + if err != nil { + t.Fatal(err) + } + + // make sure we eventually close the body + defer resp.Body.Close() + + // we expect to see not implemented + if resp.StatusCode != http.StatusNotImplemented { + t.Fatal("unexpected status code", resp.StatusCode) + } + }) + + t.Run("get psiphon config with invalid token", func(t *testing.T) { + // create HTTP request + req := runtimex.Try1(http.NewRequest( + "GET", + runtimex.Try1(urlx.ResolveReference(server.URL, "/api/v1/test-list/psiphon-config", "")), + nil, + )) + + // create the authorization token + req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", "antani")) + + // perform the round trip + resp, err := http.DefaultClient.Do(req) + + // we do not expect an error + if err != nil { + t.Fatal(err) + } + + // make sure we eventually close the body + defer resp.Body.Close() + + // we expect to see 401 + if resp.StatusCode != http.StatusUnauthorized { + t.Fatal("unexpected status code", resp.StatusCode) + } + }) + + t.Run("get psiphon config with expired token", func(t *testing.T) { + // obtain the credentials + username, password := registerflow(t) + + // obtain the token + token, _ := loginflow(username, password) + + // modify the token expiry time so that it's expired + state.DoWithLockedUserRecord(username, func(rec *OONIBackendWithLoginFlowUserRecord) error { + rec.Expire = time.Now().Add(-1 * time.Hour) + return nil + }) + + // create HTTP request + req := runtimex.Try1(http.NewRequest( + "GET", + runtimex.Try1(urlx.ResolveReference(server.URL, "/api/v1/test-list/psiphon-config", "")), + nil, + )) + + // create the authorization token + req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", token)) + + // perform the round trip + resp, err := http.DefaultClient.Do(req) + + // we do not expect an error + if err != nil { + t.Fatal(err) + } + + // make sure we eventually close the body + defer resp.Body.Close() + + // we expect to see 401 + if resp.StatusCode != http.StatusUnauthorized { + t.Fatal("unexpected status code", resp.StatusCode) + } + }) + + t.Run("we can get psiphon config", func(t *testing.T) { + // define the expected body + expectedbody := []byte(`bonsoir elliot`) + + // set the config + state.SetPsiphonConfig(expectedbody) + + // obtain the credentials + username, password := registerflow(t) + + // obtain the token + token, _ := loginflow(username, password) + + // create HTTP request + req := runtimex.Try1(http.NewRequest( + "GET", + runtimex.Try1(urlx.ResolveReference(server.URL, "/api/v1/test-list/psiphon-config", "")), + nil, + )) + + // create the authorization token + req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", token)) + + // perform the round trip + resp, err := http.DefaultClient.Do(req) + + // we do not expect an error + if err != nil { + t.Fatal(err) + } + + // make sure we eventually close the body + defer resp.Body.Close() + + // we expect to see 200 + if resp.StatusCode != http.StatusOK { + t.Fatal("unexpected status code", resp.StatusCode) + } + + // read the full body + rawrespbody := runtimex.Try1(io.ReadAll(resp.Body)) + + // make sure we've got the expected body + if diff := cmp.Diff(expectedbody, rawrespbody); err != nil { + t.Fatal(diff) + } + }) + + t.Run("we can get tor targets", func(t *testing.T) { + // define the expected body + expectedbody := []byte(`bonsoir elliot`) + + // set the targets + state.SetTorTargets(expectedbody) + + // obtain the credentials + username, password := registerflow(t) + + // obtain the token + token, _ := loginflow(username, password) + + // create HTTP request + req := runtimex.Try1(http.NewRequest( + "GET", + runtimex.Try1(urlx.ResolveReference(server.URL, "/api/v1/test-list/tor-targets", "country_code=IT")), + nil, + )) + + // create the authorization token + req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", token)) + + // perform the round trip + resp, err := http.DefaultClient.Do(req) + + // we do not expect an error + if err != nil { + t.Fatal(err) + } + + // make sure we eventually close the body + defer resp.Body.Close() + + // we expect to see 200 + if resp.StatusCode != http.StatusOK { + t.Fatal("unexpected status code", resp.StatusCode) + } + + // read the full body + rawrespbody := runtimex.Try1(io.ReadAll(resp.Body)) + + // make sure we've got the expected body + if diff := cmp.Diff(expectedbody, rawrespbody); err != nil { + t.Fatal(diff) + } + }) + + t.Run("we need query string to get tor targets", func(t *testing.T) { + // define the expected body + expectedbody := []byte(`bonsoir elliot`) + + // set the targets + state.SetTorTargets(expectedbody) + + // obtain the credentials + username, password := registerflow(t) + + // obtain the token + token, _ := loginflow(username, password) + + // create HTTP request + req := runtimex.Try1(http.NewRequest( + "GET", + runtimex.Try1(urlx.ResolveReference(server.URL, "/api/v1/test-list/tor-targets", "")), + nil, + )) + + // create the authorization token + req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", token)) + + // perform the round trip + resp, err := http.DefaultClient.Do(req) + + // we do not expect an error + if err != nil { + t.Fatal(err) + } + + // make sure we eventually close the body + defer resp.Body.Close() + + // we expect to see 400 + if resp.StatusCode != http.StatusBadRequest { + t.Fatal("unexpected status code", resp.StatusCode) + } + }) +} diff --git a/internal/urlx/DESIGN.md b/internal/urlx/DESIGN.md new file mode 100644 index 000000000..6823aa962 --- /dev/null +++ b/internal/urlx/DESIGN.md @@ -0,0 +1,32 @@ +# ./internal/urlx + +This package contains algorithms to operate on URLs. + +## ResolveReference + +This function has the following signature: + +```Go +func ResolveReference(baseURL, path, rawQuery string) (string, error) +``` + +It solves the problem of computing a composed URL starting from a base URL, an +extra path and a possibly empty raw query. The algorithm will ignore the path and +the query of the base URL and only use the scheme and the host. + +For example, assuming the following: + +```Go +baseURL := "https://api.ooni.io/antani?foo=bar" +path := "/api/v1/check-in" +query := "bar=baz" +``` + +This function will return this URL: + +```Go +"https://api.ooni.io/ap1/v1/check-in?bar=baz" +``` + +We need this functionality when implementing communication with the probe services, +where we have a base URL and specific path and optional query for each API. diff --git a/internal/urlx/urlx.go b/internal/urlx/urlx.go new file mode 100644 index 000000000..d41c061e9 --- /dev/null +++ b/internal/urlx/urlx.go @@ -0,0 +1,32 @@ +// Package urlx contains URL extensions. +package urlx + +import ( + "net/url" +) + +// ResolveReference constructs a new URL consisting of the given base URL with +// the path appended to the given path and the optional query. +// +// For example, given: +// +// URL := "https://api.ooni.io/api/v1" +// path := "/measurement_meta" +// rawQuery := "full=true" +// +// This function will return: +// +// result := "https://api.ooni.io/api/v1/measurement_meta?full=true" +// +// This function fails when we cannot parse URL as a [*net.URL]. +func ResolveReference(baseURL, path, rawQuery string) (string, error) { + parsedBase, err := url.Parse(baseURL) + if err != nil { + return "", err + } + ref := &url.URL{ + Path: path, + RawQuery: rawQuery, + } + return parsedBase.ResolveReference(ref).String(), nil +} diff --git a/internal/urlx/urlx_test.go b/internal/urlx/urlx_test.go new file mode 100644 index 000000000..5e1ba59f4 --- /dev/null +++ b/internal/urlx/urlx_test.go @@ -0,0 +1,127 @@ +package urlx + +import ( + "errors" + "testing" + + "github.com/google/go-cmp/cmp" +) + +func TestResolveReference(t *testing.T) { + // testcase is a test case used by this function + type testcase struct { + // Name is the test case name. + Name string + + // BaseURL is the base URL. + BaseURL string + + // Path is the extra path to append. + Path string + + // RawQuery is the raw query. + RawQuery string + + // ExpectErr is the expected err. + ExpectErr error + + // ExpectURL is what we expect to see. + ExpectURL string + } + + cases := []testcase{{ + Name: "when we cannot parse the base URL", + BaseURL: "\t", // invalid + Path: "", + RawQuery: "", + ExpectErr: errors.New(`parse "\t": net/url: invalid control character in URL`), + ExpectURL: "", + }, { + Name: "when there's only the base URL", + BaseURL: "https://api.ooni.io/", + Path: "", + RawQuery: "", + ExpectErr: nil, + ExpectURL: "https://api.ooni.io/", + }, { + Name: "when there's only the path", + BaseURL: "", + Path: "/api/v1/check-in", + RawQuery: "", + ExpectErr: nil, + ExpectURL: "/api/v1/check-in", + }, { + Name: "when there's only the query", + BaseURL: "", + Path: "", + RawQuery: "key1=value1&key1=value2&key3=value3", + ExpectErr: nil, + ExpectURL: "?key1=value1&key1=value2&key3=value3", + }, { + Name: "with base URL and path", + BaseURL: "https://api.ooni.io/", + Path: "/api/v1/check-in", + RawQuery: "", + ExpectErr: nil, + ExpectURL: "https://api.ooni.io/api/v1/check-in", + }, { + Name: "with base URL and query", + BaseURL: "https://api.ooni.io/", + Path: "", + RawQuery: "key1=value1&key1=value2&key3=value3", + ExpectErr: nil, + ExpectURL: "https://api.ooni.io/?key1=value1&key1=value2&key3=value3", + }, { + Name: "with base URL, path, and query", + BaseURL: "https://api.ooni.io/", + Path: "/api/v1/check-in", + RawQuery: "key1=value1&key1=value2&key3=value3", + ExpectErr: nil, + ExpectURL: "https://api.ooni.io/api/v1/check-in?key1=value1&key1=value2&key3=value3", + }, { + Name: "with base URL with path, path, and query", + BaseURL: "https://api.ooni.io/api", + Path: "/v1/check-in", + RawQuery: "key1=value1&key1=value2&key3=value3", + ExpectErr: nil, + ExpectURL: "https://api.ooni.io/v1/check-in?key1=value1&key1=value2&key3=value3", + }, { + Name: "with base URL with path and query, path, and query", + BaseURL: "https://api.ooni.io/api?foo=bar", + Path: "/v1/check-in", + RawQuery: "key1=value1&key1=value2&key3=value3", + ExpectErr: nil, + ExpectURL: "https://api.ooni.io/v1/check-in?key1=value1&key1=value2&key3=value3", + }} + + for _, tc := range cases { + t.Run(tc.Name, func(t *testing.T) { + // invoke the API we're currently testing + got, err := ResolveReference(tc.BaseURL, tc.Path, tc.RawQuery) + + // check for errors + switch { + case err == nil && tc.ExpectErr == nil: + if diff := cmp.Diff(tc.ExpectURL, got); diff != "" { + t.Fatal(diff) + } + return + + case err != nil && tc.ExpectErr == nil: + t.Fatal("expected err", tc.ExpectErr, "got", err) + return + + case err == nil && tc.ExpectErr != nil: + t.Fatal("expected err", tc.ExpectErr, "got", err) + return + + case err != nil && tc.ExpectErr != nil: + if err.Error() != tc.ExpectErr.Error() { + t.Fatal("expected err", tc.ExpectErr, "got", err) + } + return + } + + }) + } +} diff --git a/script/nocopyreadall.bash b/script/nocopyreadall.bash index 748461172..be14d720d 100755 --- a/script/nocopyreadall.bash +++ b/script/nocopyreadall.bash @@ -1,5 +1,13 @@ #!/bin/bash set -euo pipefail + +# +# This script ensures that non-testing code never uses io.ReadAll and io.Copy and +# prefers their netxlite counterparts who also take a context argument. +# +# See https://github.com/ooni/probe/issues/1609. +# + exitcode=0 for file in $(find . -type f -name \*.go); do if [ "$file" = "./internal/netemx/ooapi_test.go" ]; then @@ -71,6 +79,18 @@ for file in $(find . -type f -name \*.go); do continue fi + if [ "$file" = "./internal/testingx/oonibackendwithlogin.go" ]; then + # We're allowed to use ReadAll and Copy in this file because + # it's code that we only use for testing purposes. + continue + fi + + if [ "$file" = "./internal/testingx/oonibackendwithlogin_test.go" ]; then + # We're allowed to use ReadAll and Copy in this file because + # it's code that we only use for testing purposes. + continue + fi + if [ "$file" = "./internal/testingx/tlssniproxy.go" ]; then # We're allowed to use ReadAll and Copy in this file because # it's code that we only use for testing purposes.