database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-10-16 17:07:43 +03:00
Code Activity

Fix pg_restore's misdesigned code for detecting archive file format.

Browse Source 
Despite the clear comments pointing out that the duplicative code
segments in ReadHead() and _discoverArchiveFormat() needed to be
in sync, they were not: the latter did not bother to apply any of
the sanity checks in the former.  We'd missed noticing this partly
because none of those checks would fail in scenarios we customarily
test, and partly because the oversight would be masked if both
segments execute, which they would in cases other than needing to
autodetect the format of a non-seekable stdin source.  However,
in a case meeting all these requirements --- for example, trying
to read a newer-than-supported archive format from non-seekable
stdin --- pg_restore missed applying the version check and would
likely dump core or otherwise misbehave.

The whole thing is silly anyway, because there seems little reason
to duplicate the logic beyond the one-line verification that the
file starts with "PGDMP".  There seems to have been an undocumented
assumption that multiple major formats (major enough to require
separate reader modules) would nonetheless share the first half-dozen
fields of the custom-format header.  This seems unlikely, so let's
fix it by just nuking the duplicate logic in _discoverArchiveFormat().

Also get rid of the pointless attempt to seek back to the start of
the file after successful autodetection.  That wastes cycles and
it means we have four behaviors to verify not two.

Per bug #16951 from Sergey Koposov.  This has been broken for
decades, so back-patch to all supported versions.

Discussion: https://postgr.es/m/16951-a4dd68cf0de23048@postgresql.org
This commit is contained in:
Tom Lane
2021-04-01 13:34:16 -04:00
parent 91e7c90329
commit ec03f2df17
3 changed files with 59 additions and 118 deletions
Download Patch File Download Diff File Expand all files Collapse all files

153
src/bin/pg_dump/pg_backup_archiver.c
View File

@@ -2071,6 +2071,7 @@ _discoverArchiveFormat(ArchiveHandle *AH)
if (AH->lookahead)
free(AH->lookahead);
AH->readHeader = 0;
AH->lookaheadSize = 512;
AH->lookahead = pg_malloc0(512);
AH->lookaheadLen = 0;
@@ -2142,62 +2143,9 @@ _discoverArchiveFormat(ArchiveHandle *AH)
if (strncmp(sig, "PGDMP", 5) == 0)
{
int byteread;
char vmaj,
vmin,
vrev;
/*
* Finish reading (most of) a custom-format header.
*
* NB: this code must agree with ReadHead().
*/
if ((byteread = fgetc(fh)) == EOF)
READ_ERROR_EXIT(fh);
vmaj = byteread;
if ((byteread = fgetc(fh)) == EOF)
READ_ERROR_EXIT(fh);
vmin = byteread;
/* Save these too... */
AH->lookahead[AH->lookaheadLen++] = vmaj;
AH->lookahead[AH->lookaheadLen++] = vmin;
/* Check header version; varies from V1.0 */
if (vmaj > 1 || (vmaj == 1 && vmin > 0)) /* Version > 1.0 */
{
if ((byteread = fgetc(fh)) == EOF)
READ_ERROR_EXIT(fh);
vrev = byteread;
AH->lookahead[AH->lookaheadLen++] = vrev;
}
else
vrev = 0;
AH->version = MAKE_ARCHIVE_VERSION(vmaj, vmin, vrev);
if ((AH->intSize = fgetc(fh)) == EOF)
READ_ERROR_EXIT(fh);
AH->lookahead[AH->lookaheadLen++] = AH->intSize;
if (AH->version >= K_VERS_1_7)
{
if ((AH->offSize = fgetc(fh)) == EOF)
READ_ERROR_EXIT(fh);
AH->lookahead[AH->lookaheadLen++] = AH->offSize;
}
else
AH->offSize = AH->intSize;
if ((byteread = fgetc(fh)) == EOF)
READ_ERROR_EXIT(fh);
AH->format = byteread;
AH->lookahead[AH->lookaheadLen++] = AH->format;
/* It's custom format, stop here */
AH->format = archCustom;
AH->readHeader = 1;
}
else
{
@@ -2234,22 +2182,15 @@ _discoverArchiveFormat(ArchiveHandle *AH)
AH->format = archTar;
}
/* If we can't seek, then mark the header as read */
if (fseeko(fh, 0, SEEK_SET) != 0)
{
/*
* NOTE: Formats that use the lookahead buffer can unset this in their
* Init routine.
*/
AH->readHeader = 1;
}
else
AH->lookaheadLen = 0; /* Don't bother since we've reset the file */
/* Close the file */
/* Close the file if we opened it */
if (wantClose)
{
if (fclose(fh) != 0)
fatal("could not close input file: %m");
/* Forget lookahead, since we'll re-read header after re-opening */
AH->readHeader = 0;
AH->lookaheadLen = 0;
}
return AH->format;
}
@@ -3790,7 +3731,9 @@ WriteHead(ArchiveHandle *AH)
void
ReadHead(ArchiveHandle *AH)
{
char tmpMag[7];
char vmaj,
vmin,
vrev;
int fmt;
struct tm crtm;
@@ -3802,49 +3745,47 @@ ReadHead(ArchiveHandle *AH)
*/
if (!AH->readHeader)
{
char vmaj,
vmin,
vrev;
char tmpMag[7];
AH->ReadBufPtr(AH, tmpMag, 5);
if (strncmp(tmpMag, "PGDMP", 5) != 0)
fatal("did not find magic string in file header");
vmaj = AH->ReadBytePtr(AH);
vmin = AH->ReadBytePtr(AH);
if (vmaj > 1 || (vmaj == 1 && vmin > 0)) /* Version > 1.0 */
vrev = AH->ReadBytePtr(AH);
else
vrev = 0;
AH->version = MAKE_ARCHIVE_VERSION(vmaj, vmin, vrev);
if (AH->version < K_VERS_1_0 || AH->version > K_VERS_MAX)
fatal("unsupported version (%d.%d) in file header",
vmaj, vmin);
AH->intSize = AH->ReadBytePtr(AH);
if (AH->intSize > 32)
fatal("sanity check on integer size (%lu) failed",
(unsigned long) AH->intSize);
if (AH->intSize > sizeof(int))
pg_log_warning("archive was made on a machine with larger integers, some operations might fail");
if (AH->version >= K_VERS_1_7)
AH->offSize = AH->ReadBytePtr(AH);
else
AH->offSize = AH->intSize;
fmt = AH->ReadBytePtr(AH);
if (AH->format != fmt)
fatal("expected format (%d) differs from format found in file (%d)",
AH->format, fmt);
}
vmaj = AH->ReadBytePtr(AH);
vmin = AH->ReadBytePtr(AH);
if (vmaj > 1 || (vmaj == 1 && vmin > 0)) /* Version > 1.0 */
vrev = AH->ReadBytePtr(AH);
else
vrev = 0;
AH->version = MAKE_ARCHIVE_VERSION(vmaj, vmin, vrev);
if (AH->version < K_VERS_1_0 || AH->version > K_VERS_MAX)
fatal("unsupported version (%d.%d) in file header",
vmaj, vmin);
AH->intSize = AH->ReadBytePtr(AH);
if (AH->intSize > 32)
fatal("sanity check on integer size (%lu) failed",
(unsigned long) AH->intSize);
if (AH->intSize > sizeof(int))
pg_log_warning("archive was made on a machine with larger integers, some operations might fail");
if (AH->version >= K_VERS_1_7)
AH->offSize = AH->ReadBytePtr(AH);
else
AH->offSize = AH->intSize;
fmt = AH->ReadBytePtr(AH);
if (AH->format != fmt)
fatal("expected format (%d) differs from format found in file (%d)",
AH->format, fmt);
if (AH->version >= K_VERS_1_2)
{
if (AH->version < K_VERS_1_4)