Skip to content

Commit f364fc8

Browse files
axl8713axl8713
committed
Fixed UploadPluginController temp files usage and simplified code.
1 parent 3e466f8 commit f364fc8

2 files changed

Lines changed: 32 additions & 20 deletions

File tree

‎java/services/src/main/java/it/geosolutions/mapstore/controllers/rest/config/UploadPluginController.java‎

Lines changed: 17 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import it.geosolutions.mapstore.utils.ResourceUtils;
1717
import org.apache.commons.io.FileUtils;
1818
import org.apache.commons.io.IOUtils;
19+
import org.apache.commons.io.input.CloseShieldInputStream;
1920
import org.springframework.beans.factory.annotation.Autowired;
2021
import org.springframework.security.access.annotation.Secured;
2122
import org.springframework.stereotype.Controller;
@@ -25,13 +26,14 @@
2526
import org.springframework.web.bind.annotation.ResponseBody;
2627

2728
import jakarta.servlet.ServletContext;
29+
2830
import java.io.File;
2931
import java.io.FileInputStream;
3032
import java.io.FileNotFoundException;
3133
import java.io.FileOutputStream;
3234
import java.io.IOException;
3335
import java.io.InputStream;
34-
import java.nio.charset.StandardCharsets;
36+
import java.nio.file.Files;
3537
import java.nio.file.Path;
3638
import java.nio.file.Paths;
3739
import java.util.HashMap;
@@ -69,6 +71,8 @@ public class UploadPluginController extends BaseMapStoreController {
6971
@Secured({"ROLE_ADMIN"})
7072
@RequestMapping(value = "/uploadPlugin", method = RequestMethod.POST, headers = "Accept=application/json")
7173
public @ResponseBody String uploadPlugin(InputStream dataStream) throws IOException {
74+
File servletTempDir = (File) context.getAttribute(ServletContext.TEMPDIR);
75+
Path tempDir = Files.createTempDirectory(servletTempDir.toPath(), "mapstore-upload-");
7276
try (ZipInputStream zip = new ZipInputStream(dataStream)) {
7377
ZipEntry entry = zip.getNextEntry();
7478
String pluginName = null;
@@ -85,12 +89,12 @@ public class UploadPluginController extends BaseMapStoreController {
8589

8690
if (lower.endsWith("index.js")) {
8791
bundleEntryName = normalizedEntry;
88-
File tempBundle = File.createTempFile("mapstore-bundle", ".js");
92+
File tempBundle = Files.createTempFile(tempDir, "mapstore-bundle", ".js").toFile();
8993
storeAsset(zip, tempBundle);
9094
// We'll resolve final relative target after we know pluginName
9195
tempFilesToRelativeTargets.put(tempBundle, "__BUNDLE__");
9296
} else if (lower.equals("index.json")) {
93-
JsonNode json = readJSON(zip);
97+
JsonNode json = readJSON(CloseShieldInputStream.wrap(zip));
9498
JsonNode plugins = json.get("plugins");
9599
if (plugins == null || !plugins.isArray() || plugins.isEmpty()) {
96100
throw new IOException("Invalid bundle: index.json has no 'plugins' array");
@@ -108,13 +112,13 @@ public class UploadPluginController extends BaseMapStoreController {
108112
addPluginConfiguration(plugin);
109113
}
110114
} else if (lower.startsWith("translations/")) {
111-
File tempAsset = File.createTempFile("mapstore-asset-translations", ".json");
115+
File tempAsset = Files.createTempFile(tempDir, "mapstore-asset-translations", ".json").toFile();
112116
storeAsset(zip, tempAsset);
113117
// Target relative to extensions root: <pluginName>/<translations/...>
114118
tempFilesToRelativeTargets.put(tempAsset, PLUGIN_PATH_PREFIX + normalizedEntry);
115119
addTranslations = true;
116120
} else if (lower.startsWith("assets/")) {
117-
File tempAsset = File.createTempFile("mapstore-asset", ".tmp");
121+
File tempAsset = Files.createTempFile(tempDir, "mapstore-asset", ".tmp").toFile();
118122
storeAsset(zip, tempAsset);
119123
// Target relative to extensions root: <pluginName>/<assets/...>
120124
tempFilesToRelativeTargets.put(tempAsset, PLUGIN_PATH_PREFIX + normalizedEntry);
@@ -159,6 +163,8 @@ public class UploadPluginController extends BaseMapStoreController {
159163
}
160164

161165
return plugin.toString();
166+
} finally {
167+
FileUtils.deleteDirectory(tempDir.toFile());
162168
}
163169
}
164170

@@ -212,7 +218,7 @@ private boolean shouldStorePluginsConfigAsPatch() {
212218
private boolean canUseDataDir() {
213219
return !getDataDir().isEmpty() &&
214220
Stream.of(getDataDir().split(","))
215-
.anyMatch(folder -> !folder.trim().isEmpty() && new File(folder).exists());
221+
.anyMatch(folder -> !folder.trim().isEmpty() && new File(folder).exists());
216222
}
217223

218224
/**
@@ -250,7 +256,7 @@ private boolean canUseDataDir() {
250256
for (int i = 0; i < plugins.size(); i++) {
251257
JsonNode plugin = plugins.get(i);
252258
String name = plugin.has("name") ? plugin.get("name").asText()
253-
: plugin.get("value").get("name").asText();
259+
: plugin.get("value").get("name").asText();
254260
if (name.contentEquals(pluginName)) {
255261
toRemove = i;
256262
}
@@ -445,7 +451,7 @@ private void storeJSONConfig(Object config, String configName) throws IOExceptio
445451
}
446452

447453
private void addExtension(String pluginName, String pluginBundleRelative, String translationsRelative)
448-
throws IOException {
454+
throws IOException {
449455
ObjectNode config;
450456
Optional<File> extensionsConfigFile = findResource(getExtensionsConfigPath());
451457
if (extensionsConfigFile.isPresent()) {
@@ -484,25 +490,16 @@ private ObjectNode getExtensionConfig() throws IOException {
484490
}
485491

486492
private JsonNode readJSON(InputStream input) throws IOException {
487-
byte[] buffer = new byte[4096];
488-
int read;
489-
StringBuilder json = new StringBuilder();
490-
while ((read = input.read(buffer)) >= 0) {
491-
json.append(new String(buffer, 0, read, StandardCharsets.UTF_8));
492-
}
493-
return jsonMapper.readTree(json.toString());
493+
return jsonMapper.readTree(input);
494494
}
495495

496496
private void storeAsset(ZipInputStream zip, File file) throws IOException {
497497
try (FileOutputStream outFile = new FileOutputStream(file)) {
498-
byte[] buffer = new byte[8192];
499-
int read;
500-
while ((read = zip.read(buffer)) >= 0) {
501-
outFile.write(buffer, 0, read);
502-
}
498+
IOUtils.copy(zip, outFile);
503499
}
504500
}
505501

502+
@Override
506503
public void setContext(ServletContext context) {
507504
this.context = context;
508505
}

‎java/services/src/test/java/it/geosolutions/mapstore/UploadPluginControllerTest.java‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ public void testUninstallPluginPathNormalization() throws IOException {
5858
Mockito.when(context.getRealPath(Mockito.contains("pluginsConfig.json"))).thenReturn(tempConfig.getAbsolutePath());
5959
File tempExtensions = TestUtils.copyToTemp(ConfigControllerTest.class, "/extensionsWithPlugin.json");
6060
File tempDist = TestUtils.getDataDir();
61+
Mockito.when(context.getAttribute(ServletContext.TEMPDIR)).thenReturn(tempDist);
6162

6263
// Mock a legitimate folder inside the base directory
6364
Mockito.when(context.getRealPath(Mockito.contains("extensions.json"))).thenReturn(tempExtensions.getAbsolutePath());
@@ -89,6 +90,7 @@ public void testUploadValidBundle() throws IOException {
8990
Mockito.when(context.getRealPath(Mockito.endsWith("pluginsConfig.json"))).thenReturn(tempConfig.getAbsolutePath());
9091
File tempExtensions = TestUtils.copyToTemp(ConfigControllerTest.class, "/extensions.json");
9192
File tempDist = TestUtils.getDataDir();
93+
Mockito.when(context.getAttribute(ServletContext.TEMPDIR)).thenReturn(tempDist);
9294
Mockito.when(context.getRealPath(Mockito.contains("extensions.json"))).thenReturn(tempExtensions.getAbsolutePath());
9395
Mockito.when(context.getRealPath(Mockito.contains("My"))).thenAnswer(
9496
(Answer<String>) invocation -> {
@@ -112,6 +114,7 @@ public void testUploadValidBundleReplace() throws IOException {
112114
Mockito.when(context.getRealPath(Mockito.endsWith("pluginsConfig.json"))).thenReturn(tempConfig.getAbsolutePath());
113115
File tempExtensions = TestUtils.copyToTemp(ConfigControllerTest.class, "/extensionsWithPlugin.json");
114116
File tempDist = TestUtils.getDataDir();
117+
Mockito.when(context.getAttribute(ServletContext.TEMPDIR)).thenReturn(tempDist);
115118
Mockito.when(context.getRealPath(Mockito.contains("extensions.json"))).thenReturn(tempExtensions.getAbsolutePath());
116119
Mockito.when(context.getRealPath(Mockito.contains("My"))).thenAnswer(
117120
(Answer<String>) invocation -> {
@@ -139,6 +142,7 @@ public void testUploadValidBundleUsingPatch() throws IOException {
139142
Mockito.when(context.getRealPath(Mockito.endsWith(".patch"))).thenReturn(tempDir.getAbsolutePath() + "/pluginsConfig.json.patch");
140143
File tempExtensions = TestUtils.copyToTemp(ConfigControllerTest.class, "/extensions.json");
141144
File tempDist = TestUtils.getDataDir();
145+
Mockito.when(context.getAttribute(ServletContext.TEMPDIR)).thenReturn(tempDist);
142146
Mockito.when(context.getRealPath(Mockito.contains("extensions.json"))).thenReturn(tempExtensions.getAbsolutePath());
143147
Mockito.when(context.getRealPath(Mockito.contains("dist/extensions/"))).thenAnswer(
144148
(Answer<String>) invocation -> {
@@ -162,6 +166,7 @@ public void testCustomBundlesPath() throws IOException {
162166
Mockito.when(context.getRealPath(Mockito.endsWith("pluginsConfig.json"))).thenReturn(tempConfig.getAbsolutePath());
163167
File tempExtensions = TestUtils.copyToTemp(ConfigControllerTest.class, "/extensions.json");
164168
File tempDist = TestUtils.getDataDir();
169+
Mockito.when(context.getAttribute(ServletContext.TEMPDIR)).thenReturn(tempDist);
165170
controller.setBundlesPath("custom");
166171
Mockito.when(context.getRealPath(Mockito.contains("extensions.json"))).thenReturn(tempExtensions.getAbsolutePath());
167172
Mockito.when(context.getRealPath(Mockito.contains("custom/"))).thenAnswer(
@@ -184,6 +189,7 @@ public void testUploadInvalidBundle() throws IOException {
184189
Mockito.when(context.getRealPath(Mockito.contains("pluginsConfig.json"))).thenReturn(tempConfig.getAbsolutePath());
185190
File tempExtensions = TestUtils.copyToTemp(ConfigControllerTest.class, "/extensions.json");
186191
File tempDist = TestUtils.getDataDir();
192+
Mockito.when(context.getAttribute(ServletContext.TEMPDIR)).thenReturn(tempDist);
187193
Mockito.when(context.getRealPath(Mockito.contains("extensions.json"))).thenReturn(tempExtensions.getAbsolutePath());
188194
Mockito.when(context.getRealPath(Mockito.contains("/extensions/"))).thenAnswer(
189195
(Answer<String>) invocation -> {
@@ -206,6 +212,7 @@ public void testUploadValidBundleWithDataDir() throws IOException {
206212
controller.setDataDir(dataDir.getAbsolutePath());
207213
ServletContext context = Mockito.mock(ServletContext.class);
208214
controller.setContext(context);
215+
Mockito.when(context.getAttribute(ServletContext.TEMPDIR)).thenReturn(dataDir);
209216
File tempConfig = TestUtils.copyTo(UploadPluginControllerTest.class.getResourceAsStream("/pluginsConfig.json"), dataDir, "/configs/pluginsConfig.json");
210217
File tempExtensions = TestUtils.copyTo(UploadPluginControllerTest.class.getResourceAsStream("/extensions.json"), dataDir, "/extensions/extensions.json");
211218
InputStream zipStream = UploadPluginControllerTest.class.getResourceAsStream("/plugin.zip");
@@ -222,6 +229,7 @@ public void testUploadValidBundleWithMultipleDataDir() throws IOException {
222229
controller.setDataDir(dataDir1.getAbsolutePath() + "," + dataDir2.getAbsolutePath());
223230
ServletContext context = Mockito.mock(ServletContext.class);
224231
controller.setContext(context);
232+
Mockito.when(context.getAttribute(ServletContext.TEMPDIR)).thenReturn(dataDir1);
225233
// we load from dataDir2 (less priority)
226234
File tempConfig = TestUtils.copyTo(
227235
UploadPluginControllerTest.class.getResourceAsStream("/pluginsConfig.json"),
@@ -245,6 +253,7 @@ public void testUninstallPlugin() throws IOException {
245253
Mockito.when(context.getRealPath(Mockito.contains("pluginsConfig.json"))).thenReturn(tempConfig.getAbsolutePath());
246254
File tempExtensions = TestUtils.copyToTemp(ConfigControllerTest.class, "/extensionsWithPlugin.json");
247255
File tempDist = TestUtils.getDataDir();
256+
Mockito.when(context.getAttribute(ServletContext.TEMPDIR)).thenReturn(tempDist);
248257
Mockito.when(context.getRealPath(Mockito.contains("extensions.json"))).thenReturn(tempExtensions.getAbsolutePath());
249258
Mockito.when(context.getRealPath(Mockito.contains("My"))).thenAnswer(
250259
(Answer<String>) invocation -> {
@@ -305,6 +314,7 @@ public void testUploadBlocksZipSlip_DotDotInAssets() throws IOException {
305314
Mockito.when(context.getRealPath(Mockito.contains("extensions.json"))).thenReturn(tempExtensions.getAbsolutePath());
306315

307316
File tempDist = TestUtils.getDataDir();
317+
Mockito.when(context.getAttribute(ServletContext.TEMPDIR)).thenReturn(tempDist);
308318
Mockito.when(context.getRealPath(Mockito.contains("dist/extensions/"))).thenAnswer(
309319
(Answer<String>) invocation -> {
310320
String path = (String) invocation.getArguments()[0];
@@ -329,6 +339,7 @@ public void testUploadBlocksZipSlip_AbsoluteUnixEntry() throws IOException {
329339
Mockito.when(context.getRealPath(Mockito.contains("extensions.json"))).thenReturn(tempExtensions.getAbsolutePath());
330340

331341
File tempDist = TestUtils.getDataDir();
342+
Mockito.when(context.getAttribute(ServletContext.TEMPDIR)).thenReturn(tempDist);
332343
Mockito.when(context.getRealPath(Mockito.contains("dist/extensions/"))).thenReturn(
333344
tempDist.getAbsolutePath() + File.separator + "ignored"
334345
);
@@ -349,6 +360,7 @@ public void testUploadBlocksZipSlip_WindowsDriveEntry() throws IOException {
349360
Mockito.when(context.getRealPath(Mockito.endsWith("pluginsConfig.json"))).thenReturn(tempConfig.getAbsolutePath());
350361
File tempExtensions = TestUtils.copyToTemp(ConfigControllerTest.class, "/extensions.json");
351362
Mockito.when(context.getRealPath(Mockito.contains("extensions.json"))).thenReturn(tempExtensions.getAbsolutePath());
363+
Mockito.when(context.getAttribute(ServletContext.TEMPDIR)).thenReturn(TestUtils.getDataDir());
352364

353365
Map<String, byte[]> extras = new HashMap<>();
354366
extras.put("C:\\evil.js", "drv".getBytes(StandardCharsets.UTF_8));
@@ -368,6 +380,7 @@ public void testUploadBlocksZipSlip_BackslashTraversalInAssets() throws IOExcept
368380
Mockito.when(context.getRealPath(Mockito.contains("extensions.json"))).thenReturn(tempExtensions.getAbsolutePath());
369381

370382
File tempDist = TestUtils.getDataDir();
383+
Mockito.when(context.getAttribute(ServletContext.TEMPDIR)).thenReturn(tempDist);
371384
Mockito.when(context.getRealPath(Mockito.contains("dist/extensions/"))).thenAnswer(
372385
(Answer<String>) invocation -> {
373386
String path = (String) invocation.getArguments()[0];
@@ -390,6 +403,7 @@ public void testUploadBlocksZipSlip_TraversalInBundleName() throws IOException {
390403
Mockito.when(context.getRealPath(Mockito.endsWith("pluginsConfig.json"))).thenReturn(tempConfig.getAbsolutePath());
391404
File tempExtensions = TestUtils.copyToTemp(ConfigControllerTest.class, "/extensions.json");
392405
Mockito.when(context.getRealPath(Mockito.contains("extensions.json"))).thenReturn(tempExtensions.getAbsolutePath());
406+
Mockito.when(context.getAttribute(ServletContext.TEMPDIR)).thenReturn(TestUtils.getDataDir());
393407

394408
Map<String, byte[]> extras = new HashMap<>();
395409
// no extra assets needed; bundle name itself is malicious
@@ -408,6 +422,7 @@ public void testUploadAllowsNestedAssets() throws IOException {
408422
Mockito.when(context.getRealPath(Mockito.contains("extensions.json"))).thenReturn(tempExtensions.getAbsolutePath());
409423

410424
final File tempDist = TestUtils.getDataDir();
425+
Mockito.when(context.getAttribute(ServletContext.TEMPDIR)).thenReturn(tempDist);
411426

412427
// Cover both likely extension roots
413428
Mockito.when(context.getRealPath(Mockito.contains("dist/extensions/"))).thenAnswer(

0 commit comments

Comments
 (0)