On Thu, 2020-12-03 at 13:14 -0500, Bruce Momjian wrote:
> Andres objected (in a separate conversation) to forcing a binary-
> > format
> > value on a client that didn't ask for one. He suggested that we
> > mandate
> > that the data is ASCII-only (for both filename and content),
> > closing
> > the gap Michael raised[1]; and then just declare all values to be
> > text
> > format.
>
> How do we mandate that? Just mention it in the docs and C comments?
Attached a simple patch that enforces an all-ASCII restore point name
rather than documenting the possibility of non-ASCII text.
Regards,
Jeff Davis
[1]
https://www.postgresql.org/message-id/3679218.1602770626%40sss.pgh.pa.us
From b9381a62ab9616da3062827f84b72844748fd3fc Mon Sep 17 00:00:00 2001
From: Jeff Davis <jda...@postgresql.org>
Date: Tue, 15 Dec 2020 01:06:21 -0800
Subject: [PATCH] Enforce ASCII restore point names.
The TIMELINE_HISTORY replication command assumes that the file name
and contents are valid ASCII, but the timeline history file may
contain restore point names.
Discussion: https://postgr.es/m/20201203181456.gk20...@momjian.us
---
doc/src/sgml/protocol.sgml | 4 +---
src/backend/access/transam/xlogfuncs.c | 27 ++++++++++++++++++++++++++
2 files changed, 28 insertions(+), 3 deletions(-)
diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml
index 4899bacda7..b18a4bceac 100644
--- a/doc/src/sgml/protocol.sgml
+++ b/doc/src/sgml/protocol.sgml
@@ -1861,9 +1861,7 @@ The commands accepted in replication mode are:
<para>
Requests the server to send over the timeline history file for timeline
<replaceable class="parameter">tli</replaceable>. Server replies with a
- result set of a single row, containing two fields. While the fields
- are labeled as <type>text</type>, they effectively return raw bytes,
- with no encoding conversion:
+ result set of a single row, containing two fields:
</para>
<para>
diff --git a/src/backend/access/transam/xlogfuncs.c b/src/backend/access/transam/xlogfuncs.c
index 290658b22c..48daed56f6 100644
--- a/src/backend/access/transam/xlogfuncs.c
+++ b/src/backend/access/transam/xlogfuncs.c
@@ -44,6 +44,8 @@
static StringInfo label_file;
static StringInfo tblspc_map_file;
+static bool is_all_ascii(const char *str);
+
/*
* pg_start_backup: set up for taking an on-line backup dump
*
@@ -309,6 +311,16 @@ pg_create_restore_point(PG_FUNCTION_ARGS)
restore_name_str = text_to_cstring(restore_name);
+ /*
+ * The restore point name may be included in a timeline history file. The
+ * system assumes that the contents are ASCII, so enforce an all-ASCII
+ * restore point name.
+ */
+ if (!is_all_ascii(restore_name_str))
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("restore point name must be valid ASCII")));
+
if (strlen(restore_name_str) >= MAXFNAMELEN)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
@@ -784,3 +796,18 @@ pg_promote(PG_FUNCTION_ARGS)
(errmsg("server did not promote within %d seconds", wait_seconds)));
PG_RETURN_BOOL(false);
}
+
+/*
+ * Check a string to see if it is pure ASCII
+ */
+static bool
+is_all_ascii(const char *str)
+{
+ while (*str)
+ {
+ if (IS_HIGHBIT_SET(*str))
+ return false;
+ str++;
+ }
+ return true;
+}
--
2.17.1
