[LAUNCHER] Fix nested jar url handling - #26039
Conversation
Croway
left a comment
There was a problem hiding this comment.
Thanks for tracking this down, the analysis of the jar:nested: scheme is correct and the fix works for the loader we ship. I checked the built camel-launcher jar: the manifest carries Main-Class: org.springframework.boot.loader.launch.JarLauncher and Spring-Boot-Version: 4.1.1, and the NestedLocation parser in that loader still uses /! as the outer/inner separator. So this is not a Spring Boot 3.x specific format, it is the 3.2+ loader that Spring Boot 4 also uses. Could you reword the comment to "Spring Boot 3.2+ / 4.x loader" so nobody reads it as legacy handling?
A few things I'd like to see before merging:
1. The same bug is still in the launcher's main class
CamelLauncher.detectJarPath() in dsl/camel-jbang/camel-launcher is a verbatim copy of the old jar:file: / file: parsing. Under the current loader it returns null, so the camel.launcher.jar system property is never set and every caller ends up in the LauncherHelper fallback this PR patches. Since camel-launcher already depends on camel-jbang-core, the simplest fix is to drop detectJarPath() and call LauncherHelper.getLauncherJarPath() from main, so there is exactly one implementation of this parsing.
2. Add a unit test for the parser
The parsing is buried in a method that reads its own class's code source, so it cannot be tested directly. Extracting a package-private parseJarPath(String url) and adding a test in camel-jbang-core covering the jar:nested:, jar:file: and file: forms gives a regression check for the loader URL scheme, which is exactly what broke silently when the loader moved from jar:file: to jar:nested:.
3. Minor, optional
URLDecoder.decodealso turns+into a space, so a directory such ascamel+toolsbreaks. Spring's ownNestedLocationdecodes only percent escapes. Pre-existing in the other branches, but the new branch copies it.- Spring's parser strips the leading slash on Windows (
/C:/x.jar→C:/x.jar); ours does not. Also pre-existing. - A one-line comment explaining why
indexOf("/!")(first match) is used rather thanlastIndexOflike Spring does would help: on thejar:-wrapped URL aBOOT-INF/classes/!/suffix would confuselastIndexOf.
Claude Code on behalf of Croway
|
Follow-up after a deeper pass with a probe run inside the built Point 1 is a functional bug, not just duplication. Inside the fat jar the main class's code source is Related: the description's diagnosis is slightly off. The Windows path is a regression, not a nit. The new branch returns Path.of(URI.create("file:" + path.substring(0, idx))).toString()and the same should be applied to the JVM options are dropped on spawn. Now that the Silent fallback masks failures. When Minor: A table test on an extracted Claude Code on behalf of Croway |
|
Thanks for the comments and the analysis, Fede. So the plan is:
+Fixed the PR description |
yes, sounds reasonable |
9ae2693 to
813857d
Compare
|
Addressed the points in the following way:
|
davsclaus
left a comment
There was a problem hiding this comment.
Thanks for tackling this, @jvrubel — the core fix is correct and genuinely valuable. The jar:nested: fallback bug is real, the URI-based decoding is a solid improvement over the manual URLDecoder+substring approach, the filename-based isRunningFromLauncher() check is more precise, and de-duplicating the parsing into a shared LauncherHelper.parseJarPath() (dropping the copy in CamelLauncher) is a nice cleanup.
A few things need addressing before this can go in.
🔴 Blocking
1. The three jar:nested: unit tests use malformed URLs and will fail. parseJarPath() correctly searches for /! — the real Spring Boot boundary between the outer jar and the nested entry (jar:nested:/path/myjar.jar/!BOOT-INF/lib/mylib.jar!/, per Spring's docs, and matching your own PR description). But the test inputs use !/ instead of /! after the outer jar, so indexOf("/!") returns -1, parseJarPath returns null, and the assertions fail. See the inline suggestions on the three lines. (CI hasn't run on this PR yet, so this wasn't caught automatically.)
2. Unrelated generated file core/camel-core-model/src/generated/resources/META-INF/services/org/apache/camel/model.properties reintroduces removed model entries. It re-adds csimple, serviceCall, *ServiceDiscovery, *ServiceFilter, defaultLoadBalancer, etc. — none of which are on current main. csimple was removed on 2026-08-28 (9e8d8e4fe979 Removal of csimple language). This is a stale-branch regeneration artifact: the branch predates that removal. Please rebase on current main and drop this file from the changeset so the PR touches only the launcher files.
🟡 Concern
3. getCamelCommand() now forwards all JVM input arguments via ManagementFactory.getRuntimeMXBean().getInputArguments(). This is beyond the stated nested-URL fix, is untested, and is risky — it blindly forwards -agentlib:jdwp=… (debug port), -javaagent, and -Dcom.sun.management.jmxremote.port=… to the child, which can cause port conflicts. Consider splitting this into its own PR, or filtering to just the -D props you intend to propagate.
🟢 Minor
- The commit message
[LAUNCHER] Fix nested jar url handlingdoesn't follow the project'sCAMEL-XXXX: <description>convention and references no JIRA ticket. If none exists, one should be created.
This is a rules-and-conventions review and does not replace CodeRabbit/Sourcery or SonarCloud.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
|
Two more things on top of @davsclaus' review, both in 1. 2. The Javadoc on Minor: Claude Code on behalf of Croway |
813857d to
a3ed554
Compare
|
Thank you for the reviews @Croway and @davsclaus. I have modified the PR to address your points, validated the tests (bar Windows) and cleaned up the config files. |
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
✅ Generated files are up to dateAn earlier CI run reported uncommitted generated changes; the latest run no longer does. |
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 8 tested, 7 compile-only — current: 6 all testedMaveniverse Scalpel detected 15 affected modules (current approach: 6).
|
Fix LauncherHelper to handle Spring Boot 3.2+/4.x jar:nested: URL scheme
Problem
Camel-launcher is packaged as a Spring Boot executable fat JAR (Main-Class: org.springframework.boot.loader.launch.JarLauncher). Spring
Boot 3.2+/4.x uses a jar:nested: URL scheme for code source locations of classes loaded from nested library JARs:
jar:nested:/path/to/camel-launcher-4.22.0.jar/!BOOT-INF/lib/camel-jbang-core-4.22.0.jar!/LauncherHelper.getLauncherJarPath()only handled jar:file: (Spring Boot 2.x / Maven Shade) and file: URL schemes. When it received a jar:nested: URL, it returned null.This caused
isRunningFromLauncher()to return false, sogetCamelCommand()fell back to ["camel"] — a command that does not exist when running from the fat JAR. Any operationthat spawned a background process (e.g. camel run --background) failed with:
java.io.IOException: Cannot run program "camel": error=2, No such file or directoryFix
Add a jar:nested: case to
getLauncherJarPath(), before the existing jar:file: handler. The outer JAR path is extracted by splitting on /! (the separator between the outer JARand the nested path):
if (urlStr.startsWith("jar:nested:")) { String path = urlStr.substring("jar:nested:".length()); int idx = path.indexOf("/!"); if (idx > 0) { return URLDecoder.decode(path.substring(0, idx), StandardCharsets.UTF_8); } }For the URL above, this correctly extracts /path/to/camel-launcher-4.220.jar, causing
getCamelCommand()to return ["java", "-jar","/path/to/camel-launcher-4.22.0.jar"].
Magic number offsets in the existing jar:file: and file: branches were also replaced with named-length constants for readability.
Verification
Replicated the failure locally by running
java -jar camel-launcher-4.22.0.jar run route.java --backgroundwith camel removed from PATH. Confirmed the identicalCannot run program "camel" error. Applied the fix and verified
getLauncherJarPath()returns the correct outer JAR path from the jar:nested: URL.