gnodet-bot commented on code in PR #29805:
URL: https://github.com/apache/camel/pull/29805#discussion_r4568935092
##########
dsl/camel-jbang/camel-launcher/src/main/resources/bin/camel.bat:
##########
@@ -83,7 +83,18 @@ if "!_CDS_REST!"=="." set "_CDS_CMD=!_CDS_ARG!"
:cdsCommandDone
for %%j in ("!LAUNCHER_JAR!") do set "_CDS_VERSION=%%~nj"
set "_CDS_VERSION=!_CDS_VERSION:camel-launcher-=!"
-set "_CDS_DIR=!LOCALAPPDATA!\camel-cli\cds\!_CDS_VERSION!"
+@REM Include a hash of the install directory so two installs of the same
version get separate archives.
+@REM Different install methods (SDKMAN, zip) place the launcher in different
directories;
+@REM a shared archive from another install points at the wrong JAR or returns
wrong install metadata.
+@REM Pass BASEDIR via environment to avoid trailing-backslash breaking
PowerShell string quoting.
+set "_CDS_INSTALL_HASH="
+set "_CDS_PS_INPUT=!BASEDIR!"
+set "_CDS_PS_EXE=%SystemRoot%\System32\WindowsPowerShell\v1.0\powershell.exe"
+if exist "!_CDS_PS_EXE!" (
+ for /f "delims=" %%h in ('"!_CDS_PS_EXE!" -NoProfile -NonInteractive
-Command
"$b=[System.Text.Encoding]::UTF8.GetBytes($env:_CDS_PS_INPUT);$h=[System.Security.Cryptography.SHA256]::Create().ComputeHash($b);($h[0..3]|ForEach-Object{$_.ToString(\"x2\")})
-join \"\"" 2^>nul') do set "_CDS_INSTALL_HASH=%%h"
Review Comment:
🔴 **Regression (c5bf4635):** Quoting `!_CDS_PS_EXE!` makes the `for /f`
command string start with `"`. Cmd.exe then strips the **first** and **last**
`"` from the string — the last one is the closing quote of `-Command "..."`,
leaving PowerShell with an unterminated command string. `2^>nul` hides the
error; the hash silently falls back to `default`, breaking
`CamelBatDiscoveryTest`.
`%SystemRoot%` is always `C:\Windows` (no spaces, guaranteed by the OS
installer). Remove the quotes:
```suggestion
for /f "delims=" %%h in ('!_CDS_PS_EXE! -NoProfile -NonInteractive
-Command
"$b=[System.Text.Encoding]::UTF8.GetBytes($env:_CDS_PS_INPUT);$h=[System.Security.Cryptography.SHA256]::Create().ComputeHash($b);($h[0..3]|ForEach-Object{$_.ToString(\"x2\")})
-join \"\"" 2^>nul') do set "_CDS_INSTALL_HASH=%%h"
```
##########
dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/common/PluginHelper.java:
##########
@@ -813,33 +852,81 @@ public static JsonObject createPluginConfig(Path
configPath) {
if (parent != null) {
Files.createDirectories(parent);
}
- Files.writeString(f, config.toJson(),
- StandardOpenOption.CREATE,
- StandardOpenOption.WRITE,
- StandardOpenOption.TRUNCATE_EXISTING);
+ atomicWriteJson(f, config.toJson());
} catch (IOException e) {
throw new RuntimeCamelException("Failed to create plugin
configuration", e);
}
return config;
}
+ /**
+ * Writes {@code content} to {@code target} atomically: writes to a
sibling temp file first, then moves it into
+ * place with {@link StandardCopyOption#REPLACE_EXISTING}. When atomic
move is not supported by the underlying
+ * filesystem, falls back to a plain overwriting move (rename(2)), which
is still safe — a reader never observes a
+ * half-written file.
+ */
+ private static void atomicWriteJson(Path target, String content) throws
IOException {
+ // Resolve symlinks so the move replaces the target file, not the
symlink itself.
+ Path realTarget;
+ try {
+ realTarget = target.toRealPath();
+ } catch (IOException e) {
+ // file does not exist yet — no symlink to resolve
+ realTarget = target;
+ }
+ Path dir = realTarget.getParent();
+ if (dir != null) {
+ Files.createDirectories(dir);
+ }
+ Path tmp = dir != null
+ ? Files.createTempFile(dir, "." + realTarget.getFileName(),
".tmp")
+ : Files.createTempFile("." + realTarget.getFileName(), ".tmp");
+ try {
+ Files.writeString(tmp, content, StandardOpenOption.WRITE,
StandardOpenOption.TRUNCATE_EXISTING);
+ try {
+ Files.move(tmp, realTarget,
StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.ATOMIC_MOVE);
+ tmp = null; // moved successfully, nothing to delete
+ } catch (AtomicMoveNotSupportedException e) {
+ // fall back: non-atomic replace — still better than
truncate-then-write
+ Files.move(tmp, realTarget,
StandardCopyOption.REPLACE_EXISTING);
+ tmp = null;
+ }
+ } finally {
+ if (tmp != null) {
+ try {
+ Files.deleteIfExists(tmp);
+ } catch (IOException ignored) {
+ // best effort
+ }
+ }
+ }
+ }
+
public static void savePluginConfig(JsonObject plugins) {
savePluginConfig(plugins, null);
}
public static void savePluginConfig(JsonObject plugins, Path configPath) {
Path f = getCustomPluginConfigPath(configPath);
try {
- Files.writeString(f, plugins.toJson(),
- StandardOpenOption.CREATE,
- StandardOpenOption.WRITE,
- StandardOpenOption.TRUNCATE_EXISTING);
+ atomicWriteJson(f, plugins.toJson());
} catch (IOException e) {
throw new RuntimeCamelException("Failed to save plugin
configuration", e);
}
}
+ /**
+ * Warns about a corrupt or unreadable plugin configuration file, once per
path per JVM run, so multiple callers
+ * (e.g. {@code getActivePlugins} and {@code printAvailablePlugins}) do
not repeat the message.
+ */
+ public static void warnCorruptPluginConfig(Printer printer, Exception e) {
+ String key = e.getMessage() != null ? e.getMessage() :
"unknown-corrupt-plugin-config";
+ if (WARNED_PLUGIN_PATHS.add(key)) {
+ printer.printErr("Warning: " + e.getMessage() + ". Fix the JSON by
hand or delete the file.");
Review Comment:
💡 `e.getMessage()` on line 926 is not guarded against `null`, but line 924
guards it for the dedup key. If a `RuntimeCamelException` is constructed with a
cause but no message, this prints `"Warning: null. Fix the JSON by hand or
delete the file."` Use `key` (which is already null-safe) for the printed
message:
```suggestion
public static void warnCorruptPluginConfig(Printer printer, Exception e)
{
String msg = e.getMessage() != null ? e.getMessage() : "corrupt or
unreadable plugin configuration";
if (WARNED_PLUGIN_PATHS.add(msg)) {
printer.printErr("Warning: " + msg + ". Fix the JSON by hand or
delete the file.");
}
}
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
