Skip to content

Commit 980df58

Browse files
author
Mattia Bertorello
committed
Fix possible empty files during the download of the package index
1 parent 8ca093b commit 980df58

File tree

8 files changed

+117
-74
lines changed

8 files changed

+117
-74
lines changed

Diff for: app/src/processing/app/Base.java

+10-10
Original file line numberDiff line numberDiff line change
@@ -26,30 +26,29 @@
2626
import cc.arduino.Constants;
2727
import cc.arduino.UpdatableBoardsLibsFakeURLsHandler;
2828
import cc.arduino.UploaderUtils;
29-
import cc.arduino.packages.Uploader;
3029
import cc.arduino.contributions.*;
31-
import cc.arduino.contributions.libraries.*;
30+
import cc.arduino.contributions.libraries.ContributedLibrary;
31+
import cc.arduino.contributions.libraries.LibrariesIndexer;
32+
import cc.arduino.contributions.libraries.LibraryInstaller;
33+
import cc.arduino.contributions.libraries.LibraryOfSameTypeComparator;
3234
import cc.arduino.contributions.libraries.ui.LibraryManagerUI;
3335
import cc.arduino.contributions.packages.ContributedPlatform;
3436
import cc.arduino.contributions.packages.ContributionInstaller;
3537
import cc.arduino.contributions.packages.ContributionsIndexer;
3638
import cc.arduino.contributions.packages.ui.ContributionManagerUI;
3739
import cc.arduino.files.DeleteFilesOnShutdown;
3840
import cc.arduino.packages.DiscoveryManager;
41+
import cc.arduino.packages.Uploader;
3942
import cc.arduino.view.Event;
4043
import cc.arduino.view.JMenuUtils;
4144
import cc.arduino.view.SplashScreenHelper;
42-
45+
import com.github.zafarkhaja.semver.Version;
4346
import org.apache.commons.compress.utils.IOUtils;
4447
import org.apache.commons.lang3.StringUtils;
45-
46-
import com.github.zafarkhaja.semver.Version;
47-
4848
import processing.app.debug.TargetBoard;
4949
import processing.app.debug.TargetPackage;
5050
import processing.app.debug.TargetPlatform;
5151
import processing.app.helpers.*;
52-
import processing.app.helpers.OSUtils;
5352
import processing.app.helpers.filefilters.OnlyDirs;
5453
import processing.app.helpers.filefilters.OnlyFilesWithExtension;
5554
import processing.app.javax.swing.filechooser.FileNameExtensionFilter;
@@ -67,9 +66,9 @@
6766
import java.awt.*;
6867
import java.awt.event.*;
6968
import java.io.*;
70-
import java.util.*;
7169
import java.util.List;
7270
import java.util.Timer;
71+
import java.util.*;
7372
import java.util.logging.Handler;
7473
import java.util.logging.Level;
7574
import java.util.logging.Logger;
@@ -286,7 +285,8 @@ public Base(String[] args) throws Exception {
286285
pdeKeywords = new PdeKeywords();
287286
pdeKeywords.reload();
288287

289-
contributionInstaller = new ContributionInstaller(BaseNoGui.getPlatform(), new GPGDetachedSignatureVerifier());
288+
final GPGDetachedSignatureVerifier gpgDetachedSignatureVerifier = new GPGDetachedSignatureVerifier();
289+
contributionInstaller = new ContributionInstaller(BaseNoGui.getPlatform(), gpgDetachedSignatureVerifier);
290290
libraryInstaller = new LibraryInstaller(BaseNoGui.getPlatform());
291291

292292
parser.parseArgumentsPhase2();
@@ -301,7 +301,7 @@ public Base(String[] args) throws Exception {
301301
if (parser.isInstallBoard()) {
302302
ContributionsIndexer indexer = new ContributionsIndexer(
303303
BaseNoGui.getSettingsFolder(), BaseNoGui.getHardwareFolder(),
304-
BaseNoGui.getPlatform(), new GPGDetachedSignatureVerifier());
304+
BaseNoGui.getPlatform(), gpgDetachedSignatureVerifier);
305305
ProgressListener progressListener = new ConsoleProgressListener();
306306

307307
List<String> downloadedPackageIndexFiles = contributionInstaller.updateIndex(progressListener);

Diff for: arduino-core/src/cc/arduino/contributions/DownloadableContributionsDownloader.java

+63-4
Original file line numberDiff line numberDiff line change
@@ -30,20 +30,24 @@
3030
package cc.arduino.contributions;
3131

3232
import cc.arduino.utils.FileHash;
33+
import cc.arduino.utils.MultiStepProgress;
3334
import cc.arduino.utils.Progress;
3435
import cc.arduino.utils.network.FileDownloader;
36+
import org.slf4j.Logger;
37+
import org.slf4j.LoggerFactory;
38+
import processing.app.BaseNoGui;
39+
import processing.app.I18n;
3540

3641
import java.io.File;
3742
import java.net.URL;
38-
import java.nio.file.Files;
39-
import java.nio.file.LinkOption;
40-
import java.nio.file.Path;
41-
import java.nio.file.Paths;
43+
import java.nio.file.*;
44+
import java.util.List;
4245

4346
import static processing.app.I18n.format;
4447
import static processing.app.I18n.tr;
4548

4649
public class DownloadableContributionsDownloader {
50+
private static Logger log = LoggerFactory.getLogger(DownloadableContributionsDownloader.class);
4751

4852
private final File stagingFolder;
4953

@@ -140,4 +144,59 @@ public void download(URL url, File tmpFile, Progress progress, String statusText
140144
}
141145
}
142146

147+
public void downloadIndexAndSignature(MultiStepProgress progress, List<String> downloadedFilesAccumulator, String packageIndexUrlString, ProgressListener progressListener, SignatureVerifier signatureVerifier) throws Exception {
148+
149+
// Extract the file name from the url
150+
URL packageIndexUrl = new URL(packageIndexUrlString);
151+
URL packageIndexSignatureUrl = new URL(packageIndexUrlString + ".sig");
152+
String[] urlPathParts = packageIndexUrl.getFile().split("/");
153+
File packageIndex = BaseNoGui.indexer.getIndexFile(urlPathParts[urlPathParts.length - 1]);
154+
// Signature file name
155+
File packageIndexSignature = BaseNoGui.indexer.getIndexFile(urlPathParts[urlPathParts.length - 1] + ".sig");
156+
157+
final String statusText = tr("Downloading platforms index...");
158+
downloadedFilesAccumulator.add(packageIndex.getName());
159+
160+
// Create temp files
161+
File packageIndexTemp = File.createTempFile(packageIndexUrl.getPath(), ".tmp");
162+
File packageIndexSignatureTemp = File.createTempFile(packageIndexSignatureUrl.getPath(), ".tmp");
163+
try {
164+
// Download package index
165+
download(packageIndexUrl, packageIndexTemp, progress, statusText, progressListener, true);
166+
try {
167+
// Download signature
168+
download(packageIndexSignatureUrl, packageIndexSignatureTemp, progress, statusText, progressListener, true);
169+
170+
// Verify the signature before move the files
171+
boolean signatureVerified = signatureVerifier.isSigned(packageIndexTemp, packageIndexSignatureTemp);
172+
if (signatureVerified) {
173+
// Move if the signature is ok
174+
Files.move(packageIndexTemp.toPath(), packageIndex.toPath(), StandardCopyOption.REPLACE_EXISTING);
175+
Files.move(packageIndexSignatureTemp.toPath(), packageIndexSignature.toPath(), StandardCopyOption.REPLACE_EXISTING);
176+
downloadedFilesAccumulator.add(packageIndexSignature.getName());
177+
} else {
178+
downloadedFilesAccumulator.remove(packageIndex.getName());
179+
log.error("{} file signature verification failed. File ignored.", packageIndexSignatureUrl);
180+
System.err.println(format(tr("{0} file signature verification failed. File ignored."), packageIndexUrlString));
181+
182+
}
183+
} catch (Exception e) {
184+
log.error("Cannot download the signature from {} the package will be install in any case", packageIndexSignatureUrl, e);
185+
if (packageIndexTemp.length() > 0) {
186+
Files.move(packageIndexTemp.toPath(), packageIndex.toPath(), StandardCopyOption.REPLACE_EXISTING);
187+
} else {
188+
log.error("The temporarily package index file is empty (path:{},url:{}), It cannot be move there {} ",
189+
packageIndexTemp.toPath(), packageIndexUrlString, packageIndex.toPath());
190+
}
191+
}
192+
193+
} catch (Exception e) {
194+
downloadedFilesAccumulator.remove(packageIndex.getName());
195+
throw e;
196+
} finally {
197+
// Delete useless temp file
198+
Files.deleteIfExists(packageIndexTemp.toPath());
199+
Files.deleteIfExists(packageIndexSignatureTemp.toPath());
200+
}
201+
}
143202
}

Diff for: arduino-core/src/cc/arduino/contributions/GZippedJsonDownloader.java

+10-5
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,15 @@
2929

3030
package cc.arduino.contributions;
3131

32+
import cc.arduino.Constants;
3233
import cc.arduino.utils.Progress;
3334
import org.apache.commons.compress.compressors.gzip.GzipCompressorInputStream;
3435
import org.apache.commons.compress.compressors.gzip.GzipUtils;
3536
import org.apache.commons.compress.utils.IOUtils;
3637

3738
import java.io.*;
3839
import java.net.URL;
40+
import java.nio.file.Files;
3941

4042
public class GZippedJsonDownloader {
4143

@@ -50,17 +52,20 @@ public GZippedJsonDownloader(DownloadableContributionsDownloader downloader, URL
5052
}
5153

5254
public void download(File tmpFile, Progress progress, String statusText, ProgressListener progressListener) throws Exception {
55+
File gzipTmpFile = null;
5356
try {
54-
File gzipTmpFile = new File(tmpFile.getParentFile(), GzipUtils.getCompressedFilename(tmpFile.getName()));
57+
gzipTmpFile = File.createTempFile(new URL(Constants.LIBRARY_INDEX_URL_GZ).getPath(), GzipUtils.getCompressedFilename(tmpFile.getName()));
5558
// remove eventual leftovers from previous downloads
56-
if (gzipTmpFile.exists()) {
57-
gzipTmpFile.delete();
58-
}
59+
Files.deleteIfExists(gzipTmpFile.toPath());
60+
5961
new JsonDownloader(downloader, gzippedUrl).download(gzipTmpFile, progress, statusText, progressListener);
6062
decompress(gzipTmpFile, tmpFile);
61-
gzipTmpFile.delete();
6263
} catch (Exception e) {
6364
new JsonDownloader(downloader, url).download(tmpFile, progress, statusText, progressListener);
65+
} finally {
66+
if (gzipTmpFile != null) {
67+
Files.deleteIfExists(gzipTmpFile.toPath());
68+
}
6469
}
6570
}
6671

Diff for: arduino-core/src/cc/arduino/contributions/SignatureVerifier.java

+9
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,15 @@ public boolean isSigned(File indexFile) {
5050
}
5151
}
5252

53+
public boolean isSigned(File indexFile, File signature) {
54+
try {
55+
return verify(indexFile, signature, new File(BaseNoGui.getContentFile("lib"), "public.gpg.key"));
56+
} catch (Exception e) {
57+
BaseNoGui.showWarning(e.getMessage(), e.getMessage(), e);
58+
return false;
59+
}
60+
}
61+
5362
protected abstract boolean verify(File signedFile, File signature, File publicKey) throws IOException;
5463

5564
}

Diff for: arduino-core/src/cc/arduino/contributions/libraries/LibraryInstaller.java

+8-6
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,8 @@
4343
import java.io.File;
4444
import java.io.IOException;
4545
import java.net.URL;
46+
import java.nio.file.Files;
47+
import java.nio.file.StandardCopyOption;
4648
import java.util.Optional;
4749

4850
import static processing.app.I18n.tr;
@@ -61,10 +63,11 @@ public synchronized void updateIndex(ProgressListener progressListener) throws E
6163
DownloadableContributionsDownloader downloader = new DownloadableContributionsDownloader(BaseNoGui.librariesIndexer.getStagingFolder());
6264
// Step 1: Download index
6365
File outputFile = BaseNoGui.librariesIndexer.getIndexFile();
64-
File tmpFile = new File(outputFile.getAbsolutePath() + ".tmp");
66+
// Create temp files
67+
File libraryIndexTemp = File.createTempFile(new URL(Constants.LIBRARY_INDEX_URL).getPath(), ".tmp");
6568
try {
6669
GZippedJsonDownloader gZippedJsonDownloader = new GZippedJsonDownloader(downloader, new URL(Constants.LIBRARY_INDEX_URL), new URL(Constants.LIBRARY_INDEX_URL_GZ));
67-
gZippedJsonDownloader.download(tmpFile, progress, tr("Downloading libraries index..."), progressListener);
70+
gZippedJsonDownloader.download(libraryIndexTemp, progress, tr("Downloading libraries index..."), progressListener);
6871
} catch (InterruptedException e) {
6972
// Download interrupted... just exit
7073
return;
@@ -74,10 +77,9 @@ public synchronized void updateIndex(ProgressListener progressListener) throws E
7477
// TODO: Check downloaded index
7578

7679
// Replace old index with the updated one
77-
if (outputFile.exists())
78-
outputFile.delete();
79-
if (!tmpFile.renameTo(outputFile))
80-
throw new Exception(tr("An error occurred while updating libraries index!"));
80+
if (libraryIndexTemp.length() > 0) {
81+
Files.move(libraryIndexTemp.toPath(), outputFile.toPath(), StandardCopyOption.REPLACE_EXISTING);
82+
}
8183

8284
// Step 2: Parse index
8385
BaseNoGui.librariesIndexer.parseIndex();

Diff for: arduino-core/src/cc/arduino/contributions/packages/ContributionInstaller.java

+13-43
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@
4141
import org.apache.commons.exec.DefaultExecutor;
4242
import org.apache.commons.exec.Executor;
4343
import org.apache.commons.exec.PumpStreamHandler;
44+
import org.slf4j.Logger;
45+
import org.slf4j.LoggerFactory;
4446
import processing.app.BaseNoGui;
4547
import processing.app.I18n;
4648
import processing.app.Platform;
@@ -51,7 +53,6 @@
5153
import java.io.ByteArrayOutputStream;
5254
import java.io.File;
5355
import java.io.IOException;
54-
import java.net.URL;
5556
import java.nio.file.Files;
5657
import java.nio.file.Path;
5758
import java.nio.file.Paths;
@@ -62,6 +63,7 @@
6263
import static processing.app.I18n.tr;
6364

6465
public class ContributionInstaller {
66+
private static Logger log = LoggerFactory.getLogger(ContributionInstaller.class);
6567

6668
private final Platform platform;
6769
private final SignatureVerifier signatureVerifier;
@@ -122,10 +124,10 @@ public synchronized List<String> install(ContributedPlatform contributedPlatform
122124
// all the temporary folders and abort installation.
123125

124126
List<Map.Entry<ContributedToolReference, ContributedTool>> resolvedToolReferences = contributedPlatform
125-
.getResolvedToolReferences().entrySet().stream()
126-
.filter((entry) -> !entry.getValue().isInstalled()
127-
|| entry.getValue().isBuiltIn())
128-
.collect(Collectors.toList());
127+
.getResolvedToolReferences().entrySet().stream()
128+
.filter((entry) -> !entry.getValue().isInstalled()
129+
|| entry.getValue().isBuiltIn())
130+
.collect(Collectors.toList());
129131

130132
int i = 1;
131133
for (Map.Entry<ContributedToolReference, ContributedTool> entry : resolvedToolReferences) {
@@ -265,9 +267,10 @@ public synchronized List<String> remove(ContributedPlatform contributedPlatform)
265267
// now try to remove the containing TOOL_NAME folder
266268
// (and silently fail if another version of the tool is installed)
267269
try {
268-
Files.delete(destFolder.getParentFile().toPath());
270+
Files.deleteIfExists(destFolder.getParentFile().toPath());
269271
} catch (Exception e) {
270272
// ignore
273+
log.error(e.getMessage(), e);
271274
}
272275
}
273276

@@ -282,7 +285,8 @@ public synchronized List<String> updateIndex(ProgressListener progressListener)
282285
MultiStepProgress progress = new MultiStepProgress(1);
283286

284287
List<String> downloadedPackageIndexFilesAccumulator = new LinkedList<>();
285-
downloadIndexAndSignature(progress, downloadedPackageIndexFilesAccumulator, Constants.PACKAGE_INDEX_URL, progressListener);
288+
final DownloadableContributionsDownloader downloader = new DownloadableContributionsDownloader(BaseNoGui.indexer.getStagingFolder());
289+
downloader.downloadIndexAndSignature(progress, downloadedPackageIndexFilesAccumulator, Constants.PACKAGE_INDEX_URL, progressListener, signatureVerifier);
286290

287291
Set<String> packageIndexURLs = new HashSet<>();
288292
String additionalURLs = PreferencesData.get(Constants.PREF_BOARDS_MANAGER_ADDITIONAL_URLS, "");
@@ -292,8 +296,9 @@ public synchronized List<String> updateIndex(ProgressListener progressListener)
292296

293297
for (String packageIndexURL : packageIndexURLs) {
294298
try {
295-
downloadIndexAndSignature(progress, downloadedPackageIndexFilesAccumulator, packageIndexURL, progressListener);
299+
downloader.downloadIndexAndSignature(progress, downloadedPackageIndexFilesAccumulator, packageIndexURL, progressListener, signatureVerifier);
296300
} catch (Exception e) {
301+
log.error(e.getMessage(), e);
297302
System.err.println(e.getMessage());
298303
}
299304
}
@@ -303,41 +308,6 @@ public synchronized List<String> updateIndex(ProgressListener progressListener)
303308
return downloadedPackageIndexFilesAccumulator;
304309
}
305310

306-
private void downloadIndexAndSignature(MultiStepProgress progress, List<String> downloadedPackagedIndexFilesAccumulator, String packageIndexUrl, ProgressListener progressListener) throws Exception {
307-
File packageIndex = download(progress, packageIndexUrl, progressListener);
308-
downloadedPackagedIndexFilesAccumulator.add(packageIndex.getName());
309-
try {
310-
File packageIndexSignature = download(progress, packageIndexUrl + ".sig", progressListener);
311-
boolean signatureVerified = signatureVerifier.isSigned(packageIndex);
312-
if (signatureVerified) {
313-
downloadedPackagedIndexFilesAccumulator.add(packageIndexSignature.getName());
314-
} else {
315-
downloadedPackagedIndexFilesAccumulator.remove(packageIndex.getName());
316-
Files.delete(packageIndex.toPath());
317-
Files.delete(packageIndexSignature.toPath());
318-
System.err.println(I18n.format(tr("{0} file signature verification failed. File ignored."), packageIndexUrl));
319-
}
320-
} catch (Exception e) {
321-
//ignore errors
322-
}
323-
}
324-
325-
private File download(MultiStepProgress progress, String packageIndexUrl, ProgressListener progressListener) throws Exception {
326-
String statusText = tr("Downloading platforms index...");
327-
URL url = new URL(packageIndexUrl);
328-
String[] urlPathParts = url.getFile().split("/");
329-
File outputFile = BaseNoGui.indexer.getIndexFile(urlPathParts[urlPathParts.length - 1]);
330-
File tmpFile = new File(outputFile.getAbsolutePath() + ".tmp");
331-
DownloadableContributionsDownloader downloader = new DownloadableContributionsDownloader(BaseNoGui.indexer.getStagingFolder());
332-
boolean noResume = true;
333-
downloader.download(url, tmpFile, progress, statusText, progressListener, noResume);
334-
335-
Files.deleteIfExists(outputFile.toPath());
336-
Files.move(tmpFile.toPath(), outputFile.toPath());
337-
338-
return outputFile;
339-
}
340-
341311
public synchronized void deleteUnknownFiles(List<String> downloadedPackageIndexFiles) throws IOException {
342312
File preferencesFolder = BaseNoGui.indexer.getIndexFile(".").getParentFile();
343313
File[] additionalPackageIndexFiles = preferencesFolder.listFiles(new PackageIndexFilenameFilter(Constants.DEFAULT_INDEX_FILE_NAME));

Diff for: arduino-core/src/cc/arduino/utils/network/FileDownloader.java

+4-3
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
package cc.arduino.utils.network;
3131

3232
import org.apache.commons.compress.utils.IOUtils;
33-
3433
import org.slf4j.Logger;
3534
import org.slf4j.LoggerFactory;
3635
import processing.app.BaseNoGui;
@@ -40,12 +39,13 @@
4039
import java.io.IOException;
4140
import java.io.InputStream;
4241
import java.io.RandomAccessFile;
43-
import java.net.*;
42+
import java.net.HttpURLConnection;
43+
import java.net.SocketTimeoutException;
44+
import java.net.URL;
4445
import java.nio.file.Files;
4546
import java.nio.file.Paths;
4647
import java.util.Observable;
4748
import java.util.Optional;
48-
import java.util.logging.Level;
4949

5050
public class FileDownloader extends Observable {
5151
private static Logger log = LoggerFactory.getLogger(FileDownloader.class);
@@ -186,6 +186,7 @@ private void downloadFile(boolean noResume) throws InterruptedException {
186186
final int resp = connection.getResponseCode();
187187

188188
if (resp < 200 || resp >= 300) {
189+
Files.delete(outputFile.toPath());
189190
throw new IOException("Received invalid http status code from server: " + resp);
190191
}
191192

0 commit comments

Comments
 (0)