Skip to content

Commit 1e9ed54

Browse files
Ravi Reddycoffeys
authored andcommitted
8193682: Infinite loop in ZipOutputStream.close()
Reviewed-by: lancea, coffeys
1 parent abaa073 commit 1e9ed54

File tree

4 files changed

+229
-67
lines changed

4 files changed

+229
-67
lines changed

src/java.base/share/classes/java/util/zip/DeflaterOutputStream.java

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -234,9 +234,12 @@ public void finish() throws IOException {
234234
*/
235235
public void close() throws IOException {
236236
if (!closed) {
237-
finish();
238-
if (usesDefaultDeflater)
239-
def.end();
237+
try {
238+
finish();
239+
} finally {
240+
if (usesDefaultDeflater)
241+
def.end();
242+
}
240243
out.close();
241244
closed = true;
242245
}

src/java.base/share/classes/java/util/zip/GZIPOutputStream.java

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -157,24 +157,30 @@ public synchronized void write(byte[] buf, int off, int len)
157157
*/
158158
public void finish() throws IOException {
159159
if (!def.finished()) {
160-
def.finish();
161-
while (!def.finished()) {
162-
int len = def.deflate(buf, 0, buf.length);
163-
if (def.finished() && len <= buf.length - TRAILER_SIZE) {
164-
// last deflater buffer. Fit trailer at the end
165-
writeTrailer(buf, len);
166-
len = len + TRAILER_SIZE;
167-
out.write(buf, 0, len);
168-
return;
160+
try {
161+
def.finish();
162+
while (!def.finished()) {
163+
int len = def.deflate(buf, 0, buf.length);
164+
if (def.finished() && len <= buf.length - TRAILER_SIZE) {
165+
// last deflater buffer. Fit trailer at the end
166+
writeTrailer(buf, len);
167+
len = len + TRAILER_SIZE;
168+
out.write(buf, 0, len);
169+
return;
170+
}
171+
if (len > 0)
172+
out.write(buf, 0, len);
169173
}
170-
if (len > 0)
171-
out.write(buf, 0, len);
174+
// if we can't fit the trailer at the end of the last
175+
// deflater buffer, we write it separately
176+
byte[] trailer = new byte[TRAILER_SIZE];
177+
writeTrailer(trailer, 0);
178+
out.write(trailer);
179+
} catch (IOException e) {
180+
if (usesDefaultDeflater)
181+
def.end();
182+
throw e;
172183
}
173-
// if we can't fit the trailer at the end of the last
174-
// deflater buffer, we write it separately
175-
byte[] trailer = new byte[TRAILER_SIZE];
176-
writeTrailer(trailer, 0);
177-
out.write(trailer);
178184
}
179185
}
180186

src/java.base/share/classes/java/util/zip/ZipOutputStream.java

Lines changed: 54 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -259,58 +259,64 @@ public void putNextEntry(ZipEntry e) throws IOException {
259259
public void closeEntry() throws IOException {
260260
ensureOpen();
261261
if (current != null) {
262-
ZipEntry e = current.entry;
263-
switch (e.method) {
264-
case DEFLATED -> {
265-
def.finish();
266-
while (!def.finished()) {
267-
deflate();
268-
}
269-
if ((e.flag & 8) == 0) {
270-
// verify size, compressed size, and crc-32 settings
271-
if (e.size != def.getBytesRead()) {
272-
throw new ZipException(
273-
"invalid entry size (expected " + e.size +
274-
" but got " + def.getBytesRead() + " bytes)");
275-
}
276-
if (e.csize != def.getBytesWritten()) {
277-
throw new ZipException(
278-
"invalid entry compressed size (expected " +
279-
e.csize + " but got " + def.getBytesWritten() + " bytes)");
262+
try {
263+
ZipEntry e = current.entry;
264+
switch (e.method) {
265+
case DEFLATED -> {
266+
def.finish();
267+
while (!def.finished()) {
268+
deflate();
269+
}
270+
if ((e.flag & 8) == 0) {
271+
// verify size, compressed size, and crc-32 settings
272+
if (e.size != def.getBytesRead()) {
273+
throw new ZipException(
274+
"invalid entry size (expected " + e.size +
275+
" but got " + def.getBytesRead() + " bytes)");
276+
}
277+
if (e.csize != def.getBytesWritten()) {
278+
throw new ZipException(
279+
"invalid entry compressed size (expected " +
280+
e.csize + " but got " + def.getBytesWritten() + " bytes)");
281+
}
282+
if (e.crc != crc.getValue()) {
283+
throw new ZipException(
284+
"invalid entry CRC-32 (expected 0x" +
285+
Long.toHexString(e.crc) + " but got 0x" +
286+
Long.toHexString(crc.getValue()) + ")");
287+
}
288+
} else {
289+
e.size = def.getBytesRead();
290+
e.csize = def.getBytesWritten();
291+
e.crc = crc.getValue();
292+
writeEXT(e);
293+
}
294+
def.reset();
295+
written += e.csize;
280296
}
281-
if (e.crc != crc.getValue()) {
282-
throw new ZipException(
283-
"invalid entry CRC-32 (expected 0x" +
284-
Long.toHexString(e.crc) + " but got 0x" +
285-
Long.toHexString(crc.getValue()) + ")");
297+
case STORED -> {
298+
// we already know that both e.size and e.csize are the same
299+
if (e.size != written - locoff) {
300+
throw new ZipException(
301+
"invalid entry size (expected " + e.size +
302+
" but got " + (written - locoff) + " bytes)");
303+
}
304+
if (e.crc != crc.getValue()) {
305+
throw new ZipException(
306+
"invalid entry crc-32 (expected 0x" +
307+
Long.toHexString(e.crc) + " but got 0x" +
308+
Long.toHexString(crc.getValue()) + ")");
309+
}
286310
}
287-
} else {
288-
e.size = def.getBytesRead();
289-
e.csize = def.getBytesWritten();
290-
e.crc = crc.getValue();
291-
writeEXT(e);
311+
default -> throw new ZipException("invalid compression method");
292312
}
293-
def.reset();
294-
written += e.csize;
295-
}
296-
case STORED -> {
297-
// we already know that both e.size and e.csize are the same
298-
if (e.size != written - locoff) {
299-
throw new ZipException(
300-
"invalid entry size (expected " + e.size +
301-
" but got " + (written - locoff) + " bytes)");
302-
}
303-
if (e.crc != crc.getValue()) {
304-
throw new ZipException(
305-
"invalid entry crc-32 (expected 0x" +
306-
Long.toHexString(e.crc) + " but got 0x" +
307-
Long.toHexString(crc.getValue()) + ")");
308-
}
309-
}
310-
default -> throw new ZipException("invalid compression method");
313+
crc.reset();
314+
current = null;
315+
} catch (IOException e) {
316+
if (usesDefaultDeflater && !(e instanceof ZipException))
317+
def.end();
318+
throw e;
311319
}
312-
crc.reset();
313-
current = null;
314320
}
315321
}
316322

Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,147 @@
1+
/*
2+
* Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* This code is free software; you can redistribute it and/or modify it
6+
* under the terms of the GNU General Public License version 2 only, as
7+
* published by the Free Software Foundation.
8+
*
9+
* This code is distributed in the hope that it will be useful, but WITHOUT
10+
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
11+
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
12+
* version 2 for more details (a copy is included in the LICENSE file that
13+
* accompanied this code).
14+
*
15+
* You should have received a copy of the GNU General Public License version
16+
* 2 along with this work; if not, write to the Free Software Foundation,
17+
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
18+
*
19+
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
20+
* or visit www.oracle.com if you need additional information or have any
21+
* questions.
22+
*/
23+
24+
/**
25+
* @test
26+
* @bug 8193682
27+
* @summary Test Infinite loop while writing on closed GZipOutputStream , ZipOutputStream and JarOutputStream.
28+
* @run testng CloseDeflaterTest
29+
*/
30+
import java.io.*;
31+
import java.util.Random;
32+
import java.util.jar.JarOutputStream;
33+
import java.util.zip.GZIPOutputStream;
34+
import java.util.zip.ZipOutputStream;
35+
import java.util.zip.ZipEntry;
36+
37+
import org.testng.annotations.BeforeTest;
38+
import org.testng.annotations.DataProvider;
39+
import org.testng.annotations.Test;
40+
import static org.testng.Assert.fail;
41+
42+
43+
public class CloseDeflaterTest {
44+
45+
//number of bytes to write
46+
private static final int INPUT_LENGTH= 512;
47+
//OutputStream that will throw an exception during a write operation
48+
private static OutputStream outStream = new OutputStream() {
49+
@Override
50+
public void write(byte[] b, int off, int len) throws IOException {
51+
//throw exception during write
52+
throw new IOException();
53+
}
54+
@Override
55+
public void write(byte b[]) throws IOException {}
56+
@Override
57+
public void write(int b) throws IOException {}
58+
};
59+
private static byte[] inputBytes = new byte[INPUT_LENGTH];
60+
private static Random rand = new Random();
61+
62+
@DataProvider(name = "testgzipinput")
63+
public Object[][] testGZipInput() {
64+
//testGZip will close the GZipOutputStream using close() method when the boolean
65+
//useCloseMethod is set to true and finish() method if the value is set to false
66+
return new Object[][] {
67+
{ GZIPOutputStream.class, true },
68+
{ GZIPOutputStream.class, false },
69+
};
70+
}
71+
72+
@DataProvider(name = "testzipjarinput")
73+
public Object[][] testZipAndJarInput() {
74+
//testZipAndJarInput will perfrom write/closeEntry operations on JarOutputStream when the boolean
75+
//useJar is set to true and on ZipOutputStream if the value is set to false
76+
return new Object[][] {
77+
{ JarOutputStream.class, true },
78+
{ ZipOutputStream.class, false },
79+
};
80+
}
81+
82+
@BeforeTest
83+
public void before_test()
84+
{
85+
//add inputBytes array with random bytes to write into Zip
86+
rand.nextBytes(inputBytes);
87+
}
88+
89+
//Test for infinite loop by writing bytes to closed GZIPOutputStream
90+
@Test(dataProvider = "testgzipinput")
91+
public void testGZip(Class<?> type, boolean useCloseMethod) throws IOException {
92+
GZIPOutputStream zip = new GZIPOutputStream(outStream);
93+
try {
94+
zip.write(inputBytes, 0, INPUT_LENGTH);
95+
//close zip
96+
if(useCloseMethod) {
97+
zip.close();
98+
} else {
99+
zip.finish();
100+
}
101+
} catch (IOException e) {
102+
//expected
103+
}
104+
for (int i = 0; i < 3; i++) {
105+
try {
106+
//write on a closed GZIPOutputStream
107+
zip.write(inputBytes, 0, INPUT_LENGTH);
108+
fail("Deflater closed exception not thrown");
109+
} catch (NullPointerException e) {
110+
//expected , Deflater has been closed exception
111+
}
112+
}
113+
}
114+
115+
//Test for infinite loop by writing bytes to closed ZipOutputStream/JarOutputStream
116+
@Test(dataProvider = "testzipjarinput")
117+
public void testZipCloseEntry(Class<?> type,boolean useJar) throws IOException {
118+
ZipOutputStream zip = null;
119+
if(useJar) {
120+
zip = new JarOutputStream(outStream);
121+
} else {
122+
zip = new ZipOutputStream(outStream);
123+
}
124+
try {
125+
zip.putNextEntry(new ZipEntry(""));
126+
} catch (IOException e) {
127+
//expected to throw IOException since putNextEntry calls write method
128+
}
129+
try {
130+
zip.write(inputBytes, 0, INPUT_LENGTH);
131+
//close zip entry
132+
zip.closeEntry();
133+
} catch (IOException e) {
134+
//expected
135+
}
136+
for (int i = 0; i < 3; i++) {
137+
try {
138+
//write on a closed ZipOutputStream
139+
zip.write(inputBytes, 0, INPUT_LENGTH);
140+
fail("Deflater closed exception not thrown");
141+
} catch (NullPointerException e) {
142+
//expected , Deflater has been closed exception
143+
}
144+
}
145+
}
146+
147+
}

0 commit comments

Comments
 (0)