Correctly handle bad characters in the generated slot name

PJMODOS · PJMODOS · commit 55a5bc791184 · 2016-01-15T13:05:15.000+01:00
diff --git a/pglogical_functions.c b/pglogical_functions.c
@@ -1515,14 +1515,26 @@ static void
 gen_slot_name(Name slot_name, char *dbname, const char *provider_node,
 			  const char *subscription_name)
 {
+	char *cp;
+
 	memset(NameStr(*slot_name), 0, NAMEDATALEN);
 	snprintf(NameStr(*slot_name), NAMEDATALEN,
 			 "pgl_%s_%s_%s",
 			 shorten_hash(dbname, 16),
 			 shorten_hash(provider_node, 16),
 			 shorten_hash(subscription_name, 16));
-
 	NameStr(*slot_name)[NAMEDATALEN-1] = '\0';
+
+	/* Replace all the invalid characters in slot name with underscore. */
+	for (cp = NameStr(*slot_name); *cp; cp++)
+	{
+		if (!((*cp >= 'a' && *cp <= 'z')
+			  || (*cp >= '0' && *cp <= '9')
+			  || (*cp == '_')))
+		{
+			*cp = '_';
+		}
+	}
 }
 
 Datum
diff --git a/pglogical_node.c b/pglogical_node.c
@@ -101,6 +101,40 @@ typedef struct SubscriptionTuple
 #define Anum_sub_replication_sets	9
 #define Anum_sub_forward_origins	10
 
+/*
+ * We impose same validation rules as replication slot name validation does.
+ */
+static void
+validate_subscription_name(const char *name)
+{
+	const char *cp;
+
+	if (strlen(name) == 0)
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_NAME),
+				 errmsg("subscription  name \"%s\" is too short", name)));
+
+	if (strlen(name) >= NAMEDATALEN)
+		ereport(ERROR,
+				(errcode(ERRCODE_NAME_TOO_LONG),
+				 errmsg("subscription name \"%s\" is too long", name)));
+
+	for (cp = name; *cp; cp++)
+	{
+		if (!((*cp >= 'a' && *cp <= 'z')
+			  || (*cp >= '0' && *cp <= '9')
+			  || (*cp == '_')))
+		{
+			ereport(ERROR,
+					(errcode(ERRCODE_INVALID_NAME),
+					 errmsg("subscription name \"%s\" contains invalid character",
+							name),
+					 errhint("Subscription names may only contain lower case "
+							 "letters, numbers, and the underscore character.")));
+		}
+	}
+}
+
 /*
  * Add new node to catalog.
  */
@@ -597,6 +631,9 @@ create_subscription(PGLogicalSubscription *sub)
 	NameData	sub_name;
 	NameData	slot_name;
 
+	/* Validate the new subscription name. */
+	validate_subscription_name(sub->name);
+
 	if (get_subscription_by_name(sub->name, true) != NULL)
 		elog(ERROR, "subscription %s already exists", sub->name);
 
diff --git a/pglogical_sync.c b/pglogical_sync.c
@@ -20,6 +20,7 @@
 #include "miscadmin.h"
 
 #include "access/genam.h"
+#include "access/hash.h"
 #include "access/heapam.h"
 #include "access/skey.h"
 #include "access/stratnum.h"
@@ -794,8 +795,9 @@ pglogical_sync_main(Datum main_arg)
 										   copytable->relname);
 
 	initStringInfo(&slot_name);
-	appendStringInfo(&slot_name, "%s_%s", MySubscription->slot_name,
-					 shorten_hash(tablename, 8));
+	appendStringInfo(&slot_name, "%s_%08x", MySubscription->slot_name,
+					 DatumGetUInt32(hash_any((unsigned char *) tablename,
+											 strlen(tablename))));
 	MySubscription->slot_name = slot_name.data;
 
 	elog(LOG, "starting sync of table %s.%s for subscriber %s",
