Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -4142,7 +4142,7 @@ private fun unpackExecutableFile(
try {
val origDll = File("${imageFs.wineprefix}/dosdevices/a:/$relDllPath")
if (origDll.exists()) {
val genCmd = "wine z:\\generate_interfaces_file.exe A:\\" + relDllPath.replace('/', '\\')
val genCmd = "wine cmd /c \"z:\\generate_interfaces_file.exe A:\\" + relDllPath.replace('/', '\\') + " & wineserver -k\""

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Quote and escape the DLL path inside the cmd /c payload.

At Line 4145, relDllPath is concatenated unquoted into wine cmd /c "...". If the path contains spaces (or cmd metacharacters), generate_interfaces_file.exe parsing breaks and may execute unintended fragments. Wrap the A:\... argument in quotes and escape embedded quotes.

Suggested patch
-                                val genCmd = "wine cmd /c \"z:\\generate_interfaces_file.exe A:\\" + relDllPath.replace('/', '\\') + " & wineserver -k\""
+                                val dllPathArg = relDllPath
+                                    .replace('/', '\\')
+                                    .replace("\"", "\\\"")
+                                val genCmd =
+                                    "wine cmd /c \"z:\\generate_interfaces_file.exe \\\"A:\\$dllPathArg\\\" & wineserver -k\""
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
val genCmd = "wine cmd /c \"z:\\generate_interfaces_file.exe A:\\" + relDllPath.replace('/', '\\') + " & wineserver -k\""
val dllPathArg = relDllPath
.replace('/', '\\')
.replace("\"", "\\\"")
val genCmd =
"wine cmd /c \"z:\\generate_interfaces_file.exe \\\"A:\\$dllPathArg\\\" & wineserver -k\""
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt` at line
4145, The generated command concatenates relDllPath unquoted into genCmd in
XServerScreen.kt causing breakage when the DLL path contains spaces or
metacharacters; change the construction of genCmd (the variable using "wine cmd
/c ... generate_interfaces_file.exe A:\\...") to wrap the A:\... argument in
properly escaped quotes and escape any embedded quotes in relDllPath (e.g.,
replace " with \") so the cmd /c payload receives a single quoted path token;
update the string building around genCmd (and any helper that constructs this
payload) to produce ...generate_interfaces_file.exe "A:\<escaped relDllPath>"...
so wineserver -k remains outside the inner quoted argument.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The relDllPath is interpolated unquoted inside the cmd /c "..." payload. If the DLL path contains spaces or cmd metacharacters (e.g., &, ^, "), the command will break or execute unintended fragments. Wrap the A:\... argument in escaped quotes and escape any embedded quotes in the path.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt, line 4145:

<comment>The `relDllPath` is interpolated unquoted inside the `cmd /c "..."` payload. If the DLL path contains spaces or cmd metacharacters (e.g., `&`, `^`, `"`), the command will break or execute unintended fragments. Wrap the `A:\...` argument in escaped quotes and escape any embedded quotes in the path.</comment>

<file context>
@@ -4142,7 +4142,7 @@ private fun unpackExecutableFile(
                             val origDll = File("${imageFs.wineprefix}/dosdevices/a:/$relDllPath")
                             if (origDll.exists()) {
-                                val genCmd = "wine z:\\generate_interfaces_file.exe A:\\" + relDllPath.replace('/', '\\')
+                                val genCmd = "wine cmd /c \"z:\\generate_interfaces_file.exe A:\\" + relDllPath.replace('/', '\\') + " & wineserver -k\""
                                 Timber.i("Running generate_interfaces_file $genCmd")
                                 val genOutput = guestProgramLauncherComponent.execShellCommand(genCmd)
</file context>
Suggested change
val genCmd = "wine cmd /c \"z:\\generate_interfaces_file.exe A:\\" + relDllPath.replace('/', '\\') + " & wineserver -k\""
val dllPathArg = relDllPath
.replace('/', '\\')
.replace("\"", "\\\"")
val genCmd = "wine cmd /c \"z:\\generate_interfaces_file.exe \\\"A:\\$dllPathArg\\\" & wineserver -k\""

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: & runs commands sequentially in cmd — if generate_interfaces_file.exe hangs, wineserver -k never executes

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt, line 4145:

<comment>`&` runs commands sequentially in cmd — if `generate_interfaces_file.exe` hangs, `wineserver -k` never executes</comment>

<file context>
@@ -4142,7 +4142,7 @@ private fun unpackExecutableFile(
                             val origDll = File("${imageFs.wineprefix}/dosdevices/a:/$relDllPath")
                             if (origDll.exists()) {
-                                val genCmd = "wine z:\\generate_interfaces_file.exe A:\\" + relDllPath.replace('/', '\\')
+                                val genCmd = "wine cmd /c \"z:\\generate_interfaces_file.exe A:\\" + relDllPath.replace('/', '\\') + " & wineserver -k\""
                                 Timber.i("Running generate_interfaces_file $genCmd")
                                 val genOutput = guestProgramLauncherComponent.execShellCommand(genCmd)
</file context>

Timber.i("Running generate_interfaces_file $genCmd")
val genOutput = guestProgramLauncherComponent.execShellCommand(genCmd)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -696,9 +696,6 @@ class PerformanceHudView(
val reading = readTemperatureCWithSource(
discoverPrioritizedCpuTempPaths(),
)
if (reading != null) {
Timber.d("[HUD] CPU temp: %d°C from %s", reading.celsius, reading.source)
}
return reading?.celsius
}

Expand All @@ -708,9 +705,6 @@ class PerformanceHudView(
"/sys/class/misc/mali0/device/temp",
) + discoverPrioritizedGpuTempPaths()
val reading = readTemperatureCWithSource(paths)
if (reading != null) {
Timber.d("[HUD] GPU temp: %d°C from %s", reading.celsius, reading.source)
}
return reading?.celsius
}

Expand Down
59 changes: 30 additions & 29 deletions app/src/main/java/com/winlator/core/ProcessHelper.java
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,9 @@ public static int exec(String command, String[] envp, File workingDir) {
// such as SteamTokenLogin
public static String execWithOutput(String command, String[] envp, File workingDir, boolean includeStderr) {
StringBuilder output = new StringBuilder();
final StringBuilder stdoutBuf = new StringBuilder();
final StringBuilder stderrBuf = new StringBuilder();
Thread stdoutDrainer = null;
Thread stderrDrainer = null;
try {
if (BuildConfig.MODERN_ANDROID) command = "/system/bin/linker64 " + command;
Expand All @@ -221,45 +224,43 @@ public static String execWithOutput(String command, String[] envp, File workingD
}
}
if (workingDir != null) pb.directory(workingDir);
pb.redirectErrorStream(includeStderr); // merge only when caller wants stderr

java.lang.Process process = pb.start();

// When not merging, drain stderr on a daemon thread to prevent pipe-buffer deadlock.
// SteamTokenLogin uses this when calling includeStderr=false
if (!includeStderr) {
final InputStream stderrStream = process.getErrorStream();
stderrDrainer = new Thread(() -> {
try {
byte[] buf = new byte[4096];
while (stderrStream.read(buf) != -1) {
if (Thread.currentThread().isInterrupted())
break;
}
} catch (IOException ignored) {}
}, "stderr-drainer");
stderrDrainer.setDaemon(true); // won't block app shutdown if something goes wrong
stderrDrainer.start();
}

// Read stdout (or the merged stream) inline; EOF arrives after the process exits.
try (BufferedReader r = new BufferedReader(new InputStreamReader(process.getInputStream()))) {
String l;
while ((l = r.readLine()) != null) output.append(l).append("\n");
}
final InputStream stdoutStream = process.getInputStream();
stdoutDrainer = new Thread(() -> {
try (BufferedReader r = new BufferedReader(new InputStreamReader(stdoutStream))) {
String l;
while ((l = r.readLine()) != null) stdoutBuf.append(l).append("\n");
} catch (IOException ignored) {}
}, "stdout-drainer");
stdoutDrainer.setDaemon(true);
stdoutDrainer.start();

final InputStream stderrStream = process.getErrorStream();
stderrDrainer = new Thread(() -> {
try (BufferedReader r = new BufferedReader(new InputStreamReader(stderrStream))) {
String l;
while ((l = r.readLine()) != null) {
if (includeStderr) stderrBuf.append(l).append("\n");
}
} catch (IOException ignored) {}
}, "stderr-drainer");
stderrDrainer.setDaemon(true);
stderrDrainer.start();

// Process has already exited (we drained its stdout to EOF).
// waitFor() reaps the OS process-table entry.
process.waitFor();
try { stdoutStream.close(); } catch (IOException ignored) {}
try { stderrStream.close(); } catch (IOException ignored) {}
stdoutDrainer.join(5_000);
stderrDrainer.join(5_000);
Comment on lines +253 to +256

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Streams are closed before drainer threads finish, which can truncate captured output/logs.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/java/com/winlator/core/ProcessHelper.java, line 253:

<comment>Streams are closed before drainer threads finish, which can truncate captured output/logs.</comment>

<file context>
@@ -221,45 +224,43 @@ public static String execWithOutput(String command, String[] envp, File workingD
-            // Process has already exited (we drained its stdout to EOF).
-            // waitFor() reaps the OS process-table entry.
             process.waitFor();
+            try { stdoutStream.close(); } catch (IOException ignored) {}
+            try { stderrStream.close(); } catch (IOException ignored) {}
+            stdoutDrainer.join(5_000);
</file context>
Suggested change
try { stdoutStream.close(); } catch (IOException ignored) {}
try { stderrStream.close(); } catch (IOException ignored) {}
stdoutDrainer.join(5_000);
stderrDrainer.join(5_000);
stdoutDrainer.join(5_000);
stderrDrainer.join(5_000);


if (stderrDrainer != null) {
stderrDrainer.join(5_000); // bounded wait; daemon thread is reaped on JVM exit anyway
}
output.append(stdoutBuf);
if (includeStderr) output.append(stderrBuf);
} catch (Exception e) {
output.append("Error: ").append(e.getMessage());
}

// Format output: trim trailing whitespace/newlines
return output.toString().trim();
}

Expand Down
Loading