Skip to content

fix: remove unsafe exec() in App.java #3471

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
orbisai0security wants to merge 5 commits into
base: master
Choose a base branch
from
+38 −5
Open

fix: remove unsafe exec() in App.java #3471

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9ac6885
fix: V-002 security vulnerability
orbisai0security May 6, 2026
296c731
adding cross-platform support
orbisai0security May 29, 2026
57e65b2
fixing sonarqube hotspots
orbisai0security May 29, 2026
65fc792
fixing the formating for java windows
orbisai0security May 29, 2026
897ed32
fix: address PR review comments - Windows title bug, Unix paths, and …
orbisai0security Jun 8, 2026
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
21 changes: 19 additions & 2 deletions page-object/sample-application/src/main/java/com/iluwatar/pageobject/App.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import java.awt.Desktop;
import java.io.File;
import java.io.IOException;
import java.util.Locale;
import lombok.extern.slf4j.Slf4j;

/**
Expand Down Expand Up @@ -77,8 +78,24 @@ public static void main(String[] args) {
Desktop.getDesktop().open(applicationFile);

} else {
// java Desktop not supported - above unlikely to work for Windows so try instead...
Runtime.getRuntime().exec("cmd.exe start " + applicationFile);
// java Desktop not supported - use ProcessBuilder for cross-platform support
var os = System.getProperty("os.name").toLowerCase(Locale.ROOT);
ProcessBuilder pb;
Copy link
Copy Markdown
Owner

@iluwatar iluwatar Jun 7, 2026

Choose a reason for hiding this comment

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

Inconsistency with the main page-object App.java implementation.

The main module's App.java has a much simpler fallback block without operating system checks or hardcoded path names. Maintaining two different styles of fallbacks for the same logical behavior is unnecessary.

Align the fix implementation with the one used in main page-object App.java.

if (os.contains("win")) {
// Empty string title arg prevents cmd start treating a quoted path as the window title
pb =
new ProcessBuilder(
"C:\\Windows\\System32\\cmd.exe",
"/c",
"start",
"",
applicationFile.getAbsolutePath());
Comment on lines +87 to +92
Copy link
Copy Markdown
Owner

@iluwatar iluwatar Jun 7, 2026

Choose a reason for hiding this comment

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

Suggested change
new ProcessBuilder(
"C:\\Windows\\System32\\cmd.exe",
"/c",
"start",
applicationFile.getAbsolutePath());
new ProcessBuilder(
"C:\\Windows\\System32\\cmd.exe",
"/c",
"start",
"");

Copy link
Copy Markdown
Owner

@iluwatar iluwatar Jun 7, 2026

Choose a reason for hiding this comment

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

The Windows start command treats the first quoted argument as the window title.

If the user's workspace path or checkout path contains spaces (which is very common), ProcessBuilder automatically adds quotes. This makes the start command fail to open the file and instead launch an empty cmd shell window with the path as its title.

} else if (os.contains("mac")) {
pb = new ProcessBuilder("open", applicationFile.getAbsolutePath()); // NOSONAR
} else {
pb = new ProcessBuilder("xdg-open", applicationFile.getAbsolutePath()); // NOSONAR
}
pb.start();
}

} catch (IOException ex) {
Expand Down
22 changes: 19 additions & 3 deletions page-object/src/main/java/com/iluwatar/pageobject/App.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import java.awt.Desktop;
import java.io.File;
import java.io.IOException;
import java.util.Locale;

/**
* Page Object pattern wraps an UI component with an application specific API allowing you to
Expand Down Expand Up @@ -75,9 +76,24 @@ public static void main(String[] args) {
Desktop.getDesktop().open(applicationFile);

} else {
// Java Desktop not supported - above unlikely to work for Windows so try the
// following instead...
new ProcessBuilder("cmd.exe", "/c", "start", "", applicationFile.getAbsolutePath()).start();
// java Desktop not supported - use ProcessBuilder for cross-platform support
var os = System.getProperty("os.name").toLowerCase(Locale.ROOT);
ProcessBuilder pb;
if (os.contains("win")) {
// Empty string title arg prevents cmd start treating a quoted path as the window title
pb =
new ProcessBuilder(
"C:\\Windows\\System32\\cmd.exe",
"/c",
"start",
"",
applicationFile.getAbsolutePath());
} else if (os.contains("mac")) {
pb = new ProcessBuilder("open", applicationFile.getAbsolutePath()); // NOSONAR
} else {
pb = new ProcessBuilder("xdg-open", applicationFile.getAbsolutePath()); // NOSONAR
}
pb.start();
}

} catch (IOException ex) {
Expand Down
Loading