More progress towards release v3.10.0 (#320)

* chore: unvendor github.com/mitchellh/go-wordwrap

The library seems reasonably maintained and tested.

Part of https://github.com/ooni/probe/issues/1439

* fix(netx/quicdialer): ensure we handle all errors

Part of https://github.com/ooni/probe/issues/1439

* fix previous

* cleanup: remove unnecessary shutil fork

Part of https://github.com/ooni/probe/issues/1439

* doc: documented some undocumented functions

Part of https://github.com/ooni/probe/issues/1439

* fix(ooniprobe): rename mis-named function

Part of https://github.com/ooni/probe/issues/1439

cmd/ooniprobe/internal/cli/onboard/onboard.go

@@ -23,7 +23,7 @@ func Onboarding(config *config.Config) error {
 	fmt.Println()
 	output.Paragraph("OONI Probe checks whether your provider blocks access to sites and services. Run OONI Probe to collect evidence of internet censorship and to measure your network performance.")
 	fmt.Println()
-	err := output.PressEnterToContinue("Press 'Enter' to continue...")
+	err := output.PressAnyKeyToContinue("Press any key to continue...")
 	if err != nil {
 		return err
 	}
@@ -38,7 +38,7 @@ func Onboarding(config *config.Config) error {
 	fmt.Println()
 	output.Bullet("Read the documentation to learn more.")
 	fmt.Println()
-	err = output.PressEnterToContinue("Press 'Enter' to continue...")
+	err = output.PressAnyKeyToContinue("Press any key to continue...")
 	if err != nil {
 		return err
 	}

cmd/ooniprobe/internal/nettests/nettests_test.go

@@ -8,9 +8,16 @@ import (
 
 	"github.com/ooni/probe-cli/v3/cmd/ooniprobe/internal/database"
 	"github.com/ooni/probe-cli/v3/cmd/ooniprobe/internal/ooni"
-	"github.com/ooni/probe-cli/v3/cmd/ooniprobe/internal/utils/shutil"
 )
 
+func copyfile(source, dest string) error {
+	data, err := ioutil.ReadFile(source)
+	if err != nil {
+		return err
+	}
+	return ioutil.WriteFile(dest, data, 0600)
+}
+
 func newOONIProbe(t *testing.T) *ooni.Probe {
 	homePath, err := ioutil.TempDir("", "ooniprobetests")
 	if err != nil {
@@ -18,7 +25,9 @@ func newOONIProbe(t *testing.T) *ooni.Probe {
 	}
 	configPath := path.Join(homePath, "config.json")
 	testingConfig := path.Join("..", "..", "testdata", "testing-config.json")
-	shutil.Copy(testingConfig, configPath, false)
+	if err := copyfile(testingConfig, configPath); err != nil {
+		t.Fatal(err)
+	}
 	probe := ooni.NewProbe(configPath, homePath)
 	swName := "ooniprobe-cli-tests"
 	swVersion := "3.0.0-alpha"

cmd/ooniprobe/internal/output/output.go

@@ -7,8 +7,8 @@ import (
 	"time"
 
 	"github.com/apex/log"
+	"github.com/mitchellh/go-wordwrap"
 	"github.com/ooni/probe-cli/v3/cmd/ooniprobe/internal/database"
-	"github.com/ooni/probe-cli/v3/cmd/ooniprobe/internal/utils"
 )
 
 // MeasurementJSON prints the JSON of a measurement
@@ -29,6 +29,7 @@ func Progress(key string, perc float64, eta float64, msg string) {
 	}).Info(msg)
 }
 
+// MeasurementSummaryData contains summary information on the measurement
 type MeasurementSummaryData struct {
 	TotalRuntime       float64
 	TotalCount         int64
@@ -41,6 +42,7 @@ type MeasurementSummaryData struct {
 	StartTime          time.Time
 }
 
+// MeasurementSummary emits the measurement summary
 func MeasurementSummary(msmt MeasurementSummaryData) {
 	log.WithFields(log.Fields{
 		"type":                 "measurement_summary",
@@ -128,6 +130,7 @@ func ResultItem(result ResultItemData) {
 	}).Info("result item")
 }
 
+// ResultSummaryData contains the summary data of a result
 type ResultSummaryData struct {
 	TotalTests         int64
 	TotalDataUsageUp   float64
@@ -135,6 +138,7 @@ type ResultSummaryData struct {
 	TotalNetworks      int64
 }
 
+// ResultSummary emits the result summary
 func ResultSummary(result ResultSummaryData) {
 	log.WithFields(log.Fields{
 		"type":                  "result_summary",
@@ -153,17 +157,20 @@ func SectionTitle(text string) {
 	}).Info(text)
 }
 
+// Paragraph makes a word-wrapped paragraph out of text
 func Paragraph(text string) {
 	const width = 80
-	fmt.Println(utils.WrapString(text, width))
+	fmt.Println(wordwrap.WrapString(text, width))
 }
 
+// Bullet is like paragraph but with a bullet point in front
 func Bullet(text string) {
 	const width = 80
-	fmt.Printf("• %s\n", utils.WrapString(text, width))
+	fmt.Printf("• %s\n", wordwrap.WrapString(text, width))
 }
 
-func PressEnterToContinue(text string) error {
+// PressAnyKeyToContinue blocks until the user presses any key
+func PressAnyKeyToContinue(text string) error {
 	fmt.Print(text)
 	_, err := bufio.NewReader(os.Stdin).ReadBytes('\n')
 	return err

cmd/ooniprobe/internal/utils/shutil/shutil.go

@@ -1,319 +0,0 @@
-package shutil
-
-import (
-	"fmt"
-	"io"
-	"io/ioutil"
-	"os"
-	"path/filepath"
-)
-
-type SameFileError struct {
-	Src string
-	Dst string
-}
-
-func (e SameFileError) Error() string {
-	return fmt.Sprintf("%s and %s are the same file", e.Src, e.Dst)
-}
-
-type SpecialFileError struct {
-	File     string
-	FileInfo os.FileInfo
-}
-
-func (e SpecialFileError) Error() string {
-	return fmt.Sprintf("`%s` is a named pipe", e.File)
-}
-
-type NotADirectoryError struct {
-	Src string
-}
-
-func (e NotADirectoryError) Error() string {
-	return fmt.Sprintf("`%s` is not a directory", e.Src)
-}
-
-type AlreadyExistsError struct {
-	Dst string
-}
-
-func (e AlreadyExistsError) Error() string {
-	return fmt.Sprintf("`%s` already exists", e.Dst)
-}
-
-func samefile(src string, dst string) bool {
-	srcInfo, _ := os.Stat(src)
-	dstInfo, _ := os.Stat(dst)
-	return os.SameFile(srcInfo, dstInfo)
-}
-
-func specialfile(fi os.FileInfo) bool {
-	return (fi.Mode() & os.ModeNamedPipe) == os.ModeNamedPipe
-}
-
-func stringInSlice(a string, list []string) bool {
-	for _, b := range list {
-		if b == a {
-			return true
-		}
-	}
-	return false
-}
-
-func IsSymlink(fi os.FileInfo) bool {
-	return (fi.Mode() & os.ModeSymlink) == os.ModeSymlink
-}
-
-// Copy data from src to dst
-//
-// If followSymlinks is not set and src is a symbolic link, a
-// new symlink will be created instead of copying the file it points
-// to.
-func CopyFile(src, dst string, followSymlinks bool) error {
-	if samefile(src, dst) {
-		return &SameFileError{src, dst}
-	}
-
-	// Make sure src exists and neither are special files
-	srcStat, err := os.Lstat(src)
-	if err != nil {
-		return err
-	}
-	if specialfile(srcStat) {
-		return &SpecialFileError{src, srcStat}
-	}
-
-	dstStat, err := os.Stat(dst)
-	if err != nil && !os.IsNotExist(err) {
-		return err
-	} else if err == nil {
-		if specialfile(dstStat) {
-			return &SpecialFileError{dst, dstStat}
-		}
-	}
-
-	// If we don't follow symlinks and it's a symlink, just link it and be done
-	if !followSymlinks && IsSymlink(srcStat) {
-		return os.Symlink(src, dst)
-	}
-
-	// If we are a symlink, follow it
-	if IsSymlink(srcStat) {
-		src, err = os.Readlink(src)
-		if err != nil {
-			return err
-		}
-		srcStat, err = os.Stat(src)
-		if err != nil {
-			return err
-		}
-	}
-
-	// Do the actual copy
-	fsrc, err := os.Open(src)
-	if err != nil {
-		return err
-	}
-	defer fsrc.Close()
-
-	fdst, err := os.Create(dst)
-	if err != nil {
-		return err
-	}
-	defer fdst.Close()
-
-	size, err := io.Copy(fdst, fsrc)
-	if err != nil {
-		return err
-	}
-
-	if size != srcStat.Size() {
-		return fmt.Errorf("%s: %d/%d copied", src, size, srcStat.Size())
-	}
-
-	return nil
-}
-
-// Copy mode bits from src to dst.
-//
-// If followSymlinks is false, symlinks aren't followed if and only
-// if both `src` and `dst` are symlinks. If `lchmod` isn't available
-// and both are symlinks this does nothing. (I don't think lchmod is
-// available in Go)
-func CopyMode(src, dst string, followSymlinks bool) error {
-	srcStat, err := os.Lstat(src)
-	if err != nil {
-		return err
-	}
-
-	dstStat, err := os.Lstat(dst)
-	if err != nil {
-		return err
-	}
-
-	// They are both symlinks and we can't change mode on symlinks.
-	if !followSymlinks && IsSymlink(srcStat) && IsSymlink(dstStat) {
-		return nil
-	}
-
-	// Atleast one is not a symlink, get the actual file stats
-	srcStat, _ = os.Stat(src)
-	err = os.Chmod(dst, srcStat.Mode())
-	return err
-}
-
-// Copy data and mode bits ("cp src dst"). Return the file's destination.
-//
-// The destination may be a directory.
-//
-// If followSymlinks is false, symlinks won't be followed. This
-// resembles GNU's "cp -P src dst".
-//
-// If source and destination are the same file, a SameFileError will be
-// rased.
-func Copy(src, dst string, followSymlinks bool) (string, error) {
-	dstInfo, err := os.Stat(dst)
-
-	if err == nil && dstInfo.Mode().IsDir() {
-		dst = filepath.Join(dst, filepath.Base(src))
-	}
-
-	if err != nil && !os.IsNotExist(err) {
-		return dst, err
-	}
-
-	err = CopyFile(src, dst, followSymlinks)
-	if err != nil {
-		return dst, err
-	}
-
-	err = CopyMode(src, dst, followSymlinks)
-	if err != nil {
-		return dst, err
-	}
-
-	return dst, nil
-}
-
-type CopyTreeOptions struct {
-	Symlinks               bool
-	IgnoreDanglingSymlinks bool
-	CopyFunction           func(string, string, bool) (string, error)
-	Ignore                 func(string, []os.FileInfo) []string
-}
-
-// Recursively copy a directory tree.
-//
-// The destination directory must not already exist.
-//
-// If the optional Symlinks flag is true, symbolic links in the
-// source tree result in symbolic links in the destination tree; if
-// it is false, the contents of the files pointed to by symbolic
-// links are copied. If the file pointed by the symlink doesn't
-// exist, an error will be returned.
-//
-// You can set the optional IgnoreDanglingSymlinks flag to true if you
-// want to silence this error. Notice that this has no effect on
-// platforms that don't support os.Symlink.
-//
-// The optional ignore argument is a callable. If given, it
-// is called with the `src` parameter, which is the directory
-// being visited by CopyTree(), and `names` which is the list of
-// `src` contents, as returned by ioutil.ReadDir():
-//
-//   callable(src, entries) -> ignoredNames
-//
-// Since CopyTree() is called recursively, the callable will be
-// called once for each directory that is copied. It returns a
-// list of names relative to the `src` directory that should
-// not be copied.
-//
-// The optional copyFunction argument is a callable that will be used
-// to copy each file. It will be called with the source path and the
-// destination path as arguments. By default, Copy() is used, but any
-// function that supports the same signature (like Copy2() when it
-// exists) can be used.
-func CopyTree(src, dst string, options *CopyTreeOptions) error {
-	if options == nil {
-		options = &CopyTreeOptions{Symlinks: false,
-			Ignore:                 nil,
-			CopyFunction:           Copy,
-			IgnoreDanglingSymlinks: false}
-	}
-
-	srcFileInfo, err := os.Stat(src)
-	if err != nil {
-		return err
-	}
-
-	if !srcFileInfo.IsDir() {
-		return &NotADirectoryError{src}
-	}
-
-	_, err = os.Open(dst)
-	if !os.IsNotExist(err) {
-		return &AlreadyExistsError{dst}
-	}
-
-	entries, err := ioutil.ReadDir(src)
-	if err != nil {
-		return err
-	}
-
-	err = os.MkdirAll(dst, srcFileInfo.Mode())
-	if err != nil {
-		return err
-	}
-
-	ignoredNames := []string{}
-	if options.Ignore != nil {
-		ignoredNames = options.Ignore(src, entries)
-	}
-
-	for _, entry := range entries {
-		if stringInSlice(entry.Name(), ignoredNames) {
-			continue
-		}
-		srcPath := filepath.Join(src, entry.Name())
-		dstPath := filepath.Join(dst, entry.Name())
-
-		entryFileInfo, err := os.Lstat(srcPath)
-		if err != nil {
-			return err
-		}
-
-		// Deal with symlinks
-		if IsSymlink(entryFileInfo) {
-			linkTo, err := os.Readlink(srcPath)
-			if err != nil {
-				return err
-			}
-			if options.Symlinks {
-				os.Symlink(linkTo, dstPath)
-				//CopyStat(srcPath, dstPath, false)
-			} else {
-				// ignore dangling symlink if flag is on
-				_, err = os.Stat(linkTo)
-				if os.IsNotExist(err) && options.IgnoreDanglingSymlinks {
-					continue
-				}
-				_, err = options.CopyFunction(srcPath, dstPath, false)
-				if err != nil {
-					return err
-				}
-			}
-		} else if entryFileInfo.IsDir() {
-			err = CopyTree(srcPath, dstPath, options)
-			if err != nil {
-				return err
-			}
-		} else {
-			_, err = options.CopyFunction(srcPath, dstPath, false)
-			if err != nil {
-				return err
-			}
-		}
-	}
-	return nil
-}

cmd/ooniprobe/internal/utils/utils.go

@@ -1,12 +1,10 @@
 package utils
 
 import (
-	"bytes"
 	"fmt"
 	"os"
 	"regexp"
 	"strings"
-	"unicode"
 	"unicode/utf8"
 
 	"github.com/fatih/color"
@@ -47,71 +45,3 @@ func RightPad(str string, length int) string {
 	}
 	return str + strings.Repeat(" ", c)
 }
-
-// WrapString wraps the given string within lim width in characters.
-//
-// Wrapping is currently naive and only happens at white-space. A future
-// version of the library will implement smarter wrapping. This means that
-// pathological cases can dramatically reach past the limit, such as a very
-// long word.
-// This is taken from: https://github.com/mitchellh/go-wordwrap/tree/f253961a26562056904822f2a52d4692347db1bd
-func WrapString(s string, lim uint) string {
-	// Initialize a buffer with a slightly larger size to account for breaks
-	init := make([]byte, 0, len(s))
-	buf := bytes.NewBuffer(init)
-
-	var current uint
-	var wordBuf, spaceBuf bytes.Buffer
-
-	for _, char := range s {
-		if char == '\n' {
-			if wordBuf.Len() == 0 {
-				if current+uint(spaceBuf.Len()) > lim {
-					current = 0
-				} else {
-					current += uint(spaceBuf.Len())
-					spaceBuf.WriteTo(buf)
-				}
-				spaceBuf.Reset()
-			} else {
-				current += uint(spaceBuf.Len() + wordBuf.Len())
-				spaceBuf.WriteTo(buf)
-				spaceBuf.Reset()
-				wordBuf.WriteTo(buf)
-				wordBuf.Reset()
-			}
-			buf.WriteRune(char)
-			current = 0
-		} else if unicode.IsSpace(char) {
-			if spaceBuf.Len() == 0 || wordBuf.Len() > 0 {
-				current += uint(spaceBuf.Len() + wordBuf.Len())
-				spaceBuf.WriteTo(buf)
-				spaceBuf.Reset()
-				wordBuf.WriteTo(buf)
-				wordBuf.Reset()
-			}
-
-			spaceBuf.WriteRune(char)
-		} else {
-
-			wordBuf.WriteRune(char)
-
-			if current+uint(spaceBuf.Len()+wordBuf.Len()) > lim && uint(wordBuf.Len()) < lim {
-				buf.WriteRune('\n')
-				current = 0
-				spaceBuf.Reset()
-			}
-		}
-	}
-
-	if wordBuf.Len() == 0 {
-		if current+uint(spaceBuf.Len()) <= lim {
-			spaceBuf.WriteTo(buf)
-		}
-	} else {
-		spaceBuf.WriteTo(buf)
-		wordBuf.WriteTo(buf)
-	}
-
-	return buf.String()
-}

go.mod

@@ -26,6 +26,7 @@ require (
 	github.com/mattn/go-sqlite3 v1.14.7 // indirect
 	github.com/mgutz/ansi v0.0.0-20200706080929-d51e80ef957d // indirect
 	github.com/miekg/dns v1.1.41
+	github.com/mitchellh/go-wordwrap v1.0.1
 	github.com/montanaflynn/stats v0.6.6
 	github.com/ooni/probe-assets v0.2.0
 	github.com/ooni/psiphon v0.7.0

go.sum

@@ -238,6 +238,8 @@ github.com/miekg/dns v1.1.41/go.mod h1:p6aan82bvRIyn+zDIv9xYNUpwa73JcSh9BKwknJys
 github.com/mitchellh/cli v1.1.2/go.mod h1:6iaV0fGdElS6dPBx0EApTxHrcWvmJphyh2n8YBLPPZ4=
 github.com/mitchellh/copystructure v1.0.0/go.mod h1:SNtv71yrdKgLRyLFxmLdkAbkKEFWgYaq1OVrnRcwhnw=
 github.com/mitchellh/go-homedir v1.1.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0=
+github.com/mitchellh/go-wordwrap v1.0.1 h1:TLuKupo69TCn6TQSyGxwI1EblZZEsQ0vMlAFQflz0v0=
+github.com/mitchellh/go-wordwrap v1.0.1/go.mod h1:R62XHJLzvMFRBbcrT7m7WgmE1eOyTSsCt+hzestvNj0=
 github.com/mitchellh/mapstructure v1.1.2/go.mod h1:FVVH3fgwuzCH5S8UJGiWEs2h04kUh9fWfEaFds41c1Y=
 github.com/mitchellh/reflectwalk v1.0.0/go.mod h1:mSTlrgnPZtwu0c4WaC2kGObEpuNDbx0jmZXqmk4esnw=
 github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q=
@@ -259,8 +261,6 @@ github.com/onsi/gomega v1.5.0/go.mod h1:ex+gbHU/CVuBBDIJjb2X0qEXbFg53c61hWP/1Cpa
 github.com/onsi/gomega v1.7.1/go.mod h1:XdKZgCCFLUoM/7CFJVPcG8C1xQ1AJ0vpAezJrB7JYyY=
 github.com/onsi/gomega v1.10.1 h1:o0+MgICZLuZ7xjH7Vx6zS/zcu93/BEp1VwkIW1mEXCE=
 github.com/onsi/gomega v1.10.1/go.mod h1:iN09h71vgCQne3DLsj+A5owkum+a2tYe+TOCB1ybHNo=
-github.com/ooni/probe-assets v0.1.0 h1:t0sDDKvpLQpSBjIL0EU2qh/aRUbdd6tF8Jrw9cmY9lM=
-github.com/ooni/probe-assets v0.1.0/go.mod h1:N0PyNM3aadlYDDCFXAPzs54HC54+MZA/4/xnCtd9EAo=
 github.com/ooni/probe-assets v0.2.0 h1:x4zWJVVJCWlFD6TUvFpN7cqanCCpiYGZxqmZ1Vb0KmA=
 github.com/ooni/probe-assets v0.2.0/go.mod h1:N0PyNM3aadlYDDCFXAPzs54HC54+MZA/4/xnCtd9EAo=
 github.com/ooni/psiphon v0.7.0 h1:rWMQP7BTR5FSgzxhbOdk9XwegQJjiR2l9L57+HX300Q=

internal/engine/netx/quicdialer/system.go

@@ -26,6 +26,9 @@ type SystemDialer struct {
 func (d SystemDialer) DialContext(ctx context.Context, network string,
 	host string, tlsCfg *tls.Config, cfg *quic.Config) (quic.EarlySession, error) {
 	onlyhost, onlyport, err := net.SplitHostPort(host)
+	if err != nil {
+		return nil, err
+	}
 	port, err := strconv.Atoi(onlyport)
 	if err != nil {
 		return nil, err
@@ -36,6 +39,9 @@ func (d SystemDialer) DialContext(ctx context.Context, network string,
 		return nil, errors.New("quicdialer: invalid IP representation")
 	}
 	udpConn, err := net.ListenUDP("udp", &net.UDPAddr{IP: net.IPv4zero, Port: 0})
+	if err != nil {
+		return nil, err
+	}
 	var pconn net.PacketConn = udpConn
 	if d.Saver != nil {
 		pconn = saverUDPConn{UDPConn: udpConn, saver: d.Saver}