diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index d44b92dfe..f1cee179b 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -7,3 +7,4 @@ * update to latest FHIRPath for older versions * clean up error handling when parsing structure maps * go-publish related changes +* FTP Client upload and logging improvements diff --git a/org.hl7.fhir.dstu2/src/main/java/org/hl7/fhir/dstu2/model/Base64BinaryType.java b/org.hl7.fhir.dstu2/src/main/java/org/hl7/fhir/dstu2/model/Base64BinaryType.java index fc0b00ca2..76260ef98 100644 --- a/org.hl7.fhir.dstu2/src/main/java/org/hl7/fhir/dstu2/model/Base64BinaryType.java +++ b/org.hl7.fhir.dstu2/src/main/java/org/hl7/fhir/dstu2/model/Base64BinaryType.java @@ -99,7 +99,7 @@ public class Base64BinaryType extends PrimitiveType { * @throws DataFormatException */ public void checkValidBase64(String toCheck) throws DataFormatException { - if (!Base64.isBase64(toCheck.getBytes())) { + if (!org.hl7.fhir.utilities.Base64.isBase64(toCheck.getBytes())) { throw new DataFormatException(""); } } diff --git a/org.hl7.fhir.dstu2016may/src/main/java/org/hl7/fhir/dstu2016may/model/Base64BinaryType.java b/org.hl7.fhir.dstu2016may/src/main/java/org/hl7/fhir/dstu2016may/model/Base64BinaryType.java index 4e5ac9c13..d4a087dd2 100644 --- a/org.hl7.fhir.dstu2016may/src/main/java/org/hl7/fhir/dstu2016may/model/Base64BinaryType.java +++ b/org.hl7.fhir.dstu2016may/src/main/java/org/hl7/fhir/dstu2016may/model/Base64BinaryType.java @@ -100,7 +100,7 @@ public class Base64BinaryType extends PrimitiveType { * @throws DataFormatException */ public void checkValidBase64(String toCheck) throws DataFormatException { - if (!Base64.isBase64(toCheck.getBytes())) { + if (!org.hl7.fhir.utilities.Base64.isBase64(toCheck.getBytes())) { throw new DataFormatException(""); } } diff --git a/org.hl7.fhir.dstu3/src/main/java/org/hl7/fhir/dstu3/model/Base64BinaryType.java b/org.hl7.fhir.dstu3/src/main/java/org/hl7/fhir/dstu3/model/Base64BinaryType.java index 99b7f325d..d21204884 100644 --- a/org.hl7.fhir.dstu3/src/main/java/org/hl7/fhir/dstu3/model/Base64BinaryType.java +++ b/org.hl7.fhir.dstu3/src/main/java/org/hl7/fhir/dstu3/model/Base64BinaryType.java @@ -152,7 +152,7 @@ public class Base64BinaryType extends PrimitiveType implements IPrimitiv * @throws DataFormatException */ public void checkValidBase64(String toCheck) throws DataFormatException { - if (!Base64.isBase64(toCheck.getBytes())) { + if (!org.hl7.fhir.utilities.Base64.isBase64(toCheck.getBytes())) { throw new DataFormatException(""); } } diff --git a/org.hl7.fhir.r4/src/main/java/org/hl7/fhir/r4/model/Base64BinaryType.java b/org.hl7.fhir.r4/src/main/java/org/hl7/fhir/r4/model/Base64BinaryType.java index 7cdceedf7..5b8e67df8 100644 --- a/org.hl7.fhir.r4/src/main/java/org/hl7/fhir/r4/model/Base64BinaryType.java +++ b/org.hl7.fhir.r4/src/main/java/org/hl7/fhir/r4/model/Base64BinaryType.java @@ -152,7 +152,7 @@ public class Base64BinaryType extends PrimitiveType implements IPrimitiv * @throws DataFormatException */ public void checkValidBase64(String toCheck) throws DataFormatException { - if (!Base64.isBase64(toCheck.getBytes())) { + if (!org.hl7.fhir.utilities.Base64.isBase64(toCheck.getBytes())) { throw new DataFormatException(""); } } diff --git a/org.hl7.fhir.r4b/src/main/java/org/hl7/fhir/r4b/model/Base64BinaryType.java b/org.hl7.fhir.r4b/src/main/java/org/hl7/fhir/r4b/model/Base64BinaryType.java index a1c73f821..6d6ee67da 100644 --- a/org.hl7.fhir.r4b/src/main/java/org/hl7/fhir/r4b/model/Base64BinaryType.java +++ b/org.hl7.fhir.r4b/src/main/java/org/hl7/fhir/r4b/model/Base64BinaryType.java @@ -156,7 +156,7 @@ public class Base64BinaryType extends PrimitiveType implements IPrimitiv * @throws DataFormatException */ public void checkValidBase64(String toCheck) throws DataFormatException { - if (!Base64.isBase64(toCheck.getBytes())) { + if (!org.hl7.fhir.utilities.Base64.isBase64(toCheck.getBytes())) { throw new DataFormatException(""); } } diff --git a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/model/Base64BinaryType.java b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/model/Base64BinaryType.java index 4e83d1a8b..1e1650ee8 100644 --- a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/model/Base64BinaryType.java +++ b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/model/Base64BinaryType.java @@ -156,7 +156,7 @@ public class Base64BinaryType extends PrimitiveType implements IPrimitiv * @throws DataFormatException */ public void checkValidBase64(String toCheck) throws DataFormatException { - if (!Base64.isBase64(toCheck.getBytes())) { + if (!org.hl7.fhir.utilities.Base64.isBase64(toCheck.getBytes())) { throw new DataFormatException(""); } } diff --git a/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/Base64.java b/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/Base64.java new file mode 100644 index 000000000..2d5f10767 --- /dev/null +++ b/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/Base64.java @@ -0,0 +1,40 @@ +package org.hl7.fhir.utilities; + +/** + * This is a partial duplication of org.apache.commons.codec.binary.Base64 + * + * It exists because Android compatibility only supports version 1.2 of that + * library, which only has the deprecated isArrayByteBase64. The use of + * isBase64 from this class will allow us to avoid using a deprecated method + * or hacking a solution that involves catching exceptions on decoding. + */ +public class Base64 { + + private static final byte[] DECODE_TABLE = new byte[]{-1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, 62, -1, 62, -1, 63, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, -1, -1, -1, -1, -1, -1, -1, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, -1, -1, -1, -1, 63, -1, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51}; + + public static boolean isBase64(byte octet) { + return octet == 61 || octet >= 0 && octet < DECODE_TABLE.length && DECODE_TABLE[octet] != -1; + } + + public static boolean isBase64(byte[] arrayOctet) { + for(int i = 0; i < arrayOctet.length; ++i) { + if (!isBase64(arrayOctet[i]) && !isWhiteSpace(arrayOctet[i])) { + return false; + } + } + + return true; + } + + protected static boolean isWhiteSpace(byte byteToCheck) { + switch (byteToCheck) { + case 9: + case 10: + case 13: + case 32: + return true; + default: + return false; + } + } +} diff --git a/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/FTPClient.java b/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/FTPClient.java index e21979ba6..4021afaef 100644 --- a/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/FTPClient.java +++ b/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/FTPClient.java @@ -1,26 +1,39 @@ package org.hl7.fhir.utilities; +import lombok.Getter; import org.apache.commons.net.ftp.FTP; import org.apache.commons.net.ftp.FTPReply; +import org.hl7.fhir.exceptions.FHIRException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; - -import java.io.File; import java.io.FileInputStream; import java.io.IOException; public class FTPClient { + private static final Logger logger = LoggerFactory.getLogger(FTPClient.class); + private final org.apache.commons.net.ftp.FTPClient clientImpl; - final String server; + @Getter + private final String server; - final String path; + @Getter + private final String path; - final String user; + private String resolvedPath = null; - final String password; + @Getter + private final String user; - final int port; + @Getter + private final String password; + + @Getter + private final int port; + + private final String remoteSeparator; /** * Connect to an FTP server @@ -36,17 +49,26 @@ public class FTPClient { protected FTPClient(String server, int port, String path, String user, String password) { this.server = server; this.port = port; - if (path.endsWith("/")) { - this.path = path; - } else { - this.path = path + "/"; - } + this.remoteSeparator = "/"; + this.path = buildPath(path); + this.user = user; this.password = password; clientImpl = new org.apache.commons.net.ftp.FTPClient(); } + private String buildPath(String path) { + if (path.length() == 0) { + return ""; + } + if (path.endsWith(remoteSeparator)) + { + return path; + } + return path + remoteSeparator; + } + /** * Connect to the server, throw an exception if it fails */ @@ -60,13 +82,17 @@ public class FTPClient { clientImpl.login(user, password); + checkForPositiveCompletionAndLogErrors("FTP server could not connect.", true); - int reply = clientImpl.getReplyCode(); + logger.debug("Initial Working directory: " + clientImpl.printWorkingDirectory()); - if(!FTPReply.isPositiveCompletion(reply)) { - clientImpl.disconnect(); - throw new IOException("FTP server refused connection. Reply code: " + reply); - } + clientImpl.changeWorkingDirectory(path); + + checkForPositiveCompletionAndLogErrors("FTP server could not establish default working directory", true); + + resolvedPath = clientImpl.printWorkingDirectory(); + + logger.debug("Resolved working directory: " + resolvedPath); } /** @@ -76,11 +102,53 @@ public class FTPClient { */ public void delete(String path) throws IOException { String resolvedPath = resolveRemotePath(path); + logger.debug("Deleting remote file: " + resolvedPath); clientImpl.deleteFile(resolvedPath); + checkForPositiveCompletionAndLogErrors("Error deleting file.", false); + logger.debug("Remote file deleted: " + resolvedPath); + } + + /** + * Takes a file path and creates all intermediate directories if they do not yet exist. + * @param filePath relative to the path provided in the constructor and including the file name + * @throws IOException + */ + protected void createRemotePathIfNotExists(String filePath) throws IOException { + String[] subPath = filePath.split(remoteSeparator); + try { + for (int i = 0 ; i < subPath.length - 1; i++){ + if (subPath[i].isEmpty() ) { + continue; + } + boolean exists = clientImpl.changeWorkingDirectory(subPath[i]); + if (!exists) { + logger.debug("Remote directory does not exist: " + clientImpl.printWorkingDirectory() + remoteSeparator + subPath[i]); + clientImpl.makeDirectory(subPath[i]); + clientImpl.changeWorkingDirectory(subPath[i]); + logger.debug("Made remote directory: " + clientImpl.printWorkingDirectory()); + } + }} catch (IOException e) { + throw new IOException("Error creating remote path: " + filePath, e); + } finally { + clientImpl.changeWorkingDirectory(this.resolvedPath); + } + } + + protected boolean remotePathExists(String path) throws IOException { + boolean output; + try { + output = clientImpl.changeWorkingDirectory(path); + } finally { + clientImpl.changeWorkingDirectory(this.resolvedPath); + } + return output; } private String resolveRemotePath(String path) { - return String.join("", this.path, path); + if (path.startsWith(remoteSeparator)) { + throw new IllegalArgumentException("Absolute remote path is not permitted. Path: " + path); + } + return String.join(remoteSeparator, path); } /** @@ -90,21 +158,59 @@ public class FTPClient { */ public void upload(String source, String path) throws IOException { String resolvedPath = resolveRemotePath(path); + logger.debug("Uploading file to remote path: " + resolvedPath); + createRemotePathIfNotExists(path); + FileInputStream localStream = new FileInputStream(source); clientImpl.setFileType(FTP.BINARY_FILE_TYPE); clientImpl.enterLocalPassiveMode(); clientImpl.storeFile( resolvedPath, localStream); localStream.close(); + checkForPositiveCompletionAndLogErrors("Error uploading file.", false); + logger.debug("Remote file uploaded: " + resolvedPath); + } + + private void checkForPositiveCompletionAndLogErrors(String localErrorMessage, boolean disconnectOnError) throws IOException { int reply = clientImpl.getReplyCode(); - if(!FTPReply.isPositiveCompletion(reply)) { - throw new IOException("Error uploading file. Reply code: " + reply); + if (FTPReply.isPositiveCompletion(reply)) { + return; } + + String remoteErrorMessage = clientImpl.getReplyString(); + if (disconnectOnError) { + clientImpl.disconnect(); + } + throw new IOException(localErrorMessage + " Reply code: " + reply + " Message: " + remoteErrorMessage); + + } public void disconnect() throws IOException { clientImpl.disconnect(); } + + public static void main(String[] args) throws IOException, FHIRException { + FTPClient ftp = new FTPClient(getNamedParam(args, "-upload-server"), getNamedParam(args, "-upload-path"), getNamedParam(args, "-upload-user"), getNamedParam(args, "-upload-password")); + ftp.connect(); + ftp.upload("/Users/grahamegrieve/temp/test.xml", "testing/test.xml"); + ftp.delete("testing/test.xml"); + ftp.disconnect(); + } + + private static String getNamedParam(String[] args, String param) { + boolean found = false; + for (String a : args) { + if (found) + return a; + if (a.equals(param)) { + found = true; + } + } + return null; + } + + } diff --git a/org.hl7.fhir.utilities/src/test/java/Base64Tests.java b/org.hl7.fhir.utilities/src/test/java/Base64Tests.java new file mode 100644 index 000000000..dab656b98 --- /dev/null +++ b/org.hl7.fhir.utilities/src/test/java/Base64Tests.java @@ -0,0 +1,27 @@ +import org.junit.jupiter.api.Test; + +import org.hl7.fhir.utilities.Base64; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +public class Base64Tests { + @Test + public void testIsArrayByteBase64() { + assertFalse(Base64.isBase64(new byte[] { Byte.MIN_VALUE })); + assertFalse(Base64.isBase64(new byte[] { -125 })); + assertFalse(Base64.isBase64(new byte[] { -10 })); + assertFalse(Base64.isBase64(new byte[] { 0 })); + assertFalse(Base64.isBase64(new byte[] { 64, Byte.MAX_VALUE })); + assertFalse(Base64.isBase64(new byte[] { Byte.MAX_VALUE })); + + assertTrue(Base64.isBase64(new byte[] { 'A' })); + + assertFalse(Base64.isBase64(new byte[] { 'A', Byte.MIN_VALUE })); + + assertTrue(Base64.isBase64(new byte[] { 'A', 'Z', 'a' })); + assertTrue(Base64.isBase64(new byte[] { '/', '=', '+' })); + + assertFalse(Base64.isBase64(new byte[] { '$' })); + } +} diff --git a/org.hl7.fhir.utilities/src/test/java/org/hl7/fhir/utilities/FTPClientTest.java b/org.hl7.fhir.utilities/src/test/java/org/hl7/fhir/utilities/FTPClientTest.java index 37d985343..c16d86040 100644 --- a/org.hl7.fhir.utilities/src/test/java/org/hl7/fhir/utilities/FTPClientTest.java +++ b/org.hl7.fhir.utilities/src/test/java/org/hl7/fhir/utilities/FTPClientTest.java @@ -3,13 +3,14 @@ package org.hl7.fhir.utilities; import org.apache.commons.io.IOUtils; import org.hl7.fhir.utilities.tests.ResourceLoaderTests; -import org.jetbrains.annotations.NotNull; + import org.junit.jupiter.api.*; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; import org.mockftpserver.fake.FakeFtpServer; import org.mockftpserver.fake.UserAccount; import org.mockftpserver.fake.filesystem.*; -import java.io.File; import java.io.IOException; import java.io.InputStream; import java.nio.charset.StandardCharsets; @@ -31,6 +32,7 @@ public class FTPClientTest implements ResourceLoaderTests { public static final String DUMMY_FILE_TO_UPLOAD = "dummyFileToUpload"; public static final int FAKE_FTP_PORT = 8021; public static final String DUMMY_FILE_CONTENT = "Dummy file content\nMore content\n"; + public static final String LOCALHOST = "localhost"; FakeFtpServer fakeFtpServer; @@ -69,8 +71,6 @@ public class FTPClientTest implements ResourceLoaderTests { } public void setupFakeFtpServer() throws IOException { - - fakeFtpServer = new FakeFtpServer(); fakeFtpServer.setServerControlPort(FAKE_FTP_PORT); fakeFtpServer.addUserAccount(new UserAccount(DUMMY_USER, DUMMY_PASSWORD, fakeFtpDirectory.toFile().getAbsolutePath())); @@ -108,6 +108,21 @@ public class FTPClientTest implements ResourceLoaderTests { fakeFtpServer.stop(); } + @ParameterizedTest + @CsvSource({"/", "/test", "/test", "/test/", "/test1/test2", "/test1/test2", "test", "test/", "test1/test2"}) + public void testValidRelativePaths(String path) { + FTPClient client = new FTPClient("localhost", path, DUMMY_USER, DUMMY_PASSWORD); + assertTrue(path.length() == client.getPath().length() || path.length() + 1 == client.getPath().length()); + assertTrue(client.getPath().startsWith(path)); + assertTrue(client.getPath().endsWith("/")); + } + + @Test + public void testEmptyRelativePath() { + FTPClient client = new FTPClient("localhost", "", DUMMY_USER, DUMMY_PASSWORD); + assertEquals("", client.getPath()); + } + @Test public void testDelete() throws IOException { @@ -120,9 +135,8 @@ public class FTPClientTest implements ResourceLoaderTests { assertFalse(fakeFtpServer.getFileSystem().exists(deleteFilePath)); } - @NotNull private static FTPClient connectToFTPClient() throws IOException { - FTPClient client = new FTPClient("localhost", FAKE_FTP_PORT, RELATIVE_PATH_1 + "/", DUMMY_USER, DUMMY_PASSWORD); + FTPClient client = new FTPClient(LOCALHOST, FAKE_FTP_PORT, RELATIVE_PATH_1, DUMMY_USER, DUMMY_PASSWORD); client.connect(); return client; } @@ -130,7 +144,7 @@ public class FTPClientTest implements ResourceLoaderTests { @Test public void testDelete2() throws IOException { - FTPClient client = connectToFTPClient2(); + FTPClient client = connectToFTPClient(); String deleteFilePath = dummyFileToDeletePath.toFile().getAbsolutePath(); assertTrue(fakeFtpServer.getFileSystem().exists(deleteFilePath)); @@ -140,12 +154,6 @@ public class FTPClientTest implements ResourceLoaderTests { client.disconnect(); } - @NotNull - private static FTPClient connectToFTPClient2() throws IOException { - FTPClient client = new FTPClient("localhost", FAKE_FTP_PORT, RELATIVE_PATH_1, DUMMY_USER, DUMMY_PASSWORD); - client.connect(); - return client; - } @Test public void testUpload() throws IOException { @@ -153,17 +161,58 @@ public class FTPClientTest implements ResourceLoaderTests { FTPClient client = connectToFTPClient(); String uploadFilePath = dummyUploadedFilePath.toFile().getAbsolutePath(); - assertFalse(fakeFtpServer.getFileSystem().exists(uploadFilePath)); client.upload(dummyFileToUploadPath.toFile().getAbsolutePath(), RELATIVE_PATH_2 + "/" + DUMMY_FILE_TO_UPLOAD); - assertTrue(fakeFtpServer.getFileSystem().exists(uploadFilePath)); + assertUploadedFileCorrect(uploadFilePath); + } - FileEntry fileEntry= (FileEntry)fakeFtpServer.getFileSystem().getEntry(uploadFilePath); + private void assertUploadedFileCorrect(String uploadedFilePath) throws IOException { + assertTrue(fakeFtpServer.getFileSystem().exists(uploadedFilePath)); + FileEntry fileEntry = (FileEntry)fakeFtpServer.getFileSystem().getEntry(uploadedFilePath); + assertNotNull(fileEntry); InputStream inputStream = fileEntry.createInputStream(); byte[] bytes = IOUtils.toByteArray(inputStream); String actualContent = new String(bytes, StandardCharsets.UTF_8); assertEquals(DUMMY_FILE_CONTENT,actualContent); } + + @Test + public void testCreateRemotePathDoesntExist() throws IOException { + FTPClient client = connectToFTPClient(); + + Path newPath1 = relativePath2.resolve("newPath1"); + Path newPath2 = newPath1.resolve("newPath2"); + + assertFalse(fakeFtpServer.getFileSystem().exists(newPath1.toFile().getAbsolutePath())); + assertFalse(fakeFtpServer.getFileSystem().exists(newPath2.toFile().getAbsolutePath())); + + client.createRemotePathIfNotExists(RELATIVE_PATH_2 + "/newPath1/newPath2/newFile.txt"); + + assertTrue(fakeFtpServer.getFileSystem().exists(newPath1.toFile().getAbsolutePath())); + assertTrue(fakeFtpServer.getFileSystem().exists(newPath2.toFile().getAbsolutePath())); + + } + + @Test + public void testUploadWherePathDoesntExist() throws IOException { + Path newPath1 = relativePath2.resolve("newPath1"); + Path newPath2 = newPath1.resolve("newPath2"); + + FTPClient client = connectToFTPClient(); + + Path uploadFilePath = newPath2.resolve(DUMMY_FILE_TO_UPLOAD); + assertFalse(fakeFtpServer.getFileSystem().exists(uploadFilePath.toFile().getAbsolutePath())); + + assertFalse(fakeFtpServer.getFileSystem().exists(newPath1.toFile().getAbsolutePath())); + assertFalse(fakeFtpServer.getFileSystem().exists(newPath2.toFile().getAbsolutePath())); + + client.upload(dummyFileToUploadPath.toFile().getAbsolutePath(),RELATIVE_PATH_2 + "/newPath1/newPath2/" + DUMMY_FILE_TO_UPLOAD); + + assertTrue(fakeFtpServer.getFileSystem().exists(newPath1.toFile().getAbsolutePath())); + assertTrue(fakeFtpServer.getFileSystem().exists(newPath2.toFile().getAbsolutePath())); + + assertUploadedFileCorrect(uploadFilePath.toFile().getAbsolutePath()); + } } diff --git a/pom.xml b/pom.xml index fa95b3d9f..455bd79f6 100644 --- a/pom.xml +++ b/pom.xml @@ -391,6 +391,29 @@ org.apache.maven.plugins maven-deploy-plugin + + + org.codehaus.mojo + animal-sniffer-maven-plugin + 1.22 + + + net.sf.androidscents.signature + android-api-level-26 + 8.0.0_r2 + + false + + + + check-android-26-compliance + test + + check + + + +