add tests

config/proxy/proxy.go

@@ -52,14 +52,16 @@ type config struct {
 func Init(saveDir string, obfuscate bool) (*config, error) {
 	if saveDir == "" {
 		saveDir = defaultConfigSaveDir
-		_config.filePath = filepath.Join(saveDir, defaultConfigFilename)
 	}
 
+	_config.mu.Lock()
+	_config.filePath = filepath.Join(saveDir, defaultConfigFilename)
 	_config.obfuscate = obfuscate
+	_config.mu.Unlock()
 
 	saved, err := readExistingConfig(_config.filePath, obfuscate)
 	if err != nil {
-		log.Errorf("Failed to read existing client config: %w", err)
+		log.Error(err)
 	}
 
 	if saved == nil {
@@ -73,8 +75,9 @@ func Init(saveDir string, obfuscate bool) (*config, error) {
 
 // GetConfig implements services.ConfigHandler
 func (c *config) GetConfig() *ProxyConfig {
-	conf, _ := c.config.Get(eventual.DontWait)
-	return conf.(*ProxyConfig)
+	v, _ := c.config.Get(eventual.DontWait)
+	conf, _ := v.(*ProxyConfig)
+	return conf
 }
 
 // SetConfig implements services.ConfigHandler
@@ -95,7 +98,7 @@ func (c *config) SetConfig(new *ProxyConfig) {
 
 	c.config.Set(updated)
 	if err := saveConfig(c.filePath, updated, c.obfuscate); err != nil {
-		log.Errorf("Failed to save client config: %w", err)
+		log.Errorf("Failed to save client config: %v", err)
 	}
 
 	notifyListeners(old, updated)
@@ -143,8 +146,9 @@ func readExistingConfig(filePath string, obfuscate bool) (*ProxyConfig, error) {
 		return nil, fmt.Errorf("failed to read config from %v: %w", filePath, err)
 	}
 
-	if len(bytes) == 0 {
-		return nil, nil // file is empty
+	if len(bytes) == 0 { // file is empty
+		// we treat an empty file as if it doesn't
+		return nil, nil
 	}
 
 	conf := &ProxyConfig{}

config/proxy/proxy_test.go

@@ -0,0 +1,148 @@
+package proxyconfig
+
+import (
+	"fmt"
+	"io"
+	"os"
+	"testing"
+	"time"
+
+	"github.com/getlantern/rot13"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"google.golang.org/protobuf/proto"
+
+	"github.com/getlantern/flashlight/v7/apipb"
+)
+
+func TestInitWithSavedConfig(t *testing.T) {
+	conf := newTestConfig()
+	withTempConfigFile(t, conf, false, func(tmpfile *os.File) {
+		Init("", false)
+		existing, _ := GetConfig(0)
+
+		want := fmt.Sprintf("%+v", conf)
+		got := fmt.Sprintf("%+v", existing)
+		assert.Equal(t, want, got, "failed to read existing config file")
+	})
+
+	resetConfig()
+}
+
+func TestNotifyOnConfig(t *testing.T) {
+	conf := newTestConfig()
+	withTempConfigFile(t, conf, false, func(tmpfile *os.File) {
+		called := make(chan struct{}, 1)
+		OnConfigChange(func(old, new *ProxyConfig) {
+			called <- struct{}{}
+		})
+
+		Init("", false)
+		select {
+		case <-called:
+			t.Log("received existing config notification")
+		case <-time.After(time.Second):
+			assert.Fail(t, "timeout waiting for existing config notification")
+		}
+
+		_config.SetConfig(newTestConfig())
+
+		select {
+		case <-called:
+			t.Log("recieved config change notification")
+		case <-time.After(time.Second):
+			assert.Fail(t, "timeout waiting for config change notification")
+		}
+	})
+
+	resetConfig()
+}
+
+func TestInvalidFile(t *testing.T) {
+	withTempConfigFile(t, nil, false, func(tmpfile *os.File) {
+		tmpfile.WriteString("real-list-of-lantern-ips: https://youtu.be/dQw4w9WgXcQ?t=85")
+		tmpfile.Sync()
+
+		_, err := readExistingConfig(tmpfile.Name(), false)
+		assert.Error(t, err, "should get error if config file is invalid")
+	})
+}
+
+func TestReadObfuscatedConfig(t *testing.T) {
+	conf := newTestConfig()
+	withTempConfigFile(t, conf, true, func(tmpfile *os.File) {
+		fileConf, err := readExistingConfig(tmpfile.Name(), true)
+		assert.NoError(t, err, "unable to read obfuscated config file")
+
+		want := fmt.Sprintf("%+v", conf)
+		got := fmt.Sprintf("%+v", fileConf)
+		assert.Equal(t, want, got, "obfuscated config file doesn't match")
+	})
+}
+
+func TestSaveObfuscatedConfig(t *testing.T) {
+	withTempConfigFile(t, nil, false, func(tmpfile *os.File) {
+		tmpfile.Close()
+
+		conf := newTestConfig()
+		err := saveConfig(tmpfile.Name(), conf, true)
+		require.NoError(t, err, "unable to save obfuscated config file")
+
+		file, err := os.Open(tmpfile.Name())
+		require.NoError(t, err, "unable to open obfuscated config file")
+		defer file.Close()
+
+		reader := rot13.NewReader(file)
+		buf, err := io.ReadAll(reader)
+		require.NoError(t, err, "unable to read obfuscated config file")
+
+		fileConf := &ProxyConfig{}
+		assert.NoError(t, proto.Unmarshal(buf, fileConf), "unable to unmarshal obfuscated config file")
+
+		want := fmt.Sprintf("%+v", conf)
+		got := fmt.Sprintf("%+v", fileConf)
+		assert.Equal(t, want, got, "obfuscated config file doesn't match")
+	})
+}
+
+func newTestConfig() *ProxyConfig {
+	return &ProxyConfig{
+		Country: "Mars",
+		Ip:      "109.117.115.107",
+		Proxy:   &apipb.ConfigResponse_Proxy{},
+	}
+}
+
+func withTempConfigFile(t *testing.T, conf *ProxyConfig, obfuscate bool, f func(*os.File)) {
+	tmpfile, err := os.OpenFile(defaultConfigFilename, os.O_CREATE|os.O_TRUNC|os.O_RDWR, 0644)
+	require.NoError(t, err, "couldn't create temp file")
+	defer func() { // clean up
+		tmpfile.Close()
+		os.Remove(tmpfile.Name())
+	}()
+
+	if conf != nil {
+		buf, _ := proto.Marshal(conf)
+
+		var writer io.Writer = tmpfile
+		if obfuscate {
+			writer = rot13.NewWriter(tmpfile)
+		}
+
+		_, err := writer.Write(buf)
+		require.NoError(t, err, "unable to write to test config file")
+
+		tmpfile.Sync()
+	}
+
+	f(tmpfile)
+}
+
+func resetConfig() {
+	_config.mu.Lock()
+	_config.config.Reset()
+	_config.filePath = ""
+	_config.obfuscate = false
+	_config.listeners = nil
+	_config.mu.Unlock()
+}

services/bypass.go

@@ -58,9 +58,8 @@ type bypassService struct {
 	mxProxies sync.Mutex
 	// done is closed to notify the proxy bypass goroutines to stop.
 	done chan struct{}
-	// stopped is used to signal that the bypass service has been stopped. Once stopped, the service
-	// will not start any new proxy bypass goroutines.
-	stopped *atomic.Bool
+	// running is used to signal that the bypass service is running.
+	running *atomic.Bool
 }
 
 // StartBypassService sends periodic traffic to the bypass server. The client periodically sends
@@ -75,7 +74,7 @@ func StartBypassService(
 		infos:   make(map[string]*commonconfig.ProxyConfig),
 		proxies: make([]*proxy, 0),
 		done:    make(chan struct{}),
-		stopped: atomic.NewBool(false),
+		running: atomic.NewBool(true),
 	}
 
 	logger.Debug("Starting bypass service")
@@ -128,7 +127,7 @@ func (b *bypassService) onProxies(
 // Reset resets the bypass service by stopping all existing bypass proxy goroutines if
 // bypassService is still running. It returns true if bypassService was reset successfully.
 func (b *bypassService) Reset() bool {
-	if b.stopped.Load() {
+	if !b.running.Load() {
 		return false
 	}
 
@@ -143,7 +142,7 @@ func (b *bypassService) Reset() bool {
 }
 
 func (b *bypassService) Stop() {
-	if b.stopped.CompareAndSwap(false, true) {
+	if b.running.CompareAndSwap(true, false) {
 		close(b.done)
 	}
 }

services/config.go

@@ -2,7 +2,6 @@ package services
 
 import (
 	"bytes"
-	"compress/gzip"
 	"errors"
 	"fmt"
 	"io"
@@ -59,7 +58,8 @@ type configService struct {
 
 	done    chan struct{}
 	running bool
-	mu      sync.Mutex
+	// runningMu is used to protect the running field.
+	runningMu sync.Mutex
 }
 
 // ConfigHandler handles updating and retrieving the client config.
@@ -77,8 +77,8 @@ var (
 // StartConfigService starts a new config service with the given options and returns a func to stop
 // it. It will return an error if opts.OriginURL, opts.Rt, or opts.OnConfig are nil.
 func StartConfigService(handler ConfigHandler, opts *ConfigOptions) (StopFn, error) {
-	_configService.mu.Lock()
-	defer _configService.mu.Unlock()
+	_configService.runningMu.Lock()
+	defer _configService.runningMu.Unlock()
 
 	if _configService.running {
 		return _configService.Stop, nil
@@ -138,8 +138,8 @@ func StartConfigService(handler ConfigHandler, opts *ConfigOptions) (StopFn, err
 }
 
 func (cs *configService) Stop() {
-	cs.mu.Lock()
-	defer cs.mu.Unlock()
+	cs.runningMu.Lock()
+	defer cs.runningMu.Unlock()
 
 	if !cs.running {
 		return
@@ -158,7 +158,7 @@ func (cs *configService) fetchConfig() (int64, error) {
 
 	newConf, sleep, err := cs.fetch()
 	if err != nil {
-		logger.Errorf("configservice: Failed to fetch config: %w", err)
+		logger.Errorf("configservice: Failed to fetch config: %v", err)
 		return 0, op.FailIf(err)
 	}
 
@@ -201,18 +201,11 @@ func (cs *configService) fetch() (*apipb.ConfigResponse, int64, error) {
 	}
 	defer resp.Close()
 
-	gzReader, err := gzip.NewReader(resp)
-	if err != nil {
-		return nil, sleep, fmt.Errorf("unable to open gzip reader: %w", err)
-	}
-
-	configBytes, err := io.ReadAll(gzReader)
+	configBytes, err := io.ReadAll(resp)
 	if err != nil {
 		return nil, 0, fmt.Errorf("unable to read config response: %w", err)
 	}
 
-	gzReader.Close()
-
 	newConf := &apipb.ConfigResponse{}
 	if err = proto.Unmarshal(configBytes, newConf); err != nil {
 		return nil, 0, fmt.Errorf("unable to unmarshal config: %w", err)

services/service.go

@@ -24,13 +24,17 @@ func callRandomly(
 	jitter time.Duration,
 	done <-chan struct{},
 ) {
-	jitterInt := jitter.Nanoseconds()
-	intervalInt := interval.Nanoseconds()
+	jitterInt := jitter.Milliseconds()
+	intervalInt := interval.Milliseconds()
 
 	// calculate sleep time
 	sleep := func(extraDelay time.Duration) <-chan time.Time {
-		delay := mrand.Int64N(2*jitterInt) + intervalInt - jitterInt
-		delayDuration := time.Duration(delay) + extraDelay
+		delay := intervalInt
+		if jitterInt > 0 {
+			delay += mrand.Int64N(2*jitterInt) - jitterInt
+		}
+
+		delayDuration := time.Duration(delay)*time.Millisecond + extraDelay
 		logger.Debugf("Next run in %v", delayDuration)
 		return time.After(delayDuration)
 	}