Skip to content

fix: write openocd debug output to a file #1274

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

Merged
merged 5 commits into from
Jul 24, 2025
Merged

fix: write openocd debug output to a file #1274

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
755a916
fix: write openocd debug output to a file
kolipakakondal Jul 17, 2025
de12f51
docs: udpate docs on the openocd.log log output
kolipakakondal Jul 18, 2025
9cd8d24
feat: make fileWriter autoclosable and apply formatting
sigmaaa Jul 21, 2025
39609fc
fix: fixed the behavior when append is not used
sigmaaa Jul 23, 2025
f39a5ed
fix: removed useless code
sigmaaa Jul 23, 2025
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
31 changes: 28 additions & 3 deletions ...ag.openocd/src/com/espressif/idf/debug/gdbjtag/openocd/dsf/process/IdfRuntimeProcess.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,13 @@
import java.util.Map;

import org.eclipse.cdt.dsf.gdb.launching.GDBProcess;
import org.eclipse.core.runtime.CoreException;
import org.eclipse.debug.core.DebugPlugin;
import org.eclipse.debug.core.ILaunch;
import org.eclipse.debug.core.model.IStreamsProxy;
import org.eclipse.debug.core.model.RuntimeProcess;
import org.eclipse.debug.internal.core.NullStreamsProxy;
import org.eclipse.debug.ui.IDebugUIConstants;

import com.espressif.idf.core.util.StringUtil;
import com.espressif.idf.debug.gdbjtag.openocd.dsf.process.monitors.StreamsProxy;

/**
Expand Down Expand Up @@ -60,7 +60,32 @@ protected IStreamsProxy createStreamsProxy()
DebugPlugin.log(e);
}
}
StreamsProxy streamsProxy = new StreamsProxy(this, getSystemProcess(), charset, getLabel());
// Use Eclipse Common tab attribute for output file and append
final String outputFileName = getAttributeSafe(getLaunch().getLaunchConfiguration()::getAttribute,
IDebugUIConstants.ATTR_CAPTURE_IN_FILE, "");

final boolean append = getAttributeSafe(getLaunch().getLaunchConfiguration()::getAttribute,
IDebugUIConstants.ATTR_APPEND_TO_FILE, false);
StreamsProxy streamsProxy = new StreamsProxy(this, getSystemProcess(), charset, getLabel(), outputFileName, append);
return streamsProxy;
}

private <T> T getAttributeSafe(AttributeGetter<T> getter, String attribute, T defaultValue)
{
try
{
return getter.get(attribute, defaultValue);
}
catch (CoreException e)
{
DebugPlugin.log(e);
return defaultValue;
}
}

@FunctionalInterface
interface AttributeGetter<T>
{
T get(String attribute, T defaultValue) throws CoreException;
}
}
54 changes: 53 additions & 1 deletion ...bjtag.openocd/src/com/espressif/idf/debug/gdbjtag/openocd/dsf/process/StreamListener.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,14 @@

import java.io.BufferedReader;
import java.io.File;
import java.io.FileWriter;
import java.io.IOException;
import java.io.PrintWriter;
import java.io.StringReader;
import java.nio.charset.Charset;
import java.util.List;

import org.eclipse.core.variables.VariablesPlugin;
import org.eclipse.debug.core.IStreamListener;
import org.eclipse.debug.core.model.IFlushableStreamMonitor;
import org.eclipse.debug.core.model.IProcess;
Expand Down Expand Up @@ -50,11 +53,24 @@ public class StreamListener implements IStreamListener
private boolean fStreamClosed = false;
private List<ReHintPair> reHintsList;

private String outputFile;
private PrintWriter fileWriter;

public StreamListener(IProcess iProcess, IStreamMonitor errorStreamMonitor, IStreamMonitor outputStreamMonitor,
Charset charset)
Charset charset, String outputFile, boolean append)
{
fErrorStreamMonitor = errorStreamMonitor;
fOutputStreamMonitor = outputStreamMonitor;
this.outputFile = outputFile;
String resolvedOutputFile = resolveOutputFilePath(outputFile);
if (resolvedOutputFile != null && !resolvedOutputFile.isEmpty()) {
try {
fileWriter = new PrintWriter(new FileWriter(resolvedOutputFile, append), true);
} catch (IOException e) {
Logger.log(e);
fileWriter = null;
}
}

idfProcessConsole = IdfProcessConsoleFactory.showAndActivateConsole(charset);
// Clear the console only at the beginning, when OpenOCD starts
Expand Down Expand Up @@ -112,6 +128,9 @@ public void streamAppended(String text, IStreamMonitor monitor)
{
while ((line = bufferedReader.readLine()) != null)
{
if (outputFile != null && !outputFile.isEmpty() && fileWriter != null) {
fileWriter.println(line);
}

if (line.startsWith("Error:") && fConsoleErrorOutputStream != null) //$NON-NLS-1$
{
Expand Down Expand Up @@ -186,5 +205,38 @@ public void dispose()
}
fErrorStreamMonitor = null;
fOutputStreamMonitor = null;
if (fileWriter != null) {
fileWriter.close();
fileWriter = null;
}
}

/**
* Resolves the output file path, expands variables, handles directory case, and ensures file/parent directories exist.
*/
private static String resolveOutputFilePath(String outputFile) {
if (outputFile == null || outputFile.isEmpty()) {
return null;
}
try {
// Expand Eclipse variables (e.g., ${workspace_loc:...})
String expanded = VariablesPlugin.getDefault().getStringVariableManager().performStringSubstitution(outputFile);
File file = new File(expanded);
if (file.isDirectory() || (!file.exists() && expanded.endsWith(File.separator))) {
// If it's a directory or ends with a separator, append openocd.log
file = new File(file, "openocd.log");
}
File parent = file.getParentFile();
if (parent != null && !parent.exists()) {
parent.mkdirs();
}
if (!file.exists()) {
file.createNewFile();
}
return file.getAbsolutePath();
} catch (Exception e) {
Logger.log(e);
return null;
}
}
Copy link

@coderabbitai coderabbitai bot Jul 18, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Review file path resolution for security and robustness.

The path resolution logic handles variable expansion and directory creation, but there are some security and robustness concerns:

  1. Directory traversal vulnerability: The method doesn't validate against directory traversal attacks (e.g., ../../../etc/passwd).
  2. Broad exception handling: Catching all Exception types may hide specific issues.
  3. File creation without validation: No checks for file permissions or disk space.

Consider adding security validation:

 private static String resolveOutputFilePath(String outputFile) {
 	if (outputFile == null || outputFile.isEmpty()) {
 		return null;
 	}
 	try {
 		// Expand Eclipse variables (e.g., ${workspace_loc:...})
 		String expanded = VariablesPlugin.getDefault().getStringVariableManager().performStringSubstitution(outputFile);
+		
+		// Validate path to prevent directory traversal
+		File canonicalFile = new File(expanded).getCanonicalFile();
+		String canonicalPath = canonicalFile.getCanonicalPath();
+		
 		File file = new File(expanded);
 		if (file.isDirectory() || (!file.exists() && expanded.endsWith(File.separator))) {
 			// If it's a directory or ends with a separator, append openocd.log
 			file = new File(file, "openocd.log");
 		}
+		
+		// Validate the final path is within expected bounds
+		File finalCanonical = file.getCanonicalFile();
+		
 		File parent = file.getParentFile();
 		if (parent != null && !parent.exists()) {
 			parent.mkdirs();
 		}
 		if (!file.exists()) {
 			file.createNewFile();
 		}
 		return file.getAbsolutePath();
-	} catch (Exception e) {
+	} catch (IOException | SecurityException e) {
 		Logger.log(e);
 		return null;
 	}
 }
📝 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
private static String resolveOutputFilePath(String outputFile) {
if (outputFile == null || outputFile.isEmpty()) {
return null;
}
try {
// Expand Eclipse variables (e.g., ${workspace_loc:...})
String expanded = VariablesPlugin.getDefault().getStringVariableManager().performStringSubstitution(outputFile);
File file = new File(expanded);
if (file.isDirectory() || (!file.exists() && expanded.endsWith(File.separator))) {
// If it's a directory or ends with a separator, append openocd.log
file = new File(file, "openocd.log");
}
File parent = file.getParentFile();
if (parent != null && !parent.exists()) {
parent.mkdirs();
}
if (!file.exists()) {
file.createNewFile();
}
return file.getAbsolutePath();
} catch (Exception e) {
Logger.log(e);
return null;
}
}
private static String resolveOutputFilePath(String outputFile) {
if (outputFile == null || outputFile.isEmpty()) {
return null;
}
try {
// Expand Eclipse variables (e.g., ${workspace_loc:...})
String expanded = VariablesPlugin.getDefault().getStringVariableManager().performStringSubstitution(outputFile);
// Validate path to prevent directory traversal
File canonicalFile = new File(expanded).getCanonicalFile();
String canonicalPath = canonicalFile.getCanonicalPath();
File file = new File(expanded);
if (file.isDirectory() || (!file.exists() && expanded.endsWith(File.separator))) {
// If it's a directory or ends with a separator, append openocd.log
file = new File(file, "openocd.log");
}
// Validate the final path is within expected bounds
File finalCanonical = file.getCanonicalFile();
File parent = file.getParentFile();
if (parent != null && !parent.exists()) {
parent.mkdirs();
}
if (!file.exists()) {
file.createNewFile();
}
return file.getAbsolutePath();
} catch (IOException | SecurityException e) {
Logger.log(e);
return null;
}
}
🤖 Prompt for AI Agents 
In
bundles/com.espressif.idf.debug.gdbjtag.openocd/src/com/espressif/idf/debug/gdbjtag/openocd/dsf/process/StreamListener.java
between lines 217 and 241, enhance the resolveOutputFilePath method by adding
validation to prevent directory traversal attacks, such as normalizing the path
and ensuring it stays within an allowed base directory. Replace the broad
Exception catch with more specific exceptions to handle expected errors
explicitly. Before creating the file, add checks for write permissions and
available disk space to avoid failures or security issues.

💡 Verification agent

🧩 Analysis chain

Consider thread safety for concurrent file operations.

The resolveOutputFilePath method handles variable expansion, directory creation, and file creation well. However, consider potential race conditions if multiple processes attempt to create the same directory or file simultaneously.

Additionally, the method could benefit from more specific exception handling:

-		} catch (Exception e) {
+		} catch (IOException e) {
+			Logger.log(e);
+			return null;
+		} catch (CoreException e) {
+			Logger.log(e);
+			return null;
+		} catch (Exception e) {
 			Logger.log(e);
 			return null;
 		}

🏁 Script executed:

#!/bin/bash
# Description: Check if there are multiple instances of StreamListener that could create race conditions
# Expected: Find usage patterns to assess concurrent access risks

echo "=== Searching for StreamListener instantiation patterns ==="
rg -A 10 -B 5 "new StreamListener" 

echo "=== Checking for concurrent process creation ==="
rg -A 5 -B 5 "StreamsProxy.*new.*StreamListener"

echo "=== Verifying if launch configurations can be shared ==="
rg -A 10 "ILaunch.*getAttribute.*CAPTURE_IN_FILE"

Length of output: 3363

Refine exception handling and review concurrency in resolveOutputFilePath

  • In bundles/com.espressif.idf.debug.gdbjtag.openocd/src/com/espressif/idf/debug/gdbjtag/openocd/dsf/process/StreamListener.java (lines 217–241), replace the broad catch block with more specific handlers:

    	private static String resolveOutputFilePath(String outputFile) {
		…
		try {
			…
		} catch (IOException e) {
			Logger.log(e);
			return null;
		} catch (CoreException e) {
			Logger.log(e);
			return null;
		} catch (Exception e) {
			Logger.log(e);
			return null;
		}
	}

  • Although OS‐level mkdirs() and createNewFile() are generally safe to call concurrently, if resolveOutputFilePath can be invoked in parallel (e.g., by multiple StreamListener instances in StreamsProxy.java), review whether you need stronger guarantees:

    • Use Files.createDirectories(parent.toPath()) and Files.createFile(path) for atomic directory/file creation.
    • Synchronize access to this method or employ a file lock when targeting the same output path across threads.
📝 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
private static String resolveOutputFilePath(String outputFile) {
if (outputFile == null || outputFile.isEmpty()) {
return null;
}
try {
// Expand Eclipse variables (e.g., ${workspace_loc:...})
String expanded = VariablesPlugin.getDefault().getStringVariableManager().performStringSubstitution(outputFile);
File file = new File(expanded);
if (file.isDirectory() || (!file.exists() && expanded.endsWith(File.separator))) {
// If it's a directory or ends with a separator, append openocd.log
file = new File(file, "openocd.log");
}
File parent = file.getParentFile();
if (parent != null && !parent.exists()) {
parent.mkdirs();
}
if (!file.exists()) {
file.createNewFile();
}
return file.getAbsolutePath();
} catch (Exception e) {
Logger.log(e);
return null;
}
}
private static String resolveOutputFilePath(String outputFile) {
if (outputFile == null || outputFile.isEmpty()) {
return null;
}
try {
// Expand Eclipse variables (e.g., ${workspace_loc:...})
String expanded = VariablesPlugin.getDefault().getStringVariableManager().performStringSubstitution(outputFile);
File file = new File(expanded);
if (file.isDirectory() || (!file.exists() && expanded.endsWith(File.separator))) {
// If it's a directory or ends with a separator, append openocd.log
file = new File(file, "openocd.log");
}
File parent = file.getParentFile();
if (parent != null && !parent.exists()) {
parent.mkdirs();
}
if (!file.exists()) {
file.createNewFile();
}
return file.getAbsolutePath();
} catch (IOException e) {
Logger.log(e);
return null;
} catch (CoreException e) {
Logger.log(e);
return null;
} catch (Exception e) {
Logger.log(e);
return null;
}
}
🤖 Prompt for AI Agents 
In
bundles/com.espressif.idf.debug.gdbjtag.openocd/src/com/espressif/idf/debug/gdbjtag/openocd/dsf/process/StreamListener.java
around lines 217 to 241, replace the broad Exception catch with specific
exception types to improve error handling clarity. Change directory and file
creation calls to use Files.createDirectories(parent.toPath()) and
Files.createFile(path) for atomic operations. Additionally, if this method can
be called concurrently for the same output path, add synchronization or file
locking to prevent race conditions during directory and file creation.

}
4 changes: 2 additions & 2 deletions ...penocd/src/com/espressif/idf/debug/gdbjtag/openocd/dsf/process/monitors/StreamsProxy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,14 +62,14 @@ public class StreamsProxy implements IBinaryStreamsProxy {
* @param charset the process's charset or <code>null</code> if default
* @param processLabel The name for the process label
*/
public StreamsProxy(IProcess iProcess, Process process, Charset charset, String processLabel) {
public StreamsProxy(IProcess iProcess, Process process, Charset charset, String processLabel, String outputFile, boolean append) {
if (process == null) {
return;
}
fOutputMonitor = new CustomOutputStreamMonitor(process.getInputStream(), charset);
fErrorMonitor = new CustomOutputStreamMonitor(process.getErrorStream(), charset);
// Our own addition to make sure that we utilize only one listener for all streams
StreamListener streamListener = new StreamListener(iProcess, fErrorMonitor, fOutputMonitor, charset);
StreamListener streamListener = new StreamListener(iProcess, fErrorMonitor, fOutputMonitor, charset, outputFile, append);
fOutputMonitor.addListener(streamListener);
fErrorMonitor.addListener(streamListener);
fInputMonitor = new InputStreamMonitor(process.getOutputStream(), charset);
Expand Down
3 changes: 3 additions & 0 deletions docs/en/openocddebugging.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,9 @@ You can save your debug logs to an external file. To do this:
.. note::
Path to the file can be relative if it's located in the workspace (see screenshot below)

.. note::
When specifying a directory path (ending with a separator like ``/`` or ``\``), the system will automatically append ``openocd.log`` as the filename. For example, entering ``/tmp/logs/`` will create the file as ``/tmp/logs/openocd.log``.

.. image:: ../../media/OpenOCDDebug_13.png

Preferences for OpenOCD Configuration
Expand Down
Loading