Skip to content
This repository was archived by the owner on May 28, 2018. It is now read-only.

Commit 6ecb965

Browse files
author
Michal Gajdos
committed
JERSEY-2846: When throwing an Exception in a Multipart File upload resource, the tmp file is not deleted.
 - Fix + tests (unit + e2e) Change-Id: I60ef4812337e230648b2d5706dc3461952a90331 Signed-off-by: Michal Gajdos <[email protected]>
1 parent 0214b7d commit 6ecb965

File tree

9 files changed

+466
-6
lines changed

9 files changed

+466
-6
lines changed

‎media/multipart/src/main/java/org/glassfish/jersey/media/multipart/BodyPartEntity.java‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,10 @@
4444
import java.io.File;
4545
import java.io.IOException;
4646
import java.io.InputStream;
47+
import java.util.logging.Level;
48+
import java.util.logging.Logger;
49+
50+
import org.glassfish.jersey.media.multipart.internal.LocalizationMessages;
4751

4852
import org.jvnet.mimepull.MIMEPart;
4953

@@ -62,7 +66,10 @@
6266
*/
6367
public class BodyPartEntity implements Closeable {
6468

69+
private static final Logger LOGGER = Logger.getLogger(BodyPartEntity.class.getName());
70+
6571
private final MIMEPart mimePart;
72+
private volatile File file;
6673

6774
/**
6875
* Constructs a new {@code BodyPartEntity} with a {@link MIMEPart}.
@@ -87,6 +94,15 @@ public InputStream getInputStream() {
8794
*/
8895
public void cleanup() {
8996
mimePart.close();
97+
98+
if (file != null) {
99+
final boolean deleted = file.delete();
100+
if (!deleted) {
101+
if (LOGGER.isLoggable(Level.FINE)) {
102+
LOGGER.log(Level.FINE, LocalizationMessages.TEMP_FILE_NOT_DELETED(file.getAbsolutePath()));
103+
}
104+
}
105+
}
90106
}
91107

92108
/**
@@ -103,5 +119,9 @@ public void close() throws IOException {
103119
*/
104120
public void moveTo(final File file) {
105121
mimePart.moveTo(file);
122+
123+
// Remember the file where the mime-part object should be stored. Mimepull would not be able to delete it after
124+
// it's moved.
125+
this.file = file;
106126
}
107127
}

‎media/multipart/src/main/resources/org/glassfish/jersey/media/multipart/internal/localization.properties‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,3 +52,4 @@ no.available.mbr=No available MessageBodyReader for class "{0}" and media type "
5252
no.available.mbw=No available MessageBodyWriter for class "{0}" and media type "{1}".
5353
parsing.error=Exception occurred during parsing MultiPart message. Performing cleanup.
5454
temp.file.cannot.be.created=Cannot create temporary files. Multipart attachments will be limited to "{0}" bytes.
55+
temp.file.not.deleted=Temporary file {0} was not deleted.

‎media/multipart/src/test/java/org/glassfish/jersey/media/multipart/internal/FormDataMultiPartReaderWriterTest.java‎

Lines changed: 59 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@
6464
import javax.ws.rs.Path;
6565
import javax.ws.rs.Produces;
6666
import javax.ws.rs.QueryParam;
67+
import javax.ws.rs.WebApplicationException;
6768
import javax.ws.rs.client.Entity;
6869
import javax.ws.rs.client.Invocation;
6970
import javax.ws.rs.core.Context;
@@ -656,17 +657,30 @@ public void testMediaTypeWithQuotedBoundaryResource() throws Exception {
656657
}
657658

658659
@Path("/FileResource")
660+
@Consumes("multipart/form-data")
661+
@Produces("text/plain")
659662
public static class FileResource {
660663

661664
@POST
662-
@Consumes("multipart/form-data")
663-
@Produces("text/plain")
664-
public String put(@FormDataParam("file") final File file) {
665+
@Path("InjectedFileNotCopied")
666+
public String injectedFileNotCopied(@FormDataParam("file") final File file) {
665667
final String path = file.getAbsolutePath();
666668
//noinspection ResultOfMethodCallIgnored
667669
file.delete();
668670
return path;
669671
}
672+
673+
@POST
674+
@Path("ExceptionInMethod")
675+
public String exceptionInMethod(@FormDataParam("file") final File file) {
676+
throw new WebApplicationException(Response.serverError().entity(file.getAbsolutePath()).build());
677+
}
678+
679+
@POST
680+
@Path("SuccessfulMethod")
681+
public String successfulMethod(@FormDataParam("file") final File file) {
682+
return file.getAbsolutePath();
683+
}
670684
}
671685

672686
/**
@@ -680,7 +694,7 @@ public void testInjectedFileNotCopied(@Mocked final BodyPartEntity entity) throw
680694
"CONTENT");
681695
multipart.bodyPart(bodypart);
682696

683-
final Response response = target().path("FileResource")
697+
final Response response = target().path("FileResource").path("InjectedFileNotCopied")
684698
.request()
685699
.post(Entity.entity(multipart, MediaType.MULTIPART_FORM_DATA));
686700

@@ -696,6 +710,46 @@ public void testInjectedFileNotCopied(@Mocked final BodyPartEntity entity) throw
696710
new File(pathname).exists(), is(false));
697711
}
698712

713+
/**
714+
* JERSEY-2846 reproducer. Make sure that temporary file created by MIMEPull deleted after a successful request.
715+
*/
716+
@Test
717+
public void tempFileDeletedAfterSuccessfulProcessing() throws Exception {
718+
final FormDataMultiPart multipart = new FormDataMultiPart();
719+
final FormDataBodyPart bodypart = new FormDataBodyPart(FormDataContentDisposition.name("file").fileName("file").build(),
720+
"CONTENT");
721+
multipart.bodyPart(bodypart);
722+
723+
final Response response = target().path("FileResource").path("SuccessfulMethod")
724+
.request()
725+
.post(Entity.entity(multipart, MediaType.MULTIPART_FORM_DATA));
726+
727+
// Make sure that the temp file has been removed.
728+
final String pathname = response.readEntity(String.class);
729+
assertThat("Temporary file, " + pathname + ", on the server has not been removed",
730+
new File(pathname).exists(), is(false));
731+
}
732+
733+
/**
734+
* JERSEY-2846 reproducer. Make sure that temporary file created by MIMEPull deleted after an unsuccessful request.
735+
*/
736+
@Test
737+
public void tempFileDeletedAfterExceptionInMethod() throws Exception {
738+
final FormDataMultiPart multipart = new FormDataMultiPart();
739+
final FormDataBodyPart bodypart = new FormDataBodyPart(FormDataContentDisposition.name("file").fileName("file").build(),
740+
"CONTENT");
741+
multipart.bodyPart(bodypart);
742+
743+
final Response response = target().path("FileResource").path("ExceptionInMethod")
744+
.request()
745+
.post(Entity.entity(multipart, MediaType.MULTIPART_FORM_DATA));
746+
747+
// Make sure that the temp file has been removed.
748+
final String pathname = response.readEntity(String.class);
749+
assertThat("Temporary file, " + pathname + ", on the server has not been removed",
750+
new File(pathname).exists(), is(false));
751+
}
752+
699753
@Path("/InputStreamResource")
700754
public static class InputStreamResource {
701755

@@ -711,7 +765,7 @@ public String put(@FormDataParam("submit") final InputStream stream) {
711765
* Mocked JERSEY-2794 reproducer. Real test is under integration tests.
712766
*/
713767
@Test
714-
public void mimeTempFileRemoved(@Mocked final MIMEMessage message) throws Exception {
768+
public void mimeTempFileRemovedAfterAbortedUpload(@Mocked final MIMEMessage message) throws Exception {
715769
new Expectations() {{
716770
message.getAttachments();
717771
result = new MIMEParsingException();

‎tests/integration/jersey-2794/src/test/java/org/glassfish/jersey/tests/integration/jersey2794/Jersey2794ITCase.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,8 @@ private int matchingTempFiles(final String tempDir) throws IOException {
129129
return Iterators.size(Files.newDirectoryStream(Paths.get(tempDir), new DirectoryStream.Filter<Path>() {
130130
@Override
131131
public boolean accept(final Path path) throws IOException {
132-
return path.startsWith("MIME") && path.endsWith("tmp");
132+
final String name = path.getFileName().toString();
133+
return name.startsWith("MIME") && name.endsWith("tmp");
133134
}
134135
}).iterator());
135136
}

‎tests/integration/jersey-2846/pom.xml‎

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<!--
3+
4+
DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS HEADER.
5+
6+
Copyright (c) 2015 Oracle and/or its affiliates. All rights reserved.
7+
8+
The contents of this file are subject to the terms of either the GNU
9+
General Public License Version 2 only ("GPL") or the Common Development
10+
and Distribution License("CDDL") (collectively, the "License"). You
11+
may not use this file except in compliance with the License. You can
12+
obtain a copy of the License at
13+
http://glassfish.java.net/public/CDDL+GPL_1_1.html
14+
or packager/legal/LICENSE.txt. See the License for the specific
15+
language governing permissions and limitations under the License.
16+
17+
When distributing the software, include this License Header Notice in each
18+
file and include the License file at packager/legal/LICENSE.txt.
19+
20+
GPL Classpath Exception:
21+
Oracle designates this particular file as subject to the "Classpath"
22+
exception as provided by Oracle in the GPL Version 2 section of the License
23+
file that accompanied this code.
24+
25+
Modifications:
26+
If applicable, add the following below the License Header, with the fields
27+
enclosed by brackets [] replaced by your own identifying information:
28+
"Portions Copyright [year] [name of copyright owner]"
29+
30+
Contributor(s):
31+
If you wish your version of this file to be governed by only the CDDL or
32+
only the GPL Version 2, indicate your decision by adding "[Contributor]
33+
elects to include this software in this distribution under the [CDDL or GPL
34+
Version 2] license." If you don't indicate a single choice of license, a
35+
recipient has the option to distribute your version of this file under
36+
either the CDDL, the GPL Version 2 or to extend the choice of license to
37+
its licensees as provided above. However, if you add GPL Version 2 code
38+
and therefore, elected the GPL Version 2 license, then the option applies
39+
only if the new code is made subject to such option by the copyright
40+
holder.
41+
42+
-->
43+
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
44+
<modelVersion>4.0.0</modelVersion>
45+
46+
<parent>
47+
<groupId>org.glassfish.jersey.tests.integration</groupId>
48+
<artifactId>project</artifactId>
49+
<version>2.19-SNAPSHOT</version>
50+
</parent>
51+
52+
<artifactId>jersey-2846</artifactId>
53+
<packaging>war</packaging>
54+
<name>jersey-tests-integration-jersey-2846</name>
55+
56+
<description>
57+
Reproducer of JERSEY-2846. This test makes sure that temporary files created via mimepull (and moved by Jersey) library
58+
are deleted right after the request processing (either successful or unsuccessful) and not when the JVM is shut down.
59+
</description>
60+
61+
<dependencies>
62+
<dependency>
63+
<groupId>org.glassfish.jersey.containers</groupId>
64+
<artifactId>jersey-container-servlet</artifactId>
65+
</dependency>
66+
<dependency>
67+
<groupId>org.glassfish.jersey.media</groupId>
68+
<artifactId>jersey-media-multipart</artifactId>
69+
</dependency>
70+
71+
<dependency>
72+
<groupId>org.glassfish.jersey.test-framework.providers</groupId>
73+
<artifactId>jersey-test-framework-provider-external</artifactId>
74+
<scope>test</scope>
75+
</dependency>
76+
</dependencies>
77+
78+
<build>
79+
<plugins>
80+
<plugin>
81+
<groupId>org.apache.maven.plugins</groupId>
82+
<artifactId>maven-compiler-plugin</artifactId>
83+
</plugin>
84+
<plugin>
85+
<groupId>org.apache.maven.plugins</groupId>
86+
<artifactId>maven-failsafe-plugin</artifactId>
87+
</plugin>
88+
<plugin>
89+
<groupId>org.mortbay.jetty</groupId>
90+
<artifactId>jetty-maven-plugin</artifactId>
91+
</plugin>
92+
</plugins>
93+
</build>
94+
95+
</project>

‎tests/integration/jersey-2846/src/main/java/org/glassfish/jersey/tests/integration/jersey2846/TestApplication.java‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
/*
2+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS HEADER.
3+
*
4+
* Copyright (c) 2015 Oracle and/or its affiliates. All rights reserved.
5+
*
6+
* The contents of this file are subject to the terms of either the GNU
7+
* General Public License Version 2 only ("GPL") or the Common Development
8+
* and Distribution License("CDDL") (collectively, the "License"). You
9+
* may not use this file except in compliance with the License. You can
10+
* obtain a copy of the License at
11+
* http://glassfish.java.net/public/CDDL+GPL_1_1.html
12+
* or packager/legal/LICENSE.txt. See the License for the specific
13+
* language governing permissions and limitations under the License.
14+
*
15+
* When distributing the software, include this License Header Notice in each
16+
* file and include the License file at packager/legal/LICENSE.txt.
17+
*
18+
* GPL Classpath Exception:
19+
* Oracle designates this particular file as subject to the "Classpath"
20+
* exception as provided by Oracle in the GPL Version 2 section of the License
21+
* file that accompanied this code.
22+
*
23+
* Modifications:
24+
* If applicable, add the following below the License Header, with the fields
25+
* enclosed by brackets [] replaced by your own identifying information:
26+
* "Portions Copyright [year] [name of copyright owner]"
27+
*
28+
* Contributor(s):
29+
* If you wish your version of this file to be governed by only the CDDL or
30+
* only the GPL Version 2, indicate your decision by adding "[Contributor]
31+
* elects to include this software in this distribution under the [CDDL or GPL
32+
* Version 2] license." If you don't indicate a single choice of license, a
33+
* recipient has the option to distribute your version of this file under
34+
* either the CDDL, the GPL Version 2 or to extend the choice of license to
35+
* its licensees as provided above. However, if you add GPL Version 2 code
36+
* and therefore, elected the GPL Version 2 license, then the option applies
37+
* only if the new code is made subject to such option by the copyright
38+
* holder.
39+
*/
40+
41+
package org.glassfish.jersey.tests.integration.jersey2846;
42+
43+
import javax.ws.rs.ApplicationPath;
44+
45+
import org.glassfish.jersey.media.multipart.MultiPartFeature;
46+
import org.glassfish.jersey.server.ResourceConfig;
47+
48+
/**
49+
* Jersey application for JERSEY-2846.
50+
*
51+
* @author Michal Gajdos (michal.gajdos at oracle.com)
52+
*/
53+
@ApplicationPath("/")
54+
public class TestApplication extends ResourceConfig {
55+
56+
public TestApplication() {
57+
register(TestResource.class);
58+
register(MultiPartFeature.class);
59+
}
60+
}

‎tests/integration/jersey-2846/src/main/java/org/glassfish/jersey/tests/integration/jersey2846/TestResource.java‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
/*
2+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS HEADER.
3+
*
4+
* Copyright (c) 2015 Oracle and/or its affiliates. All rights reserved.
5+
*
6+
* The contents of this file are subject to the terms of either the GNU
7+
* General Public License Version 2 only ("GPL") or the Common Development
8+
* and Distribution License("CDDL") (collectively, the "License"). You
9+
* may not use this file except in compliance with the License. You can
10+
* obtain a copy of the License at
11+
* http://glassfish.java.net/public/CDDL+GPL_1_1.html
12+
* or packager/legal/LICENSE.txt. See the License for the specific
13+
* language governing permissions and limitations under the License.
14+
*
15+
* When distributing the software, include this License Header Notice in each
16+
* file and include the License file at packager/legal/LICENSE.txt.
17+
*
18+
* GPL Classpath Exception:
19+
* Oracle designates this particular file as subject to the "Classpath"
20+
* exception as provided by Oracle in the GPL Version 2 section of the License
21+
* file that accompanied this code.
22+
*
23+
* Modifications:
24+
* If applicable, add the following below the License Header, with the fields
25+
* enclosed by brackets [] replaced by your own identifying information:
26+
* "Portions Copyright [year] [name of copyright owner]"
27+
*
28+
* Contributor(s):
29+
* If you wish your version of this file to be governed by only the CDDL or
30+
* only the GPL Version 2, indicate your decision by adding "[Contributor]
31+
* elects to include this software in this distribution under the [CDDL or GPL
32+
* Version 2] license." If you don't indicate a single choice of license, a
33+
* recipient has the option to distribute your version of this file under
34+
* either the CDDL, the GPL Version 2 or to extend the choice of license to
35+
* its licensees as provided above. However, if you add GPL Version 2 code
36+
* and therefore, elected the GPL Version 2 license, then the option applies
37+
* only if the new code is made subject to such option by the copyright
38+
* holder.
39+
*/
40+
package org.glassfish.jersey.tests.integration.jersey2846;
41+
42+
import java.io.File;
43+
44+
import javax.ws.rs.Consumes;
45+
import javax.ws.rs.POST;
46+
import javax.ws.rs.Path;
47+
import javax.ws.rs.Produces;
48+
import javax.ws.rs.WebApplicationException;
49+
import javax.ws.rs.core.Response;
50+
51+
import org.glassfish.jersey.media.multipart.FormDataParam;
52+
53+
/**
54+
* @author Michal Gajdos (michal.gajdos at oracle.com)
55+
*/
56+
@Path("/")
57+
@Consumes("multipart/form-data")
58+
@Produces("text/plain")
59+
public class TestResource {
60+
61+
@POST
62+
@Path("ExceptionInMethod")
63+
public String exceptionInMethod(@FormDataParam("file") final File file) {
64+
throw new WebApplicationException(Response.serverError().entity(file.getAbsolutePath()).build());
65+
}
66+
67+
@POST
68+
@Path("SuccessfulMethod")
69+
public String successfulMethod(@FormDataParam("file") final File file) {
70+
return file.getAbsolutePath();
71+
}
72+
}

0 commit comments

Comments
 (0)