From bb1eca07a470961839d5202473dc76483d17c674 Mon Sep 17 00:00:00 2001 From: Ryan Mulligan Date: Tue, 31 Mar 2015 14:13:49 -0700 Subject: [PATCH 1/2] improve already downloaded contribution file check When downloading a file, it's possible that a corrupted archive could be bigger, smaller, or the same size as a non-corrupted archive, so it is not good to rely on the archive file size. Instead, the checksum can be used to verify if the already downloaded file is viable for reuse. --- .../DownloadableContributionsDownloader.java | 26 +++++++++++++++---- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/arduino-core/src/cc/arduino/contributions/packages/DownloadableContributionsDownloader.java b/arduino-core/src/cc/arduino/contributions/packages/DownloadableContributionsDownloader.java index 1a963254b30..eae3429b75a 100644 --- a/arduino-core/src/cc/arduino/contributions/packages/DownloadableContributionsDownloader.java +++ b/arduino-core/src/cc/arduino/contributions/packages/DownloadableContributionsDownloader.java @@ -57,17 +57,14 @@ public File download(DownloadableContribution contribution, // Ensure the existence of staging folder stagingFolder.mkdirs(); - // Need to download or resume downloading? - if (!outputFile.isFile() || (outputFile.length() < contribution.getSize())) { + if (!alreadyDownloadedFileViable(outputFile, contribution)) { download(url, outputFile, progress, statusText); } // Test checksum progress.setStatus(_("Verifying archive integrity...")); onProgress(progress); - String checksum = contribution.getChecksum(); - String algo = checksum.split(":")[0]; - if (!FileHash.hash(outputFile, algo).equals(checksum)) { + if (checksumMatches(outputFile, contribution)) { throw new Exception(_("CRC doesn't match. File is corrupted.")); } @@ -76,6 +73,25 @@ public File download(DownloadableContribution contribution, return outputFile; } + private boolean checksumMatches(File outputFile, + DownloadableContribution contribution) + throws Exception { + final String checksum = contribution.getChecksum(); + final String algo = checksum.split(":")[0]; + + return FileHash.hash(outputFile, algo).equals(checksum); + } + + private boolean alreadyDownloadedFileViable + (File outputFile, + DownloadableContribution contribution) throws Exception { + if (!outputFile.isFile()) { + return false; + } + + return checksumMatches(outputFile, contribution); + } + public void download(URL url, File tmpFile, final Progress progress, final String statusText) throws Exception { FileDownloader downloader = new FileDownloader(url, tmpFile); From 7ebecc254ea6806d492ca7da7050a50b4705f203 Mon Sep 17 00:00:00 2001 From: Ryan Mulligan Date: Mon, 6 Apr 2015 09:51:30 -0700 Subject: [PATCH 2/2] DownloadableContributionsDownloader: improve downloading logic and performance Renamed alreadyDownloadedFileViable to fileAlreadyDownloaded to better reflect what is going on with the download Added file length check to improve performance Reordered the logic of DownloadableContributionsDownloader.download to avoid doing multiple checksums in the case of a good file --- .../DownloadableContributionsDownloader.java | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/arduino-core/src/cc/arduino/contributions/packages/DownloadableContributionsDownloader.java b/arduino-core/src/cc/arduino/contributions/packages/DownloadableContributionsDownloader.java index eae3429b75a..2b8898619c4 100644 --- a/arduino-core/src/cc/arduino/contributions/packages/DownloadableContributionsDownloader.java +++ b/arduino-core/src/cc/arduino/contributions/packages/DownloadableContributionsDownloader.java @@ -57,10 +57,14 @@ public File download(DownloadableContribution contribution, // Ensure the existence of staging folder stagingFolder.mkdirs(); - if (!alreadyDownloadedFileViable(outputFile, contribution)) { - download(url, outputFile, progress, statusText); + if (fileAlreadyDownloaded(outputFile, contribution)) { + contribution.setDownloaded(true); + contribution.setDownloadedFile(outputFile); + return outputFile; } + download(url, outputFile, progress, statusText); + // Test checksum progress.setStatus(_("Verifying archive integrity...")); onProgress(progress); @@ -82,13 +86,17 @@ private boolean checksumMatches(File outputFile, return FileHash.hash(outputFile, algo).equals(checksum); } - private boolean alreadyDownloadedFileViable + private boolean fileAlreadyDownloaded (File outputFile, DownloadableContribution contribution) throws Exception { if (!outputFile.isFile()) { return false; } + if (outputFile.length() != contribution.getSize()) { + return false; + } + return checksumMatches(outputFile, contribution); }