From 40d01e436a905e1c4ae4c9523b0adf06f53ce25d Mon Sep 17 00:00:00 2001 From: Pierre Pouchin Date: Thu, 8 Dec 2022 15:58:24 +0100 Subject: [PATCH] CodeQL fixes (#43) * Add fixes for CodeQL scanning * Check NumberFormatException --- .../igred/omero/annotations/TableWrapper.java | 54 +++++++++++++++-- .../java/fr/igred/omero/roi/ROIWrapper.java | 13 +++- src/test/java/fr/igred/omero/BasicTest.java | 15 +++-- .../fr/igred/omero/annotations/TableTest.java | 59 +++++++++++++++++++ .../igred/omero/repository/DatasetTest.java | 6 +- .../fr/igred/omero/repository/ImageTest.java | 44 +++++++------- .../igred/omero/repository/ProjectTest.java | 10 ++-- 7 files changed, 157 insertions(+), 44 deletions(-) diff --git a/src/main/java/fr/igred/omero/annotations/TableWrapper.java b/src/main/java/fr/igred/omero/annotations/TableWrapper.java index 175acff3..bc04a579 100644 --- a/src/main/java/fr/igred/omero/annotations/TableWrapper.java +++ b/src/main/java/fr/igred/omero/annotations/TableWrapper.java @@ -207,6 +207,46 @@ public TableWrapper(Client client, ResultsTable results, Long imageId, List index2roi = new HashMap<>(ijRois.size()); Map roiName2roi = new HashMap<>(ijRois.size()); for (Roi ijRoi : ijRois) { - String index = ijRoi.getProperty(roiProperty); - String id = ijRoi.getProperty(roiIdProperty); + Integer index = safeParseInt(ijRoi.getProperty(roiProperty)); + Long id = safeParseLong(ijRoi.getProperty(roiIdProperty)); if (id != null) { - roiName2roi.put(ijRoi.getName(), id2roi.get(Long.parseLong(id))); - if (index != null) index2roi.putIfAbsent(Integer.parseInt(index), id2roi.get(Long.parseLong(id))); + ROIData roi = id2roi.get(id); + roiName2roi.put(ijRoi.getName(), roi); + if (index != null) index2roi.putIfAbsent(index, roi); } } @@ -702,12 +743,13 @@ public TableData createTable() { */ public void saveAs(String path, char delimiter) throws FileNotFoundException, UnsupportedEncodingException { StringBuilder sb = new StringBuilder(10 * columnCount * rowCount); - File f = new File(path); + + File file = new File(path); String sol = "\""; String sep = String.format("\"%c\"", delimiter); String eol = String.format("\"%n"); - try (PrintWriter stream = new PrintWriter(f, StandardCharsets.UTF_8.name())) { + try (PrintWriter stream = new PrintWriter(file, StandardCharsets.UTF_8.name())) { sb.append(sol); for (int j = 0; j < columnCount; j++) { sb.append(columns[j].getName()); diff --git a/src/main/java/fr/igred/omero/roi/ROIWrapper.java b/src/main/java/fr/igred/omero/roi/ROIWrapper.java index 324a5298..0f40172b 100644 --- a/src/main/java/fr/igred/omero/roi/ROIWrapper.java +++ b/src/main/java/fr/igred/omero/roi/ROIWrapper.java @@ -143,9 +143,16 @@ public static List fromImageJ(List ijRois, Str for (int i = 0; i < ijRois.size(); i++) { String value = ijRois.get(i).getProperty(property); if (value != null && INT_PATTERN.matcher(value).matches()) { - long id = Long.parseLong(value); - rois4D.computeIfAbsent(id, val -> new ROIWrapper()); - shape2roi.put(i, rois4D.get(id)); + Long id = null; + try { + id = Long.parseLong(value); + } catch (NumberFormatException ignored) { + // DO NOTHING + } + if (id != null) { + rois4D.computeIfAbsent(id, val -> new ROIWrapper()); + shape2roi.put(i, rois4D.get(id)); + } } else { shape2roi.put(i, new ROIWrapper()); } diff --git a/src/test/java/fr/igred/omero/BasicTest.java b/src/test/java/fr/igred/omero/BasicTest.java index 3c7e3999..b0895d10 100644 --- a/src/test/java/fr/igred/omero/BasicTest.java +++ b/src/test/java/fr/igred/omero/BasicTest.java @@ -25,6 +25,7 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.security.SecureRandom; +import java.util.Random; import java.util.logging.Logger; @@ -60,10 +61,11 @@ public abstract class BasicTest { protected static final double DOUBLE_PRECISION = 10.0e-15; + protected static final Random SECURE_RANDOM = new SecureRandom(); + protected static File createFile(String filename) throws IOException { - @SuppressWarnings("AccessOfSystemProperties") - String tmpdir = System.getProperty("java.io.tmpdir") + File.separator; + String tmpdir = Files.createTempDirectory(null).toString(); File file = new File(tmpdir + File.separator + filename); if (!file.createNewFile()) { @@ -78,7 +80,7 @@ protected static File createRandomFile(String filename) throws IOException { File file = createFile(filename); byte[] array = new byte[size]; - new SecureRandom().nextBytes(array); + SECURE_RANDOM.nextBytes(array); String generatedString = new String(array, StandardCharsets.UTF_8); try (PrintStream out = new PrintStream(Files.newOutputStream(file.toPath()), false, "UTF-8")) { out.print(generatedString); @@ -88,7 +90,12 @@ protected static File createRandomFile(String filename) throws IOException { protected static void removeFile(File file) throws IOException { - if (!file.delete()) { + File parent = file.getParentFile(); + if (file.delete()) { + if (!parent.delete()) { + logger.warning("\"" + parent.getCanonicalPath() + "\" could not be deleted."); + } + } else { logger.severe("\"" + file.getCanonicalPath() + "\" could not be deleted."); } } diff --git a/src/test/java/fr/igred/omero/annotations/TableTest.java b/src/test/java/fr/igred/omero/annotations/TableTest.java index e98dfa08..808dcf1b 100644 --- a/src/test/java/fr/igred/omero/annotations/TableTest.java +++ b/src/test/java/fr/igred/omero/annotations/TableTest.java @@ -881,6 +881,65 @@ void testAddRowsFromIJResultsError() throws Exception { } + @Test + void testNumberFormatException() throws Exception { + ImageWrapper image = client.getImage(IMAGE1.id); + + ROIWrapper roi = new ROIWrapper(); + + roi.setImage(image); + + for (int i = 0; i < 4; i++) { + RectangleWrapper rectangle = new RectangleWrapper(); + rectangle.setCoordinates(i * 2, i * 2, 10, 10); + rectangle.setZ(i); + rectangle.setT(0); + rectangle.setC(0); + + roi.addShape(rectangle); + } + + image.saveROI(client, roi); + + List rois = image.getROIs(client); + List ijRois = ROIWrapper.toImageJ(rois, null); + ijRois.get(0).setProperty(ROIWrapper.IJ_PROPERTY, "tutu"); + ijRois.get(1).setProperty(ROIWrapper.IJ_PROPERTY, "tutu"); + ijRois.get(2).setProperty(ROIWrapper.IJ_PROPERTY, "tutu"); + ijRois.get(3).setProperty(ROIWrapper.ijIDProperty(ROIWrapper.IJ_PROPERTY), "tata"); + + ResultsTable results = new ResultsTable(); + results.incrementCounter(); + results.setLabel(image.getName(), 0); + results.setValue("Image", 0, image.getName()); + results.setValue(ROIWrapper.IJ_PROPERTY, 0, 1); + results.setValue("Volume", 0, volume1); + results.setValue("Volume Unit", 0, "µm^3"); + + TableWrapper table = new TableWrapper(client, results, IMAGE1.id, ijRois); + + Object[][] data = table.getData(); + assertEquals(1, table.getRowCount()); + assertEquals(image.getId(), ((DataObject) data[0][0]).getId()); + assertEquals(image.getName(), data[1][0]); + assertEquals(image.getName(), data[2][0]); + assertEquals(1.0, data[3][0]); + assertEquals(volume1, (Double) data[4][0], Double.MIN_VALUE); + assertEquals("µm^3", data[5][0]); + + image.addTable(client, table); + + assertNotNull(table.getFileId()); + + client.delete(table); + + for (ROIWrapper r : rois) { + client.delete(r); + } + assertEquals(0, image.getROIs(client).size()); + } + + @Test void testAddRowsFromIJResultsInverted() throws Exception { ImageWrapper image = client.getImage(IMAGE1.id); diff --git a/src/test/java/fr/igred/omero/repository/DatasetTest.java b/src/test/java/fr/igred/omero/repository/DatasetTest.java index 690224c8..7ce15855 100644 --- a/src/test/java/fr/igred/omero/repository/DatasetTest.java +++ b/src/test/java/fr/igred/omero/repository/DatasetTest.java @@ -66,7 +66,7 @@ void testCreateDatasetAndDeleteIt2() throws Exception { DatasetWrapper dataset = new DatasetWrapper("To delete", description); - Long id = project.addDataset(client, dataset).getId(); + long id = project.addDataset(client, dataset).getId(); DatasetWrapper checkDataset = client.getDataset(id); client.delete(checkDataset); @@ -320,10 +320,10 @@ void testAddFileDataset() throws Exception { DatasetWrapper dataset = client.getDataset(DATASET1.id); File file = createRandomFile("test_dataset.txt"); - Long id = dataset.addFile(client, file); + long id = dataset.addFile(client, file); removeFile(file); client.deleteFile(id); - assertNotEquals(0L, id.longValue()); + assertNotEquals(0L, id); } diff --git a/src/test/java/fr/igred/omero/repository/ImageTest.java b/src/test/java/fr/igred/omero/repository/ImageTest.java index ab081f5e..f14acfc5 100644 --- a/src/test/java/fr/igred/omero/repository/ImageTest.java +++ b/src/test/java/fr/igred/omero/repository/ImageTest.java @@ -35,7 +35,6 @@ import java.awt.image.BufferedImage; import java.io.File; import java.nio.file.Files; -import java.security.SecureRandom; import java.time.LocalDate; import java.time.LocalDateTime; import java.time.format.DateTimeFormatter; @@ -44,7 +43,6 @@ import java.util.List; import java.util.Map; import java.util.NoSuchElementException; -import java.util.Random; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -273,15 +271,14 @@ void testToImagePlusBound() throws Exception { int[] zBounds = {0, 2}; int[] tBounds = {0, 2}; - Random random = new SecureRandom(); - xBounds[0] = random.nextInt(lowXY); - yBounds[0] = random.nextInt(lowXY); - cBounds[0] = random.nextInt(3); - tBounds[0] = random.nextInt(5); - xBounds[1] = random.nextInt(highXY - xBounds[0]) + xBounds[0] + 5; - yBounds[1] = random.nextInt(highXY - yBounds[0]) + yBounds[0] + 5; - cBounds[1] = random.nextInt(3 - cBounds[0]) + cBounds[0] + 2; - tBounds[1] = random.nextInt(5 - tBounds[0]) + tBounds[0] + 2; + xBounds[0] = SECURE_RANDOM.nextInt(lowXY); + yBounds[0] = SECURE_RANDOM.nextInt(lowXY); + cBounds[0] = SECURE_RANDOM.nextInt(3); + tBounds[0] = SECURE_RANDOM.nextInt(5); + xBounds[1] = SECURE_RANDOM.nextInt(highXY - xBounds[0]) + xBounds[0] + 5; + yBounds[1] = SECURE_RANDOM.nextInt(highXY - yBounds[0]) + yBounds[0] + 5; + cBounds[1] = SECURE_RANDOM.nextInt(3 - cBounds[0]) + cBounds[0] + 2; + tBounds[1] = SECURE_RANDOM.nextInt(5 - tBounds[0]) + tBounds[0] + 2; String fake = "8bit-unsigned&pixelType=uint8&sizeZ=3&sizeC=5&sizeT=7&sizeX=512&sizeY=512.fake"; File fakeFile = createFile(fake); @@ -481,18 +478,19 @@ void testAddFileImage() throws Exception { long id = image.addFile(client, file); List files = image.getFileAnnotations(client); - for (FileAnnotationWrapper f : files) { - if (f.getId() == id) { - assertEquals(file.getName(), f.getFileName()); - assertEquals("txt", f.getFileFormat()); - assertEquals("text/plain", f.getOriginalMimetype()); - assertEquals("text/plain", f.getServerFileMimetype()); - assertEquals("Plain Text Document", f.getFileKind()); - assertEquals(file.getParent() + File.separator, f.getContentAsString()); - assertEquals(file.getParent() + File.separator, f.getFilePath()); - assertFalse(f.isMovieFile()); - - File uploadedFile = f.getFile(client, "." + File.separator + "uploaded.txt"); + for (FileAnnotationWrapper fileAnn : files) { + if (fileAnn.getId() == id) { + assertEquals(file.getName(), fileAnn.getFileName()); + assertEquals("txt", fileAnn.getFileFormat()); + assertEquals("text/plain", fileAnn.getOriginalMimetype()); + assertEquals("text/plain", fileAnn.getServerFileMimetype()); + assertEquals("Plain Text Document", fileAnn.getFileKind()); + assertEquals(file.getParent() + File.separator, fileAnn.getContentAsString()); + assertEquals(file.getParent() + File.separator, fileAnn.getFilePath()); + assertFalse(fileAnn.isMovieFile()); + + String tmpdir = Files.createTempDirectory(null).toString(); + File uploadedFile = fileAnn.getFile(client, tmpdir + File.separator + "uploaded.txt"); List expectedLines = Files.readAllLines(file.toPath()); List lines = Files.readAllLines(uploadedFile.toPath()); diff --git a/src/test/java/fr/igred/omero/repository/ProjectTest.java b/src/test/java/fr/igred/omero/repository/ProjectTest.java index 5e420d2b..c39182e1 100644 --- a/src/test/java/fr/igred/omero/repository/ProjectTest.java +++ b/src/test/java/fr/igred/omero/repository/ProjectTest.java @@ -296,9 +296,9 @@ void testCopyAnnotations() throws Exception { File file = createRandomFile("test_project.txt"); - Long fileId = project1.addFile(client, file); + long fileId = project1.addFile(client, file); removeFile(file); - assertNotEquals(0L, fileId.longValue()); + assertNotEquals(0L, fileId); TagAnnotationWrapper tag = new TagAnnotationWrapper(client, "CopyTestTag", "Copy annotations"); project1.addTag(client, tag); @@ -341,9 +341,9 @@ void testCopyFileAnnotation() throws Exception { File file = createRandomFile("test_project.txt"); - Long fileId = project1.addFile(client, file); + long fileId = project1.addFile(client, file); removeFile(file); - assertNotEquals(0L, fileId.longValue()); + assertNotEquals(0L, fileId); List files = project1.getFileAnnotations(client); assertEquals(1, files.size()); @@ -356,7 +356,7 @@ void testCopyFileAnnotation() throws Exception { client.deleteFile(fileId); client.delete(project2); - assertNotEquals(0L, fileId.longValue()); + assertNotEquals(0L, fileId); }