-
Notifications
You must be signed in to change notification settings - Fork 54
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
poc: rewrite urlgetter using step-by-step
This diff shows how we could incrementally rewrite urlgetter using a step-by-step measurement style. Additionally, this diff modifies the facebook_messanger experiment to show what changes are required to upgrade it. The general idea of these changes is to incrementally move experiments away from depending on ./internal/experiment/urlgetter, and instead use a near drop-in replacement implementation, implemented in ./internal/urlgetter, which uses step-by-step to measure. Because ./internal/experiment/urlgetter depends on ./internal/legacy/netx and, instead, ./internal/urlgetter depends on ./internal/measurexlite, by performing this kind of migration we make ./internal/legacy/netx unnecessary. Also, because most users of ./internal/experiment/urlgetter only use limited functionality, incremental refactoring would be possible. Reference issue: ooni/probe#2751.
- Loading branch information
1 parent
5be3a9a
commit 0f18874
Showing
16 changed files
with
1,394 additions
and
39 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| package urlgetter | ||
|
|
||
| // Config contains the configuration. | ||
| type Config struct { | ||
| // HTTPHost allows overriding the default HTTP host. | ||
| HTTPHost string `ooni:"Force using specific HTTP Host header"` | ||
|
|
||
| // HTTPReferer sets the HTTP referer value. | ||
| HTTPReferer string `ooni:"Force using the specific HTTP Referer header"` | ||
|
|
||
| // Method selects the HTTP method to use. | ||
| Method string `ooni:"Force HTTP method different than GET"` | ||
|
|
||
| // NoFollowRedirects disables following redirects. | ||
| NoFollowRedirects bool `ooni:"Disable following redirects"` | ||
|
|
||
| // TLSNextProtos is an OPTIONAL comma separated ALPN list. | ||
| TLSNextProtos string `ooni:"Comma-separated list of next protocols for ALPN"` | ||
|
|
||
| // TLSServerName is the OPTIONAL SNI value. | ||
| TLSServerName string `ooni:"SNI value to use"` | ||
| } | ||
|
|
||
| // Clone returns a deep copy of the given [*Config]. | ||
| func (cx *Config) Clone() *Config { | ||
| return &Config{ | ||
| HTTPHost: cx.HTTPHost, | ||
| HTTPReferer: cx.HTTPReferer, | ||
| Method: cx.Method, | ||
| NoFollowRedirects: cx.NoFollowRedirects, | ||
| TLSNextProtos: cx.TLSNextProtos, | ||
| TLSServerName: cx.TLSServerName, | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,110 @@ | ||
| package urlgetter | ||
|
|
||
| import ( | ||
| "context" | ||
| "net" | ||
| "net/url" | ||
| "time" | ||
|
|
||
| "github.com/ooni/probe-cli/v3/internal/logx" | ||
| "github.com/ooni/probe-cli/v3/internal/measurexlite" | ||
| "github.com/ooni/probe-cli/v3/internal/netxlite" | ||
| ) | ||
|
|
||
| // DNSLookup measures a dnslookup://domain/ URL. | ||
| func (rx *Runner) DNSLookup(ctx context.Context, config *Config, URL *url.URL) error { | ||
| _, err := rx.DNSLookupOp(ctx, config, URL) | ||
| return err | ||
| } | ||
|
|
||
| // DNSLookupResult contains the results of a DNS lookup. | ||
| type DNSLookupResult struct { | ||
| // Address is the resolved address. | ||
| Address string | ||
|
|
||
| // Config is the original config. | ||
| Config *Config | ||
|
|
||
| // URL is the original URL. | ||
| URL *url.URL | ||
| } | ||
|
|
||
| // endpoint returns an endpoint given the address and the URL scheme. | ||
| func (rx *DNSLookupResult) endpoint() (string, error) { | ||
| // handle the case where there is an explicit port | ||
| if port := rx.URL.Port(); port != "" { | ||
| return net.JoinHostPort(rx.Address, port), nil | ||
| } | ||
|
|
||
| // use the scheme to deduce the port | ||
| switch rx.URL.Scheme { | ||
| case "http": | ||
| return net.JoinHostPort(rx.Address, "80"), nil | ||
| case "https": | ||
| return net.JoinHostPort(rx.Address, "443"), nil | ||
| case "dot": | ||
| return net.JoinHostPort(rx.Address, "853"), nil | ||
| default: | ||
| return "", ErrUnknownURLScheme | ||
| } | ||
| } | ||
|
|
||
| // DNSLookupOp resolves a domain name using the configured resolver. | ||
| func (rx *Runner) DNSLookupOp(ctx context.Context, config *Config, URL *url.URL) ([]*DNSLookupResult, error) { | ||
| // TODO(bassosimone): choose the proper DNS resolver depending on configuration. | ||
| return rx.DNSLookupGetaddrinfoOp(ctx, config, URL) | ||
| } | ||
|
|
||
| // DNSLookupGetaddrinfoOp performs a DNS lookup using getaddrinfo. | ||
| func (rx *Runner) DNSLookupGetaddrinfoOp(ctx context.Context, config *Config, URL *url.URL) ([]*DNSLookupResult, error) { | ||
| // enforce timeout | ||
| const timeout = 4 * time.Second | ||
| ctx, cancel := context.WithTimeout(ctx, timeout) | ||
| defer cancel() | ||
|
|
||
| // obtain the next trace index | ||
| index := rx.IndexGen.Next() | ||
|
|
||
| // create trace using the given underlying network | ||
| trace := measurexlite.NewTrace(index, rx.Begin) | ||
| trace.Netx = &netxlite.Netx{Underlying: rx.UNet} | ||
|
|
||
| // obtain logger | ||
| logger := rx.Session.Logger() | ||
|
|
||
| // create resolver | ||
| reso := trace.NewStdlibResolver(logger) | ||
|
|
||
| // the domain to resolve is the URL's hostname | ||
| domain := URL.Hostname() | ||
|
|
||
| // start operation logger | ||
| ol := logx.NewOperationLogger(logger, "[#%d] lookup %s using getaddrinfo", index, domain) | ||
|
|
||
| // perform the lookup | ||
| addrs, err := reso.LookupHost(ctx, domain) | ||
|
|
||
| // append the DNS lookup results | ||
| rx.TestKeys.AppendQueries(trace.DNSLookupsFromRoundTrip()...) | ||
|
|
||
| // stop the operation logger | ||
| ol.Stop(err) | ||
|
|
||
| // manually set the failure and failed operation | ||
| if err != nil { | ||
| rx.TestKeys.MaybeSetFailedOperation(netxlite.DNSRoundTripOperation) | ||
| rx.TestKeys.MaybeSetFailure(err.Error()) | ||
| return nil, err | ||
| } | ||
|
|
||
| // emit results | ||
| var results []*DNSLookupResult | ||
| for _, addr := range addrs { | ||
| results = append(results, &DNSLookupResult{ | ||
| Address: addr, | ||
| Config: config, | ||
| URL: URL, | ||
| }) | ||
| } | ||
| return results, nil | ||
| } |
Oops, something went wrong.