From d08932334263bdc6534fa42e32e0fec6b907232d Mon Sep 17 00:00:00 2001 From: Mattia Bertorello Date: Fri, 28 Jun 2019 17:16:08 +0200 Subject: [PATCH] Fix possible empty files during the download of the package index --- app/src/processing/app/Base.java | 20 +++--- .../DownloadableContributionsDownloader.java | 66 +++++++++++++++++-- .../contributions/GZippedJsonDownloader.java | 15 +++-- .../contributions/SignatureVerifier.java | 9 +++ .../libraries/LibraryInstaller.java | 14 ++-- .../packages/ContributionInstaller.java | 56 ++++------------ .../arduino/utils/network/FileDownloader.java | 7 +- arduino-core/src/processing/app/Platform.java | 3 - 8 files changed, 116 insertions(+), 74 deletions(-) diff --git a/app/src/processing/app/Base.java b/app/src/processing/app/Base.java index ef4d9b30a..d37dcf4fd 100644 --- a/app/src/processing/app/Base.java +++ b/app/src/processing/app/Base.java @@ -26,9 +26,11 @@ import cc.arduino.Compiler; import cc.arduino.Constants; import cc.arduino.UpdatableBoardsLibsFakeURLsHandler; import cc.arduino.UploaderUtils; -import cc.arduino.packages.Uploader; import cc.arduino.contributions.*; -import cc.arduino.contributions.libraries.*; +import cc.arduino.contributions.libraries.ContributedLibrary; +import cc.arduino.contributions.libraries.LibrariesIndexer; +import cc.arduino.contributions.libraries.LibraryInstaller; +import cc.arduino.contributions.libraries.LibraryOfSameTypeComparator; import cc.arduino.contributions.libraries.ui.LibraryManagerUI; import cc.arduino.contributions.packages.ContributedPlatform; import cc.arduino.contributions.packages.ContributionInstaller; @@ -36,20 +38,17 @@ import cc.arduino.contributions.packages.ContributionsIndexer; import cc.arduino.contributions.packages.ui.ContributionManagerUI; import cc.arduino.files.DeleteFilesOnShutdown; import cc.arduino.packages.DiscoveryManager; +import cc.arduino.packages.Uploader; import cc.arduino.view.Event; import cc.arduino.view.JMenuUtils; import cc.arduino.view.SplashScreenHelper; - +import com.github.zafarkhaja.semver.Version; import org.apache.commons.compress.utils.IOUtils; import org.apache.commons.lang3.StringUtils; - -import com.github.zafarkhaja.semver.Version; - import processing.app.debug.TargetBoard; import processing.app.debug.TargetPackage; import processing.app.debug.TargetPlatform; import processing.app.helpers.*; -import processing.app.helpers.OSUtils; import processing.app.helpers.filefilters.OnlyDirs; import processing.app.helpers.filefilters.OnlyFilesWithExtension; import processing.app.javax.swing.filechooser.FileNameExtensionFilter; @@ -67,9 +66,9 @@ import javax.swing.*; import java.awt.*; import java.awt.event.*; import java.io.*; -import java.util.*; import java.util.List; import java.util.Timer; +import java.util.*; import java.util.logging.Handler; import java.util.logging.Level; import java.util.logging.Logger; @@ -286,7 +285,8 @@ public class Base { pdeKeywords = new PdeKeywords(); pdeKeywords.reload(); - contributionInstaller = new ContributionInstaller(BaseNoGui.getPlatform(), new GPGDetachedSignatureVerifier()); + final GPGDetachedSignatureVerifier gpgDetachedSignatureVerifier = new GPGDetachedSignatureVerifier(); + contributionInstaller = new ContributionInstaller(BaseNoGui.getPlatform(), gpgDetachedSignatureVerifier); libraryInstaller = new LibraryInstaller(BaseNoGui.getPlatform()); parser.parseArgumentsPhase2(); @@ -301,7 +301,7 @@ public class Base { if (parser.isInstallBoard()) { ContributionsIndexer indexer = new ContributionsIndexer( BaseNoGui.getSettingsFolder(), BaseNoGui.getHardwareFolder(), - BaseNoGui.getPlatform(), new GPGDetachedSignatureVerifier()); + BaseNoGui.getPlatform(), gpgDetachedSignatureVerifier); ProgressListener progressListener = new ConsoleProgressListener(); List downloadedPackageIndexFiles = contributionInstaller.updateIndex(progressListener); diff --git a/arduino-core/src/cc/arduino/contributions/DownloadableContributionsDownloader.java b/arduino-core/src/cc/arduino/contributions/DownloadableContributionsDownloader.java index 3157514f8..07cbbc1a8 100644 --- a/arduino-core/src/cc/arduino/contributions/DownloadableContributionsDownloader.java +++ b/arduino-core/src/cc/arduino/contributions/DownloadableContributionsDownloader.java @@ -30,20 +30,23 @@ package cc.arduino.contributions; import cc.arduino.utils.FileHash; +import cc.arduino.utils.MultiStepProgress; import cc.arduino.utils.Progress; import cc.arduino.utils.network.FileDownloader; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import processing.app.BaseNoGui; import java.io.File; import java.net.URL; -import java.nio.file.Files; -import java.nio.file.LinkOption; -import java.nio.file.Path; -import java.nio.file.Paths; +import java.nio.file.*; +import java.util.List; import static processing.app.I18n.format; import static processing.app.I18n.tr; public class DownloadableContributionsDownloader { + private static Logger log = LoggerFactory.getLogger(DownloadableContributionsDownloader.class); private final File stagingFolder; @@ -140,4 +143,59 @@ public class DownloadableContributionsDownloader { } } + public void downloadIndexAndSignature(MultiStepProgress progress, List downloadedFilesAccumulator, String packageIndexUrlString, ProgressListener progressListener, SignatureVerifier signatureVerifier) throws Exception { + + // Extract the file name from the url + URL packageIndexUrl = new URL(packageIndexUrlString); + URL packageIndexSignatureUrl = new URL(packageIndexUrlString + ".sig"); + String[] urlPathParts = packageIndexUrl.getFile().split("/"); + File packageIndex = BaseNoGui.indexer.getIndexFile(urlPathParts[urlPathParts.length - 1]); + // Signature file name + File packageIndexSignature = BaseNoGui.indexer.getIndexFile(urlPathParts[urlPathParts.length - 1] + ".sig"); + + final String statusText = tr("Downloading platforms index..."); + downloadedFilesAccumulator.add(packageIndex.getName()); + + // Create temp files + File packageIndexTemp = File.createTempFile(packageIndexUrl.getPath(), ".tmp"); + File packageIndexSignatureTemp = File.createTempFile(packageIndexSignatureUrl.getPath(), ".tmp"); + try { + // Download package index + download(packageIndexUrl, packageIndexTemp, progress, statusText, progressListener, true); + try { + // Download signature + download(packageIndexSignatureUrl, packageIndexSignatureTemp, progress, statusText, progressListener, true); + + // Verify the signature before move the files + boolean signatureVerified = signatureVerifier.isSigned(packageIndexTemp, packageIndexSignatureTemp); + if (signatureVerified) { + // Move if the signature is ok + Files.move(packageIndexTemp.toPath(), packageIndex.toPath(), StandardCopyOption.REPLACE_EXISTING); + Files.move(packageIndexSignatureTemp.toPath(), packageIndexSignature.toPath(), StandardCopyOption.REPLACE_EXISTING); + downloadedFilesAccumulator.add(packageIndexSignature.getName()); + } else { + downloadedFilesAccumulator.remove(packageIndex.getName()); + log.error("{} file signature verification failed. File ignored.", packageIndexSignatureUrl); + System.err.println(format(tr("{0} file signature verification failed. File ignored."), packageIndexUrlString)); + + } + } catch (Exception e) { + log.error("Cannot download the signature from {} the package will be install in any case", packageIndexSignatureUrl, e); + if (packageIndexTemp.length() > 0) { + Files.move(packageIndexTemp.toPath(), packageIndex.toPath(), StandardCopyOption.REPLACE_EXISTING); + } else { + log.error("The temporarily package index file is empty (path:{},url:{}), It cannot be move there {} ", + packageIndexTemp.toPath(), packageIndexUrlString, packageIndex.toPath()); + } + } + + } catch (Exception e) { + downloadedFilesAccumulator.remove(packageIndex.getName()); + throw e; + } finally { + // Delete useless temp file + Files.deleteIfExists(packageIndexTemp.toPath()); + Files.deleteIfExists(packageIndexSignatureTemp.toPath()); + } + } } diff --git a/arduino-core/src/cc/arduino/contributions/GZippedJsonDownloader.java b/arduino-core/src/cc/arduino/contributions/GZippedJsonDownloader.java index 6b6f38123..0eb53fc25 100644 --- a/arduino-core/src/cc/arduino/contributions/GZippedJsonDownloader.java +++ b/arduino-core/src/cc/arduino/contributions/GZippedJsonDownloader.java @@ -29,6 +29,7 @@ package cc.arduino.contributions; +import cc.arduino.Constants; import cc.arduino.utils.Progress; import org.apache.commons.compress.compressors.gzip.GzipCompressorInputStream; import org.apache.commons.compress.compressors.gzip.GzipUtils; @@ -36,6 +37,7 @@ import org.apache.commons.compress.utils.IOUtils; import java.io.*; import java.net.URL; +import java.nio.file.Files; public class GZippedJsonDownloader { @@ -50,17 +52,20 @@ public class GZippedJsonDownloader { } public void download(File tmpFile, Progress progress, String statusText, ProgressListener progressListener) throws Exception { + File gzipTmpFile = null; try { - File gzipTmpFile = new File(tmpFile.getParentFile(), GzipUtils.getCompressedFilename(tmpFile.getName())); + gzipTmpFile = File.createTempFile(new URL(Constants.LIBRARY_INDEX_URL_GZ).getPath(), GzipUtils.getCompressedFilename(tmpFile.getName())); // remove eventual leftovers from previous downloads - if (gzipTmpFile.exists()) { - gzipTmpFile.delete(); - } + Files.deleteIfExists(gzipTmpFile.toPath()); + new JsonDownloader(downloader, gzippedUrl).download(gzipTmpFile, progress, statusText, progressListener); decompress(gzipTmpFile, tmpFile); - gzipTmpFile.delete(); } catch (Exception e) { new JsonDownloader(downloader, url).download(tmpFile, progress, statusText, progressListener); + } finally { + if (gzipTmpFile != null) { + Files.deleteIfExists(gzipTmpFile.toPath()); + } } } diff --git a/arduino-core/src/cc/arduino/contributions/SignatureVerifier.java b/arduino-core/src/cc/arduino/contributions/SignatureVerifier.java index 6e2a80626..a4ea7a7ba 100644 --- a/arduino-core/src/cc/arduino/contributions/SignatureVerifier.java +++ b/arduino-core/src/cc/arduino/contributions/SignatureVerifier.java @@ -50,6 +50,15 @@ public abstract class SignatureVerifier { } } + public boolean isSigned(File indexFile, File signature) { + try { + return verify(indexFile, signature, new File(BaseNoGui.getContentFile("lib"), "public.gpg.key")); + } catch (Exception e) { + BaseNoGui.showWarning(e.getMessage(), e.getMessage(), e); + return false; + } + } + protected abstract boolean verify(File signedFile, File signature, File publicKey) throws IOException; } diff --git a/arduino-core/src/cc/arduino/contributions/libraries/LibraryInstaller.java b/arduino-core/src/cc/arduino/contributions/libraries/LibraryInstaller.java index 4b4fb7f7d..a9e35b005 100644 --- a/arduino-core/src/cc/arduino/contributions/libraries/LibraryInstaller.java +++ b/arduino-core/src/cc/arduino/contributions/libraries/LibraryInstaller.java @@ -43,6 +43,8 @@ import processing.app.helpers.FileUtils; import java.io.File; import java.io.IOException; import java.net.URL; +import java.nio.file.Files; +import java.nio.file.StandardCopyOption; import java.util.Optional; import static processing.app.I18n.tr; @@ -61,10 +63,11 @@ public class LibraryInstaller { DownloadableContributionsDownloader downloader = new DownloadableContributionsDownloader(BaseNoGui.librariesIndexer.getStagingFolder()); // Step 1: Download index File outputFile = BaseNoGui.librariesIndexer.getIndexFile(); - File tmpFile = new File(outputFile.getAbsolutePath() + ".tmp"); + // Create temp files + File libraryIndexTemp = File.createTempFile(new URL(Constants.LIBRARY_INDEX_URL).getPath(), ".tmp"); try { GZippedJsonDownloader gZippedJsonDownloader = new GZippedJsonDownloader(downloader, new URL(Constants.LIBRARY_INDEX_URL), new URL(Constants.LIBRARY_INDEX_URL_GZ)); - gZippedJsonDownloader.download(tmpFile, progress, tr("Downloading libraries index..."), progressListener); + gZippedJsonDownloader.download(libraryIndexTemp, progress, tr("Downloading libraries index..."), progressListener); } catch (InterruptedException e) { // Download interrupted... just exit return; @@ -74,10 +77,9 @@ public class LibraryInstaller { // TODO: Check downloaded index // Replace old index with the updated one - if (outputFile.exists()) - outputFile.delete(); - if (!tmpFile.renameTo(outputFile)) - throw new Exception(tr("An error occurred while updating libraries index!")); + if (libraryIndexTemp.length() > 0) { + Files.move(libraryIndexTemp.toPath(), outputFile.toPath(), StandardCopyOption.REPLACE_EXISTING); + } // Step 2: Parse index BaseNoGui.librariesIndexer.parseIndex(); diff --git a/arduino-core/src/cc/arduino/contributions/packages/ContributionInstaller.java b/arduino-core/src/cc/arduino/contributions/packages/ContributionInstaller.java index c75d1352a..4bd8f766b 100644 --- a/arduino-core/src/cc/arduino/contributions/packages/ContributionInstaller.java +++ b/arduino-core/src/cc/arduino/contributions/packages/ContributionInstaller.java @@ -41,6 +41,8 @@ import org.apache.commons.exec.CommandLine; import org.apache.commons.exec.DefaultExecutor; import org.apache.commons.exec.Executor; import org.apache.commons.exec.PumpStreamHandler; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import processing.app.BaseNoGui; import processing.app.I18n; import processing.app.Platform; @@ -51,7 +53,6 @@ import processing.app.helpers.filefilters.OnlyDirs; import java.io.ByteArrayOutputStream; import java.io.File; import java.io.IOException; -import java.net.URL; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; @@ -62,6 +63,7 @@ import static processing.app.I18n.format; import static processing.app.I18n.tr; public class ContributionInstaller { + private static Logger log = LoggerFactory.getLogger(ContributionInstaller.class); private final Platform platform; private final SignatureVerifier signatureVerifier; @@ -122,10 +124,10 @@ public class ContributionInstaller { // all the temporary folders and abort installation. List> resolvedToolReferences = contributedPlatform - .getResolvedToolReferences().entrySet().stream() - .filter((entry) -> !entry.getValue().isInstalled() - || entry.getValue().isBuiltIn()) - .collect(Collectors.toList()); + .getResolvedToolReferences().entrySet().stream() + .filter((entry) -> !entry.getValue().isInstalled() + || entry.getValue().isBuiltIn()) + .collect(Collectors.toList()); int i = 1; for (Map.Entry entry : resolvedToolReferences) { @@ -265,9 +267,10 @@ public class ContributionInstaller { // now try to remove the containing TOOL_NAME folder // (and silently fail if another version of the tool is installed) try { - Files.delete(destFolder.getParentFile().toPath()); + Files.deleteIfExists(destFolder.getParentFile().toPath()); } catch (Exception e) { // ignore + log.error(e.getMessage(), e); } } @@ -282,7 +285,8 @@ public class ContributionInstaller { MultiStepProgress progress = new MultiStepProgress(1); List downloadedPackageIndexFilesAccumulator = new LinkedList<>(); - downloadIndexAndSignature(progress, downloadedPackageIndexFilesAccumulator, Constants.PACKAGE_INDEX_URL, progressListener); + final DownloadableContributionsDownloader downloader = new DownloadableContributionsDownloader(BaseNoGui.indexer.getStagingFolder()); + downloader.downloadIndexAndSignature(progress, downloadedPackageIndexFilesAccumulator, Constants.PACKAGE_INDEX_URL, progressListener, signatureVerifier); Set packageIndexURLs = new HashSet<>(); String additionalURLs = PreferencesData.get(Constants.PREF_BOARDS_MANAGER_ADDITIONAL_URLS, ""); @@ -292,8 +296,9 @@ public class ContributionInstaller { for (String packageIndexURL : packageIndexURLs) { try { - downloadIndexAndSignature(progress, downloadedPackageIndexFilesAccumulator, packageIndexURL, progressListener); + downloader.downloadIndexAndSignature(progress, downloadedPackageIndexFilesAccumulator, packageIndexURL, progressListener, signatureVerifier); } catch (Exception e) { + log.error(e.getMessage(), e); System.err.println(e.getMessage()); } } @@ -303,41 +308,6 @@ public class ContributionInstaller { return downloadedPackageIndexFilesAccumulator; } - private void downloadIndexAndSignature(MultiStepProgress progress, List downloadedPackagedIndexFilesAccumulator, String packageIndexUrl, ProgressListener progressListener) throws Exception { - File packageIndex = download(progress, packageIndexUrl, progressListener); - downloadedPackagedIndexFilesAccumulator.add(packageIndex.getName()); - try { - File packageIndexSignature = download(progress, packageIndexUrl + ".sig", progressListener); - boolean signatureVerified = signatureVerifier.isSigned(packageIndex); - if (signatureVerified) { - downloadedPackagedIndexFilesAccumulator.add(packageIndexSignature.getName()); - } else { - downloadedPackagedIndexFilesAccumulator.remove(packageIndex.getName()); - Files.delete(packageIndex.toPath()); - Files.delete(packageIndexSignature.toPath()); - System.err.println(I18n.format(tr("{0} file signature verification failed. File ignored."), packageIndexUrl)); - } - } catch (Exception e) { - //ignore errors - } - } - - private File download(MultiStepProgress progress, String packageIndexUrl, ProgressListener progressListener) throws Exception { - String statusText = tr("Downloading platforms index..."); - URL url = new URL(packageIndexUrl); - String[] urlPathParts = url.getFile().split("/"); - File outputFile = BaseNoGui.indexer.getIndexFile(urlPathParts[urlPathParts.length - 1]); - File tmpFile = new File(outputFile.getAbsolutePath() + ".tmp"); - DownloadableContributionsDownloader downloader = new DownloadableContributionsDownloader(BaseNoGui.indexer.getStagingFolder()); - boolean noResume = true; - downloader.download(url, tmpFile, progress, statusText, progressListener, noResume); - - Files.deleteIfExists(outputFile.toPath()); - Files.move(tmpFile.toPath(), outputFile.toPath()); - - return outputFile; - } - public synchronized void deleteUnknownFiles(List downloadedPackageIndexFiles) throws IOException { File preferencesFolder = BaseNoGui.indexer.getIndexFile(".").getParentFile(); File[] additionalPackageIndexFiles = preferencesFolder.listFiles(new PackageIndexFilenameFilter(Constants.DEFAULT_INDEX_FILE_NAME)); diff --git a/arduino-core/src/cc/arduino/utils/network/FileDownloader.java b/arduino-core/src/cc/arduino/utils/network/FileDownloader.java index 27f677bfd..aefb2788f 100644 --- a/arduino-core/src/cc/arduino/utils/network/FileDownloader.java +++ b/arduino-core/src/cc/arduino/utils/network/FileDownloader.java @@ -30,7 +30,6 @@ package cc.arduino.utils.network; import org.apache.commons.compress.utils.IOUtils; - import org.slf4j.Logger; import org.slf4j.LoggerFactory; import processing.app.BaseNoGui; @@ -40,12 +39,13 @@ import java.io.File; import java.io.IOException; import java.io.InputStream; import java.io.RandomAccessFile; -import java.net.*; +import java.net.HttpURLConnection; +import java.net.SocketTimeoutException; +import java.net.URL; import java.nio.file.Files; import java.nio.file.Paths; import java.util.Observable; import java.util.Optional; -import java.util.logging.Level; public class FileDownloader extends Observable { private static Logger log = LoggerFactory.getLogger(FileDownloader.class); @@ -186,6 +186,7 @@ public class FileDownloader extends Observable { final int resp = connection.getResponseCode(); if (resp < 200 || resp >= 300) { + Files.delete(outputFile.toPath()); throw new IOException("Received invalid http status code from server: " + resp); } diff --git a/arduino-core/src/processing/app/Platform.java b/arduino-core/src/processing/app/Platform.java index a60c1cc59..14e60edfa 100644 --- a/arduino-core/src/processing/app/Platform.java +++ b/arduino-core/src/processing/app/Platform.java @@ -26,8 +26,6 @@ import cc.arduino.packages.BoardPort; import cc.arduino.utils.network.HttpConnectionManager; import com.fasterxml.jackson.databind.DeserializationFeature; import com.fasterxml.jackson.databind.ObjectMapper; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import processing.app.debug.TargetBoard; import processing.app.debug.TargetPackage; import processing.app.debug.TargetPlatform; @@ -60,7 +58,6 @@ import static processing.app.I18n.tr; * know if name is proper Java package syntax.) */ public class Platform { - private static Logger log = LoggerFactory.getLogger(Platform.class); /** * Set the default L & F. While I enjoy the bounty of the sixteen possible