Skip to content

Commit 00d9dcf

Browse files
tvondratvondra
committed
pg_dump: Fix gzip compression of empty data
The pg_dump Compressor API has three basic callbacks - Allocate, Write and End. The gzip implementation (since e996073) wrongly assumed the Write function would always be called, and deferred the initialization of the internal compression system until the first such call. But when there's no data to compress (e.g. for empty LO), this would result in not finalizing the compression state (because it was not actually initialized), producing invalid dump. Fixed by initializing the internal compression system in the Allocate call, whenever the caller provides the Write. For decompression the state is not needed, so we leave the private_data member unpopulated. Introduces a pg_dump TAP test compressing an empty large object. This also rearranges the functions to their original order, to make diffs against older code simpler to understand. Finally, replace an unreachable pg_fatal() with a simple assert check. Reported-by: Justin Pryzby Author: Justin Pryzby, Georgios Kokolatos Reviewed-by: Georgios Kokolatos, Tomas Vondra https://postgr.es/m/20230228235834.GC30529%40telsasoft.com
1 parent 1671f99 commit 00d9dcf

File tree

2 files changed

+100
-59
lines changed

2 files changed

+100
-59
lines changed

‎src/bin/pg_dump/compress_gzip.c‎

Lines changed: 77 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,73 @@ typedef struct GzipCompressorState
3333
} GzipCompressorState;
3434

3535
/* Private routines that support gzip compressed data I/O */
36+
static void DeflateCompressorInit(CompressorState *cs);
37+
static void DeflateCompressorEnd(ArchiveHandle *AH, CompressorState *cs);
38+
static void DeflateCompressorCommon(ArchiveHandle *AH, CompressorState *cs,
39+
bool flush);
40+
static void EndCompressorGzip(ArchiveHandle *AH, CompressorState *cs);
41+
static void WriteDataToArchiveGzip(ArchiveHandle *AH, CompressorState *cs,
42+
const void *data, size_t dLen);
43+
static void ReadDataFromArchiveGzip(ArchiveHandle *AH, CompressorState *cs);
44+
45+
static void
46+
DeflateCompressorInit(CompressorState *cs)
47+
{
48+
GzipCompressorState *gzipcs;
49+
z_streamp zp;
50+
51+
gzipcs = (GzipCompressorState *) pg_malloc0(sizeof(GzipCompressorState));
52+
zp = gzipcs->zp = (z_streamp) pg_malloc(sizeof(z_stream));
53+
zp->zalloc = Z_NULL;
54+
zp->zfree = Z_NULL;
55+
zp->opaque = Z_NULL;
56+
57+
/*
58+
* outsize is the buffer size we tell zlib it can output to. We actually
59+
* allocate one extra byte because some routines want to append a trailing
60+
* zero byte to the zlib output.
61+
*/
62+
gzipcs->outsize = DEFAULT_IO_BUFFER_SIZE;
63+
gzipcs->outbuf = pg_malloc(gzipcs->outsize + 1);
64+
65+
/* -Z 0 uses the "None" compressor -- not zlib with no compression */
66+
Assert(cs->compression_spec.level != 0);
67+
68+
if (deflateInit(zp, cs->compression_spec.level) != Z_OK)
69+
pg_fatal("could not initialize compression library: %s", zp->msg);
70+
71+
/* Just be paranoid - maybe End is called after Start, with no Write */
72+
zp->next_out = gzipcs->outbuf;
73+
zp->avail_out = gzipcs->outsize;
74+
75+
/* Keep track of gzipcs */
76+
cs->private_data = gzipcs;
77+
}
78+
79+
static void
80+
DeflateCompressorEnd(ArchiveHandle *AH, CompressorState *cs)
81+
{
82+
GzipCompressorState *gzipcs = (GzipCompressorState *) cs->private_data;
83+
z_streamp zp;
84+
85+
zp = gzipcs->zp;
86+
zp->next_in = NULL;
87+
zp->avail_in = 0;
88+
89+
/* Flush any remaining data from zlib buffer */
90+
DeflateCompressorCommon(AH, cs, true);
91+
92+
if (deflateEnd(zp) != Z_OK)
93+
pg_fatal("could not close compression stream: %s", zp->msg);
94+
95+
pg_free(gzipcs->outbuf);
96+
pg_free(gzipcs->zp);
97+
pg_free(gzipcs);
98+
cs->private_data = NULL;
99+
}
100+
36101
static void
37-
DeflateCompressorGzip(ArchiveHandle *AH, CompressorState *cs, bool flush)
102+
DeflateCompressorCommon(ArchiveHandle *AH, CompressorState *cs, bool flush)
38103
{
39104
GzipCompressorState *gzipcs = (GzipCompressorState *) cs->private_data;
40105
z_streamp zp = gzipcs->zp;
@@ -78,69 +143,20 @@ DeflateCompressorGzip(ArchiveHandle *AH, CompressorState *cs, bool flush)
78143
static void
79144
EndCompressorGzip(ArchiveHandle *AH, CompressorState *cs)
80145
{
81-
GzipCompressorState *gzipcs = (GzipCompressorState *) cs->private_data;
82-
z_streamp zp;
83-
84-
if (gzipcs->zp)
85-
{
86-
zp = gzipcs->zp;
87-
zp->next_in = NULL;
88-
zp->avail_in = 0;
89-
90-
/* Flush any remaining data from zlib buffer */
91-
DeflateCompressorGzip(AH, cs, true);
92-
93-
if (deflateEnd(zp) != Z_OK)
94-
pg_fatal("could not close compression stream: %s", zp->msg);
95-
96-
pg_free(gzipcs->outbuf);
97-
pg_free(gzipcs->zp);
98-
}
99-
100-
pg_free(gzipcs);
101-
cs->private_data = NULL;
146+
/* If deflation was initialized, finalize it */
147+
if (cs->private_data)
148+
DeflateCompressorEnd(AH, cs);
102149
}
103150

104151
static void
105152
WriteDataToArchiveGzip(ArchiveHandle *AH, CompressorState *cs,
106153
const void *data, size_t dLen)
107154
{
108155
GzipCompressorState *gzipcs = (GzipCompressorState *) cs->private_data;
109-
z_streamp zp;
110-
111-
if (!gzipcs->zp)
112-
{
113-
zp = gzipcs->zp = (z_streamp) pg_malloc(sizeof(z_stream));
114-
zp->zalloc = Z_NULL;
115-
zp->zfree = Z_NULL;
116-
zp->opaque = Z_NULL;
117-
118-
/*
119-
* outsize is the buffer size we tell zlib it can output to. We
120-
* actually allocate one extra byte because some routines want to
121-
* append a trailing zero byte to the zlib output.
122-
*/
123-
gzipcs->outsize = DEFAULT_IO_BUFFER_SIZE;
124-
gzipcs->outbuf = pg_malloc(gzipcs->outsize + 1);
125-
126-
/*
127-
* A level of zero simply copies the input one block at the time. This
128-
* is probably not what the user wanted when calling this interface.
129-
*/
130-
if (cs->compression_spec.level == 0)
131-
pg_fatal("requested to compress the archive yet no level was specified");
132-
133-
if (deflateInit(zp, cs->compression_spec.level) != Z_OK)
134-
pg_fatal("could not initialize compression library: %s", zp->msg);
135-
136-
/* Just be paranoid - maybe End is called after Start, with no Write */
137-
zp->next_out = gzipcs->outbuf;
138-
zp->avail_out = gzipcs->outsize;
139-
}
140156

141157
gzipcs->zp->next_in = (void *) unconstify(void *, data);
142158
gzipcs->zp->avail_in = dLen;
143-
DeflateCompressorGzip(AH, cs, false);
159+
DeflateCompressorCommon(AH, cs, false);
144160
}
145161

146162
static void
@@ -214,17 +230,19 @@ void
214230
InitCompressorGzip(CompressorState *cs,
215231
const pg_compress_specification compression_spec)
216232
{
217-
GzipCompressorState *gzipcs;
218-
219233
cs->readData = ReadDataFromArchiveGzip;
220234
cs->writeData = WriteDataToArchiveGzip;
221235
cs->end = EndCompressorGzip;
222236

223237
cs->compression_spec = compression_spec;
224238

225-
gzipcs = (GzipCompressorState *) pg_malloc0(sizeof(GzipCompressorState));
226-
227-
cs->private_data = gzipcs;
239+
/*
240+
* If the caller has defined a write function, prepare the necessary
241+
* state. Note that if the data is empty, End may be called immediately
242+
* after Init, without ever calling Write.
243+
*/
244+
if (cs->writeF)
245+
DeflateCompressorInit(cs);
228246
}
229247

230248

‎src/bin/pg_dump/t/002_pg_dump.pl‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1265,6 +1265,29 @@
12651265
},
12661266
},
12671267

1268+
'LO create (with no data)' => {
1269+
create_sql =>
1270+
'SELECT pg_catalog.lo_create(0);',
1271+
regexp => qr/^
1272+
\QSELECT pg_catalog.lo_open\E \('\d+',\ \d+\);\n
1273+
\QSELECT pg_catalog.lo_close(0);\E
1274+
/xm,
1275+
like => {
1276+
%full_runs,
1277+
column_inserts => 1,
1278+
data_only => 1,
1279+
inserts => 1,
1280+
section_data => 1,
1281+
test_schema_plus_large_objects => 1,
1282+
},
1283+
unlike => {
1284+
binary_upgrade => 1,
1285+
no_large_objects => 1,
1286+
schema_only => 1,
1287+
section_pre_data => 1,
1288+
},
1289+
},
1290+
12681291
'COMMENT ON DATABASE postgres' => {
12691292
regexp => qr/^COMMENT ON DATABASE postgres IS .+;/m,
12701293

0 commit comments

Comments
 (0)