Gitlab Community Edition Instance

Commit 81cf7c03 authored by mhellka's avatar mhellka
Browse files

api / ZIP import: Report file name encoding errors

Fail if imported ZIP file contains filenames non-utf-8 filenames.
This unfortunately is not feasible for TAR archives because the tar
implementation silently replaces bad characters with '?'.
parent bb12e9d9
Pipeline #243283 passed with stages
in 7 minutes and 6 seconds
......@@ -42,9 +42,9 @@ public class TarImporter implements Runnable {
/**
*
* @param target
* to import files to.
* to import files to.
* @param source
* to read from. The stream is closed after calling run().
* to read from. The stream is closed after calling run().
*/
public TarImporter(ImportTarget target, Path source, String encodingHint) {
......@@ -76,7 +76,7 @@ public class TarImporter implements Runnable {
TarArchiveInputStream tar = null;
try {
tar = new TarArchiveInputStream(decodeStream());
tar = new TarArchiveInputStream(decodeStream(), "UTF-8");
TarArchiveEntry entry;
while ((entry = tar.getNextTarEntry()) != null) {
......@@ -105,6 +105,9 @@ public class TarImporter implements Runnable {
}
}
target.close();
} catch (final IOException e) {
// TODO: File name encoding errors are currently silently ignored (chars replaced with ?)
throw new ErrorResponse(400, "InvalidImportFormat", "Failed to extract archive: " + e.getMessage());
} catch (final ErrorResponse e) {
throw e;
} catch (final Exception e) {
......
package de.gwdg.cdstar.rest.v3.ingest;
import java.io.IOException;
import java.io.InputStream;
import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets;
import java.nio.file.Path;
import java.util.Arrays;
import java.util.HashSet;
......@@ -41,7 +43,7 @@ public class ZipImporter implements Runnable {
public void run() {
ZipFile zip = null;
try {
zip = new ZipFile(source.toFile());
zip = new ZipFile(source.toFile(), StandardCharsets.UTF_8);
for (final ZipEntry entry : Utils.iter(zip.entries())) {
if (entry.isDirectory())
continue;
......@@ -65,6 +67,8 @@ public class ZipImporter implements Runnable {
} catch (final NullPointerException e) {
// This happens for example if the name of a zip-entry is empty.
throw new ErrorResponse(400, "InvalidImportFormat", "Invalid zip file content");
} catch (final IOException e) {
throw new ErrorResponse(400, "InvalidImportFormat", "Failed to extract archive: "+e.getMessage());
} catch (final ErrorResponse e) {
throw e;
} catch (final Exception e) {
......
......@@ -12,14 +12,12 @@ import java.util.Arrays;
import java.util.Collection;
import java.util.List;
import jakarta.ws.rs.client.Entity;
import jakarta.ws.rs.core.Response.Status;
import org.apache.commons.compress.archivers.ArchiveOutputStream;
import org.apache.commons.compress.archivers.tar.TarArchiveEntry;
import org.apache.commons.compress.archivers.tar.TarArchiveOutputStream;
import org.apache.commons.compress.archivers.zip.ZipArchiveEntry;
import org.apache.commons.compress.archivers.zip.ZipArchiveOutputStream;
import org.junit.AssumptionViolatedException;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
......@@ -27,6 +25,8 @@ import org.junit.runners.Parameterized;
import org.junit.runners.Parameterized.Parameters;
import de.gwdg.cdstar.rest.BaseRestTest;
import jakarta.ws.rs.client.Entity;
import jakarta.ws.rs.core.Response.Status;
@RunWith(Parameterized.class)
public class ZipImportTest extends BaseRestTest {
......@@ -65,6 +65,7 @@ public class ZipImportTest extends BaseRestTest {
testArchive = new ArrayList<>();
testArchive.add(new FileEntry("example.txt", "text/plain", "example".getBytes()));
testArchive.add(new FileEntry("folder/file.json", "application/json", "{\"test\": \"value\"}".getBytes()));
testArchive.add(new FileEntry("i18n/測試.txt", "text/plain", "測試".getBytes()));
}
File makeZip() throws IOException, URISyntaxException {
......@@ -94,7 +95,7 @@ public class ZipImportTest extends BaseRestTest {
File makeZipFile() throws IOException, URISyntaxException {
final File out = testFolder.newFile();
try (ArchiveOutputStream zip = new ZipArchiveOutputStream(new FileOutputStream(out))) {
try (ZipArchiveOutputStream zip = new ZipArchiveOutputStream(new FileOutputStream(out))) {
for (final FileEntry file : testArchive) {
final ZipArchiveEntry e = new ZipArchiveEntry(file.name);
e.setSize(file.getSize());
......@@ -128,7 +129,7 @@ public class ZipImportTest extends BaseRestTest {
target("/v3/test/" + id).queryParam("order", "name").request().get();
assertStatus(Status.OK);
assertEquals(id, getJsonString("id"));
assertEquals(2, getJsonLong("file_count"));
assertEquals(3, getJsonLong("file_count"));
testArchive.forEach(this::assertPresent);
}
......@@ -143,7 +144,7 @@ public class ZipImportTest extends BaseRestTest {
target("/v3/test/" + id).queryParam("order", "name").request().get();
assertStatus(Status.OK);
assertEquals(id, getJsonString("id"));
assertEquals(2, getJsonLong("file_count"));
assertEquals(3, getJsonLong("file_count"));
testArchive.forEach(this::assertPresent);
}
......@@ -172,6 +173,32 @@ public class ZipImportTest extends BaseRestTest {
assertStatus(Status.BAD_REQUEST);
}
@Test
public void testBadUnicodeZip() throws IOException, URISyntaxException {
final File out = testFolder.newFile();
if (mediaType.equals("application/zip")) {
try (ZipArchiveOutputStream zip = new ZipArchiveOutputStream(new FileOutputStream(out))) {
zip.setEncoding("latin1");
zip.setFallbackToUTF8(false);
zip.putArchiveEntry(new ZipArchiveEntry("schöner_mist.txt"));
zip.closeArchiveEntry();
}
} else if (mediaType.equals("application/x-tar")) {
try (TarArchiveOutputStream zip = new TarArchiveOutputStream(new FileOutputStream(out), "latin1")) {
zip.putArchiveEntry(new TarArchiveEntry("schöner_mist.txt"));
zip.closeArchiveEntry();
}
throw new AssumptionViolatedException("TAR implementation cannot me bade to fail on encoding errors...");
} else {
fail("Unsupported media type: " + mediaType);
}
target("/v3/test/").request().post(Entity.entity(out, mediaType));
assertError(Status.BAD_REQUEST, "InvalidImportFormat");
}
@Test
public void testInclude() throws IOException, URISyntaxException {
testArchive.clear();
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment