Remove version-checking from client-side

Because the server now sends a boolean indicating whether the local version is up-to-date, we simply validate the server's response and remove any version-checking code from the client-side. The client now only checks for well-formed versions before printing, if the local version is not up-to-date.

Author: Amy Tai

sql/updates/1.0.0--1.0.1-dev.sql

@@ -0,0 +1 @@
+ DROP FUNCTION IF EXISTS _timescaledb_internal.get_version();

sql/version.sql

@@ -10,9 +10,5 @@ CREATE OR REPLACE FUNCTION _timescaledb_internal.get_os_info()
     RETURNS TABLE(sysname TEXT, version TEXT, release TEXT)
     AS '@MODULE_PATHNAME@', 'ts_get_os_info' LANGUAGE C IMMUTABLE STRICT PARALLEL SAFE;
 
-CREATE OR REPLACE FUNCTION _timescaledb_internal.get_version()
-    RETURNS TABLE(major INTEGER, minor INTEGER, patch INTEGER, modtag TEXT)
-    AS '@MODULE_PATHNAME@', 'ts_version_get_info' LANGUAGE C IMMUTABLE STRICT PARALLEL SAFE;
-
 CREATE OR REPLACE FUNCTION get_telemetry_report() RETURNS TEXT
     AS '@MODULE_PATHNAME@', 'ts_get_telemetry_report' LANGUAGE C IMMUTABLE STRICT PARALLEL SAFE;

src/telemetry/telemetry.c

@@ -24,6 +24,7 @@
 #include "net/http.h"
 
 #define TS_VERSION_JSON_FIELD "current_timescaledb_version"
+#define TS_IS_UPTODATE_JSON_FIELD "is_up_to_date"
 
 /*  HTTP request details */
 #define TIMESCALE_TYPE	"application/json"
@@ -49,9 +50,30 @@
 
 static const char *related_extensions[] = {PG_PROMETHEUS, POSTGIS};
 
+static bool
+char_in_valid_version_digits(const char c)
+{
+	switch (c)
+	{
+		case '.':
+		case '-':
+			return true;
+		default:
+			return false;
+	}
+}
+
+/*
+ * Makes sure the server version string is less than MAX_VERSION_STR_LEN
+ * chars, and all digits are "valid". Valid chars are either
+ * alphanumeric or in the array valid_version_digits above.
+ *
+ * Returns false if either of these conditions are false.
+ */
 bool
-telemetry_parse_version(const char *json, VersionInfo *installed_version, VersionResult *result)
+validate_server_version(const char *json, VersionResult *result)
 {
+	int			i;
 	Datum		version = DirectFunctionCall2(json_object_field_text,
 											  CStringGetTextDatum(json),
 											  PointerGetDatum(cstring_to_text(TS_VERSION_JSON_FIELD)));
@@ -60,26 +82,26 @@ telemetry_parse_version(const char *json, VersionInfo *installed_version, Versio
 
 	result->versionstr = text_to_cstring(DatumGetTextPP(version));
 
-	result->is_up_to_date = false;
-
 	if (result->versionstr == NULL)
 	{
 		result->errhint = "no version string in response";
 		return false;
 	}
 
-	/*
-	 * Now parse the version string. We expect format to be
-	 * XX.XX.XX-<prerelease_tag>, and if not, we error out
-	 */
-	if (!version_parse(result->versionstr, &result->vinfo))
+	if (strlen(result->versionstr) > MAX_VERSION_STR_LEN)
 	{
-		result->errhint = psprintf("parsing failed for version string \"%s\"", result->versionstr);
+		result->errhint = "version string is too long";
 		return false;
 	}
 
-	if (version_cmp(installed_version, &result->vinfo) >= 0)
-		result->is_up_to_date = true;
+	for (i = 0; i < strlen(result->versionstr); i++)
+	{
+		if (!isalpha(result->versionstr[i]) && !isdigit(result->versionstr[i]) && !char_in_valid_version_digits(result->versionstr[i]))
+		{
+			result->errhint = "version string has invalid characters";
+			return false;
+		}
+	}
 
 	return true;
 }
@@ -89,29 +111,31 @@ telemetry_parse_version(const char *json, VersionInfo *installed_version, Versio
  * called "current_timescaledb_version". Check this against the local
  * version, and notify the user if it is behind.
  */
-static bool
+static void
 process_response(const char *json)
 {
-	VersionInfo installed_version;
 	VersionResult result;
+	bool		is_uptodate = DatumGetBool(DirectFunctionCall2(texteq,
+															   DirectFunctionCall2(json_object_field_text,
+																				   CStringGetTextDatum(json),
+																				   PointerGetDatum(cstring_to_text(TS_IS_UPTODATE_JSON_FIELD))),
+															   PointerGetDatum(cstring_to_text("true"))));
 
-	version_get_info(&installed_version);
-
-	if (!telemetry_parse_version(json, &installed_version, &result))
-	{
-		elog(WARNING, "could not get TimescaleDB version from server response: %s", result.errhint);
-		return false;
-	}
-
-	if (result.is_up_to_date)
+	if (is_uptodate)
 		elog(NOTICE, "the \"%s\" extension is up-to-date", EXTENSION_NAME);
 	else
+	{
+		if (!validate_server_version(json, &result))
+		{
+			elog(WARNING, "server did not return a valid TimescaleDB version: %s", result.errhint);
+			return;
+		}
+
 		ereport(LOG,
 				(errmsg("the \"%s\" extension is not up-to-date", EXTENSION_NAME),
 				 errhint("The most up-to-date version is %s, the installed version is %s",
 						 result.versionstr, TIMESCALEDB_VERSION_MOD)));
-
-	return true;
+	}
 }
 
 static char *

src/telemetry/telemetry.h

@@ -21,17 +21,17 @@
 #define TELEMETRY_HOST "telemetry.timescale.com"
 #define TELEMETRY_PATH "/v1/metrics"
 
+#define	MAX_VERSION_STR_LEN	128
+
 typedef struct VersionResult
 {
-	VersionInfo vinfo;
 	const char *versionstr;
-	bool		is_up_to_date;
 	const char *errhint;
 } VersionResult;
 
 HttpRequest *build_version_request(const char *host, const char *path);
 Connection *telemetry_connect(const char *host, const char *service);
-bool		telemetry_parse_version(const char *json, VersionInfo *vinfo, VersionResult *result);
+bool		validate_server_version(const char *json, VersionResult *result);
 
 /*
  *	This function is intended as the main function for a BGW.

src/version.c

@@ -20,140 +20,6 @@
 #define STR_EXPAND(x) #x
 #define STR(x) STR_EXPAND(x)
 
-void
-version_get_info(VersionInfo *vinfo)
-{
-	memset(vinfo, 0, sizeof(VersionInfo));
-	vinfo->version[0] = strtol(TIMESCALEDB_MAJOR_VERSION, NULL, 10);
-	vinfo->version[1] = strtol(TIMESCALEDB_MINOR_VERSION, NULL, 10);
-	vinfo->version[2] = strtol(TIMESCALEDB_PATCH_VERSION, NULL, 10);
-
-	if (strlen(TIMESCALEDB_MOD_VERSION) > 0)
-	{
-		StrNCpy(vinfo->version_mod, TIMESCALEDB_MOD_VERSION, sizeof(vinfo->version_mod));
-		vinfo->has_version_mod = true;
-	}
-}
-
-/*
- * Compare two versions.
- *
- * It returns an integer less than, equal to, or greater than zero if version
- * v1 is found, respectively, to be less than, to match, or be greater than
- * version v2.
- */
-int
-version_cmp(VersionInfo *v1, VersionInfo *v2)
-{
-	int			i;
-
-	for (i = 0; i < 3; i++)
-	{
-		if (v1->version[i] > v2->version[i])
-			return 1;
-
-		if (v1->version[i] < v2->version[i])
-			return -1;
-	}
-
-	/*
-	 * Note that the version mod signifies a pre-release version, so having a
-	 * version mod is "less" than not having one with otherwise identical
-	 * versions
-	 */
-	if (v1->has_version_mod && !v2->has_version_mod)
-		return -1;
-
-	if (!v1->has_version_mod && v2->has_version_mod)
-		return 1;
-
-	/* Compare the version mod lexicographically */
-	if (v1->has_version_mod && v2->has_version_mod)
-		return strncmp(v1->version_mod, v2->version_mod, sizeof(v1->version_mod));
-
-	return 0;
-}
-
-bool
-version_parse(const char *version, VersionInfo *result)
-{
-	size_t		version_len = strlen(version);
-	int			fields_parsed = 0;
-	int			i = 0;
-	int			parse_length[4] = {0};
-
-	memset(result, 0, sizeof(VersionInfo));
-
-	/*
-	 * a version is a string of the form
-	 *
-	 * <number>[.<number>[.<number>[-<string>]]]
-	 *
-	 * this corresponds to the format-string
-	 *
-	 * "%lu.%lu.%lu-%<VERSION_INFO_LEN>s"
-	 *
-	 * after parsing each field in the version-string, we output the number of
-	 * bytes currently parsed using %n, a version-string is valid if all of
-	 * the bytes of the string were parsable using the above grammar.
-	 */
-	fields_parsed = sscanf(version, "%lu%n.%lu%n.%lu%n-%" STR(VERSION_INFO_LEN) "s%n",
-						   &result->version[0], &parse_length[0],
-						   &result->version[1], &parse_length[1],
-						   &result->version[2], &parse_length[2],
-						   result->version_mod, &parse_length[3]);
-
-	/*
-	 * sscanf is allowed to return EOF if no parses succeed, so make sure
-	 * fields_parsed is between 1 and 4;
-	 */
-	if (fields_parsed <= 0 || fields_parsed > 4)
-		return false;
-
-	result->has_version_mod = fields_parsed > 3;
-
-	result->version_mod[VERSION_INFO_LEN - 1] = '\0';
-
-	for (i = 0; i < VERSION_INFO_LEN; i++)
-		if (!isprint(result->version_mod[i]))
-			result->version_mod[i] = '\0';
-
-	return ((size_t) parse_length[fields_parsed - 1]) == version_len;
-}
-
-TS_FUNCTION_INFO_V1(ts_version_get_info);
-
-Datum
-ts_version_get_info(PG_FUNCTION_ARGS)
-{
-	VersionInfo info;
-	TupleDesc	tupdesc;
-	Datum		values[4];
-	bool		nulls[4] = {false};
-	HeapTuple	tuple;
-
-	version_get_info(&info);
-
-	if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
-		ereport(ERROR,
-				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-				 errmsg("function returning record called in context "
-						"that cannot accept type record")));
-
-	values[0] = Int32GetDatum((int32) info.version[0]);
-	values[1] = Int32GetDatum((int32) info.version[1]);
-	values[2] = Int32GetDatum((int32) info.version[2]);
-
-	if (info.has_version_mod)
-		values[3] = CStringGetTextDatum(info.version_mod);
-	else
-		nulls[3] = true;
-
-	tuple = heap_form_tuple(tupdesc, values, nulls);
-
-	return HeapTupleGetDatum(tuple);
-}
-
 const char *git_commit = STR(EXT_GIT_COMMIT);
 
 TS_FUNCTION_INFO_V1(ts_get_git_commit);

src/version.h

@@ -10,14 +10,6 @@
 #include <postgres.h>
 
 #define VERSION_INFO_LEN 128
-#define VERSION_PARTS 3
-
-typedef struct VersionInfo
-{
-	long		version[VERSION_PARTS];
-	char		version_mod[VERSION_INFO_LEN];
-	bool		has_version_mod;
-} VersionInfo;
 
 typedef struct VersionOSInfo
 {
@@ -26,10 +18,6 @@ typedef struct VersionOSInfo
 	char		release[VERSION_INFO_LEN];
 } VersionOSInfo;
 
-extern void version_get_info(VersionInfo *vinfo);
-extern int	version_cmp(VersionInfo *v1, VersionInfo *v2);
-extern bool version_parse(const char *version, VersionInfo *result);
-
 extern bool version_get_os_info(VersionOSInfo *info);
 
 #endif							/* TIMESCALEDB_VERSION_H */