matthiasblaesing commented on a change in pull request #2317:
URL: https://github.com/apache/netbeans/pull/2317#discussion_r481321250
##########
File path:
platform/autoupdate.services/src/org/netbeans/api/autoupdate/OperationContainer.java
##########
@@ -352,6 +353,17 @@ public void removeAll() {
impl.removeAll ();
}
+ /** Specifies location of unpack200 executable. {@code unpack200} has been
+ * removed from JDK 14. As such it is not possible to unpack older NBM
+ * files without providing alternative JDK implementation of this file.
+ *
+ * @param executable path to the executable
+ * @since 1.65
+ */
+ public final void setUnpack200(File executable) {
+ this.impl.setUnpack200(executable);
+ }
Review comment:
Maybe at some point in the future (no I don't want to wait for that
moment), we might be able to ship a pack200 implementation (if I read the
discussion on apache-legal correctly pure GPLv2-CPE might ok, the problem was,
that the JDK is a mix of GPLv2 and GPLv2-CPE). If we can ship an
implementation, would it make sense to do the decompression in process? For
tar, zip and maybe other compression formats I would not consider to call the
CLI version.
##########
File path:
platform/autoupdate.services/src/org/netbeans/modules/autoupdate/services/OperationContainerImpl.java
##########
@@ -530,4 +532,33 @@ public OperationType getType () {
}
private OperationType type;
private OperationContainer delegate;
+
+ /**
+ * @return the unpack200 executable or {@code null}
+ */
+ public final File getUnpack200() {
+ NO_PACK: if (unpack200 == null) {
+ final String jreHome = System.getProperty("java.home"); // NOI18N
+ if (jreHome == null) {
+ break NO_PACK;
+ }
Review comment:
This looks strange to me. I have never seen a break without a loop (is
this java style `goto`)? Can `java.home` even be unset? I admit I don't know
what jlink and/or graal static image returns here.
##########
File path:
platform/autoupdate.services/src/org/netbeans/modules/autoupdate/services/InstallSupportImpl.java
##########
@@ -1121,7 +1121,22 @@ private int verifyNbm (UpdateElement el, File nbmFile,
ProgressHandle progress,
if(res == null) {
try {
- Collection<CodeSigner> nbmCerts =
Utilities.getNbmCertificates(nbmFile);
+ List<String> pack200Entries = new ArrayList<>();
+ Collection<CodeSigner> nbmCerts =
Utilities.getNbmCertificates(nbmFile, pack200Entries);
+ if (!pack200Entries.isEmpty()) {
+ OperationContainer<InstallSupport> operationContainer
= support.getContainer();
+ OperationContainerImpl ocImpl =
Trampoline.API.impl(operationContainer);
+ File unpack200 = ocImpl.getUnpack200();
+ if (unpack200 == null || !unpack200.canExecute()) {
+ StringBuilder sb = new StringBuilder();
+ for (String entry : pack200Entries) {
+ sb.append("\n").append(entry);
+ }
+ throw new
OperationException(OperationException.ERROR_TYPE.MISSING_UNPACK200,
+ NbBundle.getMessage(InstallSupportImpl.class,
"InstallSupportImpl_Validate_MissingUnpack200", nbmFile, sb.toString()) //
NOI18N
+ );
+ }
+ }
Review comment:
This code path will not be hit when the download happens from a trusted
source (See Plugin Configuration -> Settings -> Update Center Customizer ->
"Trust update center fully and allow automatic installations").
The determination whether pack200 is required could be done seperatedly from
the codesigner extraction. We are not on a hotpath, so scanning the ZIP/JAR/NBM
twice should be ok IMHO.
##########
File path:
platform/autoupdate.services/libsrc/org/netbeans/updater/ModuleUpdater.java
##########
@@ -579,6 +579,34 @@ private boolean unpack200(File src, File dest) {
return result == 0;
}
+ private File findUnpack200Executable(String unpack200) {
+ File unpack200Executable = new File(new
File(System.getProperty("java.home"), "bin"), unpack200);
+ if (!unpack200Executable.canExecute()) {
+ for (File clusterRoot : UpdateTracking.clusters(true)) {
+ File uiConfig = new File(new File(new File(new File(new
File(new File(new File(
+ clusterRoot, "config"), "Preferences"), "org"),
"netbeans"), "modules"), // NOI18N
+ "autoupdate"), "services.properties"); // NOI18N
Review comment:
What is the meaning of this path?
For me this would be more readable:
```java
File uiConfig = new File(clusterRoot,
"config/Preferences/org/netbeans/modules/autoupdate/services.properties");
```
##########
File path:
platform/autoupdate.services/src/org/netbeans/modules/autoupdate/services/Utilities.java
##########
@@ -289,13 +289,15 @@ private static int mapVerificationResultToInt(String
input) {
* Get the certpaths that were used to sign the NBM content.
*
* @param nbmFile
+ * @param pack200Entries list of strings to add any entries in the NBM file
+ * that end with {@code .pack.gz} extension and may require pack200 tool
* @return collection of CodeSigners, that were used to sign the
non-signature
* entries of the NBM
* @throws IOException
* @throws SecurityException if JAR was tampered with or if the certificate
* chains are not consistent
*/
- public static Collection<CodeSigner> getNbmCertificates (File nbmFile)
throws IOException, SecurityException {
+ public static Collection<CodeSigner> getNbmCertificates (File nbmFile,
List<String> pack200Entries) throws IOException, SecurityException {
Review comment:
I wrote it above. I'm not sure whether this mix of return (return via
return statement and return via method parameter) is worth the saved file
scanning.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
