Merge branch 'master' into inproxy-enhancements

psiphon/common/obfuscator/obfuscator_test.go

@@ -125,8 +125,8 @@ func TestObfuscator(t *testing.T) {
 	irregularLogFields = nil
 
 	_, err = NewServerObfuscator(config, clientIP, bytes.NewReader(preamble))
-	if err != nil {
-		t.Fatalf("NewServerObfuscator failed: %s", err)
+	if err == nil {
+		t.Fatalf("NewServerObfuscator unexpectedly succeeded")
 	}
 
 	duplicateClientID := irregularLogFields["duplicate_client_ip"]
@@ -540,11 +540,11 @@ func TestIrregularConnections(t *testing.T) {
 
 	irregularLogFields = nil
 
-	// Test: replayed prefixd connection with same IP
+	// Test: replayed prefixed connection with same IP
 	clientReader = WrapConnWithSkipReader(newConn(preamble))
 	_, err = NewServerObfuscator(config, clientIP, clientReader)
-	if err != nil {
-		t.Fatalf("NewServerObfuscator failed: %s", err)
+	if err == nil {
+		t.Fatalf("NewServerObfuscator unexpectedly succeeded")
 	}
 
 	duplicateClientID := irregularLogFields["duplicate_client_ip"]

psiphon/server/tunnelServer.go

@@ -1298,8 +1298,18 @@ func (sshServer *sshServer) getLoadStats() (
 		//   exact number of other _network connections_, even from the same
 		//   client.
 		//
-		// - For in-proxy tunnel protocols, the same GeoIP caveats
-		//   (see comments above) apply.
+		//   Futhermore, since client.Locks aren't held between the previous
+		//   loop and this one, it's also possible that the client's
+		//   clientGeoIPData was None in the previous loop and is now not
+		//   None. In this case, the regionStats may not be populated at all
+		//   for the client's current region; if so, the client is skipped.
+		//   This scenario can also result in a proximate undercount by one,
+		//   when the regionStats _is_ populated: this client was counted
+		//   under None, not the current client.peerGeoIPData.Country, so
+		//   the -1 subtracts some _other_ client from the populated regionStats.
+		//
+		// - For in-proxy protocols, the accepted proximate metric uses the
+		//   peer GeoIP, which represents the proxy, not the client.
 
 		stats := regionStats[client.peerGeoIPData.Country]["ALL"]
 
@@ -1316,6 +1326,15 @@ func (sshServer *sshServer) getLoadStats() (
 			}
 		}
 
+		// Handle the in-proxy None and None/not-None cases (and any other
+		// potential scenario where regionStats[client.clientGeoIPData.Country]
+		// may not be populated).
+		if client.clientGeoIPData.Country == GEOIP_UNKNOWN_VALUE ||
+			regionStats[client.clientGeoIPData.Country] == nil {
+			client.Unlock()
+			continue
+		}
+
 		stats = regionStats[client.clientGeoIPData.Country]["ALL"]
 
 		n = stats["established_clients"].(int64) - 1