Skip to content

Commit 6545a90

Browse files
tglsfdctglsfdc
committed
Fix pg_restore's direct-to-database mode for standard_conforming_strings.
pg_backup_db.c contained a mini SQL lexer with which it tried to identify boundaries between SQL commands, but that code was not designed to cope with standard_conforming_strings, and would get the wrong answer if a backslash immediately precedes a closing single quote in such a string, as per report from Julian Mehnle. The bug only affects direct-to-database restores from archive files made with standard_conforming_strings = on. Rather than complicating the code some more to try to fix that, let's just rip it all out. The only reason it was needed was to cope with COPY data embedded into ordinary archive entries, which was a layout that was used only for about the first three weeks of the archive format's existence, and never in any production release of pg_dump. Instead, just rely on the archive file layout to tell us whether we're printing COPY data or not. This bug represents a data corruption hazard in all releases in which standard_conforming_strings can be turned on, ie 8.2 and later, so back-patch to all supported branches.
1 parent 0fe8150 commit 6545a90

File tree

4 files changed

+67
-386
lines changed

4 files changed

+67
-386
lines changed

‎src/bin/pg_dump/pg_backup_archiver.c

Lines changed: 18 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ static TocEntry *getTocEntryByDumpId(ArchiveHandle *AH, DumpId id);
117117
static void _moveBefore(ArchiveHandle *AH, TocEntry *pos, TocEntry *te);
118118
static int _discoverArchiveFormat(ArchiveHandle *AH);
119119

120+
static int RestoringToDB(ArchiveHandle *AH);
120121
static void dump_lo_buf(ArchiveHandle *AH);
121122
static void _write_msg(const char *modulename, const char *fmt, va_list ap);
122123
static void _die_horribly(ArchiveHandle *AH, const char *modulename, const char *fmt, va_list ap);
@@ -589,13 +590,7 @@ restore_toc_entry(ArchiveHandle *AH, TocEntry *te,
589590
}
590591

591592
/*
592-
* If we have a copy statement, use it. As of V1.3, these
593-
* are separate to allow easy import from withing a
594-
* database connection. Pre 1.3 archives can not use DB
595-
* connections and are sent to output only.
596-
*
597-
* For V1.3+, the table data MUST have a copy statement so
598-
* that we can go into appropriate mode with libpq.
593+
* If we have a copy statement, use it.
599594
*/
600595
if (te->copyStmt && strlen(te->copyStmt) > 0)
601596
{
@@ -605,7 +600,15 @@ restore_toc_entry(ArchiveHandle *AH, TocEntry *te,
605600

606601
(*AH->PrintTocDataPtr) (AH, te, ropt);
607602

608-
AH->writingCopyData = false;
603+
/*
604+
* Terminate COPY if needed.
605+
*/
606+
if (AH->writingCopyData)
607+
{
608+
if (RestoringToDB(AH))
609+
EndDBCopyMode(AH, te);
610+
AH->writingCopyData = false;
611+
}
609612

610613
/* close out the transaction started above */
611614
if (is_parallel && te->created)
@@ -1248,17 +1251,13 @@ ahprintf(ArchiveHandle *AH, const char *fmt,...)
12481251
{
12491252
char *p = NULL;
12501253
va_list ap;
1251-
int bSize = strlen(fmt) + 256; /* Should be enough */
1254+
int bSize = strlen(fmt) + 256; /* Usually enough */
12521255
int cnt = -1;
12531256

12541257
/*
12551258
* This is paranoid: deal with the possibility that vsnprintf is willing
1256-
* to ignore trailing null
1257-
*/
1258-
1259-
/*
1260-
* or returns > 0 even if string does not fit. It may be the case that it
1261-
* returns cnt = bufsize
1259+
* to ignore trailing null or returns > 0 even if string does not fit.
1260+
* It may be the case that it returns cnt = bufsize.
12621261
*/
12631262
while (cnt < 0 || cnt >= (bSize - 1))
12641263
{
@@ -1340,7 +1339,7 @@ dump_lo_buf(ArchiveHandle *AH)
13401339

13411340

13421341
/*
1343-
* Write buffer to the output file (usually stdout). This is user for
1342+
* Write buffer to the output file (usually stdout). This is used for
13441343
* outputting 'restore' scripts etc. It is even possible for an archive
13451344
* format to create a custom output routine to 'fake' a restore if it
13461345
* wants to generate a script (see TAR output).
@@ -1392,7 +1391,7 @@ ahwrite(const void *ptr, size_t size, size_t nmemb, ArchiveHandle *AH)
13921391
* connected then send it to the DB.
13931392
*/
13941393
if (RestoringToDB(AH))
1395-
return ExecuteSqlCommandBuf(AH, (void *) ptr, size * nmemb); /* Always 1, currently */
1394+
return ExecuteSqlCommandBuf(AH, (const char *) ptr, size * nmemb);
13961395
else
13971396
{
13981397
res = fwrite((void *) ptr, size, nmemb, AH->OF);
@@ -1985,9 +1984,6 @@ _allocAH(const char *FileSpec, const ArchiveFormat fmt,
19851984
AH->mode = mode;
19861985
AH->compression = compression;
19871986

1988-
AH->pgCopyBuf = createPQExpBuffer();
1989-
AH->sqlBuf = createPQExpBuffer();
1990-
19911987
/* Open stdout with no compression for AH output handle */
19921988
AH->gzOut = 0;
19931989
AH->OF = stdout;
@@ -4199,10 +4195,7 @@ CloneArchive(ArchiveHandle *AH)
41994195
die_horribly(AH, modulename, "out of memory\n");
42004196
memcpy(clone, AH, sizeof(ArchiveHandle));
42014197

4202-
/* Handle format-independent fields */
4203-
clone->pgCopyBuf = createPQExpBuffer();
4204-
clone->sqlBuf = createPQExpBuffer();
4205-
clone->sqlparse.tagBuf = NULL;
4198+
/* Handle format-independent fields ... none at the moment */
42064199

42074200
/* The clone will have its own connection, so disregard connection state */
42084201
clone->connection = NULL;
@@ -4235,11 +4228,7 @@ DeCloneArchive(ArchiveHandle *AH)
42354228
/* Clear format-specific state */
42364229
(AH->DeClonePtr) (AH);
42374230

4238-
/* Clear state allocated by CloneArchive */
4239-
destroyPQExpBuffer(AH->pgCopyBuf);
4240-
destroyPQExpBuffer(AH->sqlBuf);
4241-
if (AH->sqlparse.tagBuf)
4242-
destroyPQExpBuffer(AH->sqlparse.tagBuf);
4231+
/* Clear state allocated by CloneArchive ... none at the moment */
42434232

42444233
/* Clear any connection-local state */
42454234
if (AH->currUser)

‎src/bin/pg_dump/pg_backup_archiver.h

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -132,28 +132,6 @@ typedef void (*DeClonePtr) (struct _archiveHandle * AH);
132132

133133
typedef size_t (*CustomOutPtr) (struct _archiveHandle * AH, const void *buf, size_t len);
134134

135-
typedef enum
136-
{
137-
SQL_SCAN = 0, /* normal */
138-
SQL_IN_SQL_COMMENT, /* -- comment */
139-
SQL_IN_EXT_COMMENT, /* slash-star comment */
140-
SQL_IN_SINGLE_QUOTE, /* '...' literal */
141-
SQL_IN_E_QUOTE, /* E'...' literal */
142-
SQL_IN_DOUBLE_QUOTE, /* "..." identifier */
143-
SQL_IN_DOLLAR_TAG, /* possible dollar-quote starting tag */
144-
SQL_IN_DOLLAR_QUOTE /* body of dollar quote */
145-
} sqlparseState;
146-
147-
typedef struct
148-
{
149-
sqlparseState state; /* see above */
150-
char lastChar; /* preceding char, or '\0' initially */
151-
bool backSlash; /* next char is backslash quoted? */
152-
int braceDepth; /* parenthesis nesting depth */
153-
PQExpBuffer tagBuf; /* dollar quote tag (NULL if not created) */
154-
int minTagEndPos; /* first possible end position of $-quote */
155-
} sqlparseInfo;
156-
157135
typedef enum
158136
{
159137
STAGE_NONE = 0,
@@ -189,9 +167,6 @@ typedef struct _archiveHandle
189167
* Added V1.7 */
190168
ArchiveFormat format; /* Archive format */
191169

192-
sqlparseInfo sqlparse;
193-
PQExpBuffer sqlBuf;
194-
195170
time_t createDate; /* Date archive created */
196171

197172
/*
@@ -244,8 +219,6 @@ typedef struct _archiveHandle
244219
* required */
245220
bool writingCopyData; /* True when we are sending COPY data */
246221
bool pgCopyIn; /* Currently in libpq 'COPY IN' mode. */
247-
PQExpBuffer pgCopyBuf; /* Left-over data from incomplete lines in
248-
* COPY IN */
249222

250223
int loFd; /* BLOB fd */
251224
int writingBlob; /* Flag */

0 commit comments

Comments
 (0)