Просмотр исходного кода

Fix remote_server_list field processing

- Correctly apply remoteServerListStatParams flags such as
  requestParamLogFlagAsBool

- No longer log any field sent in remote_server_list_stats

- Do not log extraneous fields including dial parameters for
  the tunnel delivering the persistent stats

- Explicitly implement backwards compatibility case

- Include device_region when recording remote server list
  stats
Rod Hynes 6 лет назад
Родитель
Сommit
8071bfeedb
2 измененных файлов с 35 добавлено и 13 удалено
  1. 32 13
      psiphon/server/api.go
  2. 3 0
      psiphon/serverApi.go

+ 32 - 13
psiphon/server/api.go

@@ -414,12 +414,13 @@ var statusRequestParams = append(
 	baseRequestParams...)
 
 var remoteServerListStatParams = []requestParamSpec{
-	{"session_id", isHexDigits, requestParamOptional},
-	{"propagation_channel_id", isHexDigits, requestParamOptional},
-	{"sponsor_id", isHexDigits, requestParamOptional},
-	{"client_version", isAnyString, requestParamOptional},
-	{"client_platform", isAnyString, requestParamOptional},
+	{"session_id", isHexDigits, 0},
+	{"propagation_channel_id", isHexDigits, 0},
+	{"sponsor_id", isHexDigits, 0},
+	{"client_version", isIntString, requestParamLogStringAsInt},
+	{"client_platform", isAnyString, 0},
 	{"client_build_rev", isAnyString, requestParamOptional},
+	{"device_region", isAnyString, requestParamOptional},
 	{"client_download_timestamp", isISO8601Date, 0},
 	{"tunneled", isBooleanFlag, requestParamOptional | requestParamLogFlagAsBool},
 	{"url", isAnyString, 0},
@@ -427,6 +428,22 @@ var remoteServerListStatParams = []requestParamSpec{
 	{"authenticated", isBooleanFlag, requestParamOptional | requestParamLogFlagAsBool},
 }
 
+// Backwards compatibility case: legacy clients do not include these fields in
+// the remote_server_list_stats entries. Use the values from the outer status
+// request as an approximation (these values reflect the client at persistent
+// stat shipping time, which may differ from the client at persistent stat
+// recording time). Note that all but client_build_rev and device_region are
+// required fields.
+var remoteServerListStatBackwardsCompatibilityParamNames = []string{
+	"session_id",
+	"propagation_channel_id",
+	"sponsor_id",
+	"client_version",
+	"client_platform",
+	"client_build_rev",
+	"device_region",
+}
+
 var failedTunnelStatParams = append(
 	[]requestParamSpec{
 		{"server_entry_tag", isAnyString, requestParamOptional},
@@ -520,23 +537,25 @@ func statusAPIRequestHandler(
 		}
 		for _, remoteServerListStat := range remoteServerListStats {
 
+			for _, name := range remoteServerListStatBackwardsCompatibilityParamNames {
+				if _, ok := remoteServerListStat[name]; !ok {
+					if field, ok := params[name]; ok {
+						remoteServerListStat[name] = field
+					}
+				}
+			}
+
 			err := validateRequestParams(support.Config, remoteServerListStat, remoteServerListStatParams)
 			if err != nil {
 				return nil, errors.Trace(err)
 			}
 
-			// remote_server_list defaults to using the common params from the
-			// outer statusRequestParams
 			remoteServerListFields := getRequestLogFields(
 				"remote_server_list",
 				geoIPData,
 				authorizedAccessTypes,
-				params,
-				statusRequestParams)
-
-			for name, value := range remoteServerListStat {
-				remoteServerListFields[name] = value
-			}
+				remoteServerListStat,
+				remoteServerListStatParams)
 
 			logQueue = append(logQueue, remoteServerListFields)
 		}

+ 3 - 0
psiphon/serverApi.go

@@ -621,6 +621,9 @@ func RecordRemoteServerListStat(
 	params["client_version"] = config.ClientVersion
 	params["client_platform"] = config.ClientPlatform
 	params["client_build_rev"] = buildinfo.GetBuildInfo().BuildRev
+	if config.DeviceRegion != "" {
+		params["device_region"] = config.DeviceRegion
+	}
 
 	params["client_download_timestamp"] = common.TruncateTimestampToHour(common.GetCurrentTimestamp())
 	tunneledStr := "0"