From 06a7453984adca0b34e62421a1baa8fe54b0d7bb Mon Sep 17 00:00:00 2001
From: "kali kaneko (leap communications)" <kali@leap.se>
Date: Mon, 22 Jun 2020 22:00:23 +0200
Subject: [PATCH] [refactor] several simplifications after review
- simplify notification routine (we dont need no rejected action). we
just check every hour, as in the original code.
- open links directly from Qt
- rename some global variables to make them less cryptic
- move cleanup function to the same module that created them
---
go.mod | 2 ++
go.sum | 4 ++++
gui/backend.go | 5 -----
gui/handlers.cpp | 10 ----------
gui/handlers.h | 2 --
gui/qml/DonateDialog.qml | 7 +------
gui/qml/main.qml | 2 +-
pkg/backend/api.go | 18 ++----------------
pkg/backend/callbacks.go | 20 ++++++++++----------
pkg/backend/cleanup.go | 13 +++----------
pkg/backend/donate.go | 31 +++++++++++++++++--------------
pkg/backend/status.go | 11 +++++++----
pkg/vpn/main.go | 23 +++++++++++++++++++----
13 files changed, 66 insertions(+), 82 deletions(-)
diff --git a/go.mod b/go.mod
index b89aa45f..7b6e6150 100644
--- a/go.mod
+++ b/go.mod
@@ -11,12 +11,14 @@ require (
github.com/agl/ed25519 v0.0.0-20170116200512-5312a6153412 // indirect
github.com/apparentlymart/go-openvpn-mgmt v0.0.0-20161009010951-9a305aecd7f2
github.com/dchest/siphash v1.2.1 // indirect
+ github.com/godoctor/godoctor v0.0.0-20181123222458-69df17f3a6f6 // indirect
github.com/jmshal/go-locale v0.0.0-20190124211249-eb00fb25cc61
github.com/kardianos/osext v0.0.0-20190222173326-2bc1f35cddc0 // indirect
github.com/keybase/go-ps v0.0.0-20190827175125-91aafc93ba19
github.com/rakyll/statik v0.1.7
github.com/sevlyar/go-daemon v0.1.5
github.com/stretchr/testify v1.3.0 // indirect
+ github.com/willf/bitset v1.1.10 // indirect
golang.org/x/crypto v0.0.0-20191105034135-c7e5f84aec59 // indirect
golang.org/x/sys v0.0.0-20200212091648-12a6c2dcc1e4
golang.org/x/text v0.3.2
diff --git a/go.sum b/go.sum
index b449a895..0ef2264b 100644
--- a/go.sum
+++ b/go.sum
@@ -16,6 +16,8 @@ github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/dchest/siphash v1.2.1 h1:4cLinnzVJDKxTCl9B01807Yiy+W7ZzVHj/KIroQRvT4=
github.com/dchest/siphash v1.2.1/go.mod h1:q+IRvb2gOSrUnYoPqHiyHXS0FOBBOdl6tONBlVnOnt4=
+github.com/godoctor/godoctor v0.0.0-20181123222458-69df17f3a6f6 h1:iRn4qXDcqlb2sFqTdyTdYKjaPoLka7tvcyF+FZA9/qw=
+github.com/godoctor/godoctor v0.0.0-20181123222458-69df17f3a6f6/go.mod h1:+tyhT8jBF8E0XvdlSXOSL7Iko7DlNiongHq3q+wcsPs=
github.com/jmshal/go-locale v0.0.0-20190124211249-eb00fb25cc61 h1:9vsXCXRCUb82jJKv4O+R8Hyo4oPJsOjVwT0pWvHgeyc=
github.com/jmshal/go-locale v0.0.0-20190124211249-eb00fb25cc61/go.mod h1:+Ny9b1U6p4zX0L9w+k3hSkz3puupLFP14Mion+rGNF8=
github.com/kardianos/osext v0.0.0-20190222173326-2bc1f35cddc0 h1:iQTw/8FWTuc7uiaSepXwyf3o52HaUYcV+Tu66S3F5GA=
@@ -31,6 +33,8 @@ github.com/sevlyar/go-daemon v0.1.5/go.mod h1:6dJpPatBT9eUwM5VCw9Bt6CdX9Tk6UWvhW
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
github.com/stretchr/testify v1.3.0 h1:TivCn/peBQ7UY8ooIcPgZFpTNSz0Q2U6UrFlUfqbe0Q=
github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
+github.com/willf/bitset v1.1.10 h1:NotGKqX0KwQ72NUzqrjZq5ipPNDQex9lo3WpaS8L2sc=
+github.com/willf/bitset v1.1.10/go.mod h1:RjeCKbqT1RxIR/KWY6phxZiaY1IyutSBfGjNPySAYV4=
github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
diff --git a/gui/backend.go b/gui/backend.go
index 92cda6e2..f7816ee8 100644
--- a/gui/backend.go
+++ b/gui/backend.go
@@ -37,11 +37,6 @@ func DonateAccepted() {
backend.DonateAccepted()
}
-//export DonateRejected
-func DonateRejected() {
- backend.DonateRejected()
-}
-
//export SubscribeToEvent
func SubscribeToEvent(event string, f unsafe.Pointer) {
backend.SubscribeToEvent(event, f)
diff --git a/gui/handlers.cpp b/gui/handlers.cpp
index 39a57469..9ce268d2 100644
--- a/gui/handlers.cpp
+++ b/gui/handlers.cpp
@@ -30,16 +30,6 @@ void Backend::donateAccepted()
DonateAccepted();
}
-void Backend::donateRejected()
-{
- DonateRejected();
-}
-
-void Backend::openURL(QString link)
-{
- QDesktopServices::openUrl(QUrl(link));
-}
-
void Backend::quit()
{
Quit();
diff --git a/gui/handlers.h b/gui/handlers.h
index e65fd5eb..0dc4c1ea 100644
--- a/gui/handlers.h
+++ b/gui/handlers.h
@@ -34,8 +34,6 @@ public slots:
void switchOff();
void unblock();
void donateAccepted();
- void donateRejected();
- void openURL(QString link);
void quit();
};
diff --git a/gui/qml/DonateDialog.qml b/gui/qml/DonateDialog.qml
index fd27a6bf..de7ab5ba 100644
--- a/gui/qml/DonateDialog.qml
+++ b/gui/qml/DonateDialog.qml
@@ -16,13 +16,8 @@ MessageDialog {
onAccepted: {
if (backend) {
- backend.openURL(ctx.donateURL)
+ Qt.openUrlExternally(ctx.donateURL)
backend.donateAccepted()
}
}
- onRejected: {
- if (backend) {
- backend.donateRejected()
- }
- }
}
diff --git a/gui/qml/main.qml b/gui/qml/main.qml
index efcfef21..efe0111a 100644
--- a/gui/qml/main.qml
+++ b/gui/qml/main.qml
@@ -178,7 +178,7 @@ ApplicationWindow {
MenuItem {
text: qsTr("Help...")
- onTriggered: backend.openURL(ctx.helpURL)
+ onTriggered: Qt.openUrlExternally(ctx.helpURL)
}
MenuItem {
diff --git a/pkg/backend/api.go b/pkg/backend/api.go
index 74220eda..cf9ec5c8 100644
--- a/pkg/backend/api.go
+++ b/pkg/backend/api.go
@@ -6,7 +6,6 @@ import (
"C"
"fmt"
"log"
- "time"
"unsafe"
"0xacab.org/leap/bitmask-vpn/pkg/bitmask"
@@ -34,17 +33,13 @@ func Quit() {
ctx.cfg.SetUserStoppedVPN(true)
stopVPN()
}
- cleanupTempDirs()
+ cleanup()
}
func DonateAccepted() {
donateAccepted()
}
-func DonateRejected() {
- donateRejected()
-}
-
func SubscribeToEvent(event string, f unsafe.Pointer) {
subscribe(event, f)
}
@@ -53,17 +48,8 @@ func InitializeBitmaskContext() {
p := bitmask.GetConfiguredProvider()
initOnce.Do(func() { initializeContext(p.Provider, p.AppName) })
+ runDonationReminder()
go ctx.updateStatus()
-
- go func() {
- if needsDonationReminder() {
- // wait a bit before launching reminder
- timer := time.NewTimer(time.Minute * 5)
- <-timer.C
- showDonate()
- }
-
- }()
}
func RefreshContext() *C.char {
diff --git a/pkg/backend/callbacks.go b/pkg/backend/callbacks.go
index f3bb39fb..5fb8642d 100644
--- a/pkg/backend/callbacks.go
+++ b/pkg/backend/callbacks.go
@@ -19,11 +19,11 @@ import (
// }
import "C"
-/* callbacks into C-land */
+/* callbacks into C-land. We keep a registry, and protect its updates with a mutex. */
+
+var callbacks = make(map[string](*[0]byte))
+var callbackMutex sync.Mutex
-var mut sync.Mutex
-var stmut sync.Mutex
-var cbs = make(map[string](*[0]byte))
var initOnce sync.Once
// Events are just a enumeration of all the posible events that C functions can
@@ -38,23 +38,23 @@ const OnStatusChanged string = "OnStatusChanged"
// subscribe registers a callback from C-land.
// This callback needs to be passed as a void* C function pointer.
func subscribe(event string, fp unsafe.Pointer) {
- mut.Lock()
- defer mut.Unlock()
+ callbackMutex.Lock()
+ defer callbackMutex.Unlock()
e := &Events{}
v := reflect.Indirect(reflect.ValueOf(&e))
hf := v.Elem().FieldByName(event)
if reflect.ValueOf(hf).IsZero() {
fmt.Println("ERROR: not a valid event:", event)
} else {
- cbs[event] = (*[0]byte)(fp)
+ callbacks[event] = (*[0]byte)(fp)
}
}
// trigger fires a callback from C-land.
func trigger(event string) {
- mut.Lock()
- defer mut.Unlock()
- cb := cbs[event]
+ callbackMutex.Lock()
+ defer callbackMutex.Unlock()
+ cb := callbacks[event]
if cb != nil {
C._do_callback(cb)
} else {
diff --git a/pkg/backend/cleanup.go b/pkg/backend/cleanup.go
index 77c55e69..16f36e47 100644
--- a/pkg/backend/cleanup.go
+++ b/pkg/backend/cleanup.go
@@ -1,16 +1,9 @@
package backend
import (
- "log"
- "os"
- "path"
- "path/filepath"
+ "0xacab.org/leap/bitmask-vpn/pkg/vpn"
)
-func cleanupTempDirs() {
- dirs, _ := filepath.Glob(path.Join(os.TempDir(), "leap-*"))
- for _, d := range dirs {
- log.Println("removing temp dir:", d)
- os.RemoveAll(d)
- }
+func cleanup() {
+ vpn.Cleanup()
}
diff --git a/pkg/backend/donate.go b/pkg/backend/donate.go
index 608128f4..20d56130 100644
--- a/pkg/backend/donate.go
+++ b/pkg/backend/donate.go
@@ -1,12 +1,24 @@
package backend
import (
- "log"
"time"
"0xacab.org/leap/bitmask-vpn/pkg/config"
)
+// runDonationReminder checks every hour if we need to show the reminder,
+// and trigger the launching of the dialog if needed.
+func runDonationReminder() {
+ go func() {
+ for {
+ time.Sleep(time.Hour)
+ if needsDonationReminder() {
+ showDonate()
+ }
+ }
+ }()
+}
+
func wantDonations() bool {
if config.AskForDonations == "true" {
return true
@@ -19,25 +31,16 @@ func needsDonationReminder() bool {
}
func donateAccepted() {
- stmut.Lock()
- defer stmut.Unlock()
+ statusMutex.Lock()
+ defer statusMutex.Unlock()
ctx.DonateDialog = false
- log.Println("marking as donated")
ctx.cfg.SetDonated()
go trigger(OnStatusChanged)
}
-func donateRejected() {
- timer := time.NewTimer(time.Hour)
- go func() {
- <-timer.C
- showDonate()
- }()
-}
-
func showDonate() {
- stmut.Lock()
- defer stmut.Unlock()
+ statusMutex.Lock()
+ defer statusMutex.Unlock()
ctx.DonateDialog = true
ctx.cfg.SetLastReminded()
go trigger(OnStatusChanged)
diff --git a/pkg/backend/status.go b/pkg/backend/status.go
index 2e9c282d..2bfb52da 100644
--- a/pkg/backend/status.go
+++ b/pkg/backend/status.go
@@ -4,6 +4,7 @@ import (
"bytes"
"encoding/json"
"log"
+ "sync"
"0xacab.org/leap/bitmask-vpn/pkg/bitmask"
"0xacab.org/leap/bitmask-vpn/pkg/config"
@@ -17,6 +18,8 @@ const (
failedStr = "failed"
)
+var statusMutex sync.Mutex
+
// ctx will be our glorious global object.
// if we ever switch again to a provider-agnostic app, we should keep a map here.
var ctx *connectionCtx
@@ -42,8 +45,8 @@ type connectionCtx struct {
}
func (c connectionCtx) toJson() ([]byte, error) {
- stmut.Lock()
- defer stmut.Unlock()
+ statusMutex.Lock()
+ defer statusMutex.Unlock()
b, err := json.Marshal(c)
if err != nil {
log.Println(err)
@@ -69,8 +72,8 @@ func (c connectionCtx) updateStatus() {
}
func setStatus(st status) {
- stmut.Lock()
- defer stmut.Unlock()
+ statusMutex.Lock()
+ defer statusMutex.Unlock()
ctx.Status = st
go trigger(OnStatusChanged)
}
diff --git a/pkg/vpn/main.go b/pkg/vpn/main.go
index 9d59131b..f3c5b833 100644
--- a/pkg/vpn/main.go
+++ b/pkg/vpn/main.go
@@ -19,6 +19,8 @@ import (
"io/ioutil"
"log"
"os"
+ "path"
+ "path/filepath"
"0xacab.org/leap/bitmask-vpn/pkg/config"
"0xacab.org/leap/bitmask-vpn/pkg/vpn/bonafide"
@@ -52,10 +54,15 @@ func Init() (*Bitmask, error) {
b := Bitmask{tempdir, statusCh, nil, bonafide, launch, "", nil}
/*
- err = b.StopVPN()
- if err != nil {
- return nil, err
- }
+ TODO -- we still want to do this, since it resets the fw/vpn if running
+ from a previous one, but first we need to complete all the
+ system/helper checks that we can do. otherwise this times out with an
+ error that's captured badly as of today.
+
+ err = b.StopVPN()
+ if err != nil {
+ return nil, err
+ }
*/
err = ioutil.WriteFile(b.getCaCertPath(), config.CaCert, 0600)
@@ -87,3 +94,11 @@ func (b *Bitmask) Close() {
func (b *Bitmask) Version() (string, error) {
return "", nil
}
+
+func Cleanup() {
+ dirs, _ := filepath.Glob(path.Join(os.TempDir(), "leap-*"))
+ for _, d := range dirs {
+ log.Println("removing temp dir:", d)
+ os.RemoveAll(d)
+ }
+}
--
GitLab