You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@guacamole.apache.org by GitBox <gi...@apache.org> on 2022/01/21 00:10:12 UTC
[GitHub] [guacamole-client] mike-jumper opened a new pull request #687: GUACAMOLE-1508: Add support for nesting .jar files within extensions.
mike-jumper opened a new pull request #687:
URL: https://github.com/apache/guacamole-client/pull/687
This change adds support for nesting .jar files within extensions, automatically extracting those files to a temporary directory for inclusion within the classpath. The files and directory are deleted automatically upon webapp shutdown or, if that somehow fails, upon JVM exit.
This should also remove the need to exclude metadata from signed .jars, as such libraries will now be able to successfully access and verify themselves.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-client] necouchman commented on a change in pull request #687: GUACAMOLE-1508: Add support for nesting .jar files within extensions.
Posted by GitBox <gi...@apache.org>.
necouchman commented on a change in pull request #687:
URL: https://github.com/apache/guacamole-client/pull/687#discussion_r790320827
##########
File path: extensions/pom.xml
##########
@@ -64,12 +64,11 @@
<id>copy-runtime-dependencies</id>
<phase>prepare-package</phase>
<goals>
- <goal>unpack-dependencies</goal>
+ <goal>copy-dependencies</goal>
</goals>
<configuration>
<includeScope>runtime</includeScope>
<outputDirectory>${project.build.directory}/classes</outputDirectory>
- <excludes>META-INF/*.SF,META-INF/*.DSA</excludes>
Review comment:
Sorry, one other question - is this going to cause any issues with signed JARs? I seem to recall that BouncyCastle is a signed JAR, and some of these items had to be removed to get extensions that use it (RADIUS, maybe?) to run correctly.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-client] mike-jumper commented on a change in pull request #687: GUACAMOLE-1508: Add support for nesting .jar files within extensions.
Posted by GitBox <gi...@apache.org>.
mike-jumper commented on a change in pull request #687:
URL: https://github.com/apache/guacamole-client/pull/687#discussion_r789318956
##########
File path: guacamole/src/main/java/org/apache/guacamole/extension/Extension.java
##########
@@ -355,12 +356,18 @@
* @param file
* The file to load as an extension.
*
+ * @param temporaryFiles
+ * A modifiable List that should be populated with all temporary files
+ * created for this extension. These files should be deleted on
+ * application shutdown in reverse order.
+ *
* @throws GuacamoleException
* If the provided file is not a .jar file, does not contain the
* guac-manifest.json, or if guac-manifest.json is invalid and cannot
* be parsed.
*/
- public Extension(final ClassLoader parent, final File file) throws GuacamoleException {
+ public Extension(final ClassLoader parent, final File file,
+ final List<File> temporaryFiles) throws GuacamoleException {
Review comment:
> Should there be a backward-compatible convenience method?
Nope! The classes touched here are strictly the internals of the webapp, not part of guacamole-ext or any other library.
> Is this going to break older versions of extensions in whatever version of Guacamole (1.5.0?) gets this functionality?
Nope! The previous functionality was:
* `.class` files are loaded directly from the extension `.jar` file
while the new functionality is:
* `.class` files are loaded directly from the extension `.jar` file
* `.class` files are additionally loaded directly from any `.jar` files within the archive root of the extension `.jar` file
So what we have here is a superset of what we had before, with modifications to only internal components.
The only way that an extension might encounter problems is if it already includes one or more `.jar` files in its root and relies on Guacamole _not_ loading them ... which would be pretty odd IMHO.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-client] mike-jumper commented on a change in pull request #687: GUACAMOLE-1508: Add support for nesting .jar files within extensions.
Posted by GitBox <gi...@apache.org>.
mike-jumper commented on a change in pull request #687:
URL: https://github.com/apache/guacamole-client/pull/687#discussion_r790323847
##########
File path: extensions/pom.xml
##########
@@ -64,12 +64,11 @@
<id>copy-runtime-dependencies</id>
<phase>prepare-package</phase>
<goals>
- <goal>unpack-dependencies</goal>
+ <goal>copy-dependencies</goal>
</goals>
<configuration>
<includeScope>runtime</includeScope>
<outputDirectory>${project.build.directory}/classes</outputDirectory>
- <excludes>META-INF/*.SF,META-INF/*.DSA</excludes>
Review comment:
I believe those issues should no longer occur, since those signed .jar files can now be correctly verified by the JVM.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-client] necouchman commented on a change in pull request #687: GUACAMOLE-1508: Add support for nesting .jar files within extensions.
Posted by GitBox <gi...@apache.org>.
necouchman commented on a change in pull request #687:
URL: https://github.com/apache/guacamole-client/pull/687#discussion_r789283663
##########
File path: guacamole/src/main/java/org/apache/guacamole/extension/Extension.java
##########
@@ -355,12 +356,18 @@
* @param file
* The file to load as an extension.
*
+ * @param temporaryFiles
+ * A modifiable List that should be populated with all temporary files
+ * created for this extension. These files should be deleted on
+ * application shutdown in reverse order.
+ *
* @throws GuacamoleException
* If the provided file is not a .jar file, does not contain the
* guac-manifest.json, or if guac-manifest.json is invalid and cannot
* be parsed.
*/
- public Extension(final ClassLoader parent, final File file) throws GuacamoleException {
+ public Extension(final ClassLoader parent, final File file,
+ final List<File> temporaryFiles) throws GuacamoleException {
Review comment:
Should there be a backward-compatible convenience method? Is this going to break older versions of extensions in whatever version of Guacamole (1.5.0?) gets this functionality?
--
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.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-client] necouchman merged pull request #687: GUACAMOLE-1508: Add support for nesting .jar files within extensions.
Posted by GitBox <gi...@apache.org>.
necouchman merged pull request #687:
URL: https://github.com/apache/guacamole-client/pull/687
--
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.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org