fix: fixing bug with unpacking new eim version on linux - #1498
Conversation
📝 WalkthroughWalkthroughThe PR moves EIM ZIP extraction into ChangesEIM extraction flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant EimLoader
participant EimZipExtractor
participant DestinationDirectory
EimLoader->>EimZipExtractor: extract(zipPath, destDir)
EimZipExtractor->>DestinationDirectory: create and materialize ZIP entries
EimZipExtractor->>DestinationDirectory: repair materialized eim payload
EimZipExtractor-->>EimLoader: return preferred launch path
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
tests/com.espressif.idf.core.test/src/com/espressif/idf/core/tools/test/EimZipExtractorTest.java (1)
83-109: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd a ZIP traversal regression test.
The new extractor rejects traversal entries, but this test suite does not protect that behavior. Create a ZIP entry such as
"../escaped". Assert thatEimZipExtractor.extractthrowsIOException. Assert that no file is created outside the destination.Proposed test
+import static org.junit.jupiter.api.Assertions.assertThrows; + + `@Test` + void rejectsZipEntryOutsideDestination() throws Exception + { + Path zip = tempDir.resolve("traversal.zip"); + Path dest = tempDir.resolve("out"); + writeSingleFileZip(zip, "../escaped", "payload"); + + assertThrows(IOException.class, () -> EimZipExtractor.extract(zip, dest)); + assertFalse(Files.exists(tempDir.resolve("escaped"))); + }🤖 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 `@tests/com.espressif.idf.core.test/src/com/espressif/idf/core/tools/test/EimZipExtractorTest.java` around lines 83 - 109, Add a regression test in EimZipExtractorTest that creates a ZIP containing a "../escaped" traversal entry, invokes EimZipExtractor.extract, and asserts IOException is thrown. Verify afterward that the escaped path outside the destination does not exist, using the existing temporary-directory setup and test conventions.
🤖 Prompt for all review comments with 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.
Inline comments:
In
`@bundles/com.espressif.idf.core/src/com/espressif/idf/core/tools/EimZipExtractor.java`:
- Around line 200-220: Update EimZipExtractor’s archive extraction flow and
resolvePreferredLaunchPath so candidate stable and versioned EIM paths are
tracked only while processing the current ZIP, rather than discovered from all
files in destDir. Exclude pre-existing stable and versioned binaries when
selecting the launch path, preserve firstRegularFile fallback behavior, and add
an update regression test covering a pre-existing EIM file.
- Around line 163-170: Update looksLikeMaterializedSymlinkPayload to apply the
same target validation as repairMaterializedEimSymlink: reject carriage returns
and the ".." target in addition to the existing empty, newline, and
path-separator checks. Reuse the existing validation logic or shared helper so
payload detection and repair accept identical targets.
---
Nitpick comments:
In
`@tests/com.espressif.idf.core.test/src/com/espressif/idf/core/tools/test/EimZipExtractorTest.java`:
- Around line 83-109: Add a regression test in EimZipExtractorTest that creates
a ZIP containing a "../escaped" traversal entry, invokes
EimZipExtractor.extract, and asserts IOException is thrown. Verify afterward
that the escaped path outside the destination does not exist, using the existing
temporary-directory setup and test conventions.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a82a144a-ea30-482b-83ce-76d1702b589f
📒 Files selected for processing (3)
bundles/com.espressif.idf.core/src/com/espressif/idf/core/tools/EimLoader.javabundles/com.espressif.idf.core/src/com/espressif/idf/core/tools/EimZipExtractor.javatests/com.espressif.idf.core.test/src/com/espressif/idf/core/tools/test/EimZipExtractorTest.java
|
@sigmaaa hi ! EIM archive extraction issue fixed ✅ @kolipakakondal please, review |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
tests/com.espressif.idf.core.test/src/com/espressif/idf/core/tools/test/EimZipExtractorTest.java (1)
68-80: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winBase fallback assertions on symlink creation outcome.
repairMaterializedEimSymlink()catches symlink creation failures and copies the target on Windows and non-Windows. This test still expects every non-Windows run to create a realFiles.isSymbolicLink(eim); run it on a Linux filesystem without symlink support, and the fallback branch is untested. Use aFiles.isSymbolicLink(eim)-based branch, or make the symlink creation deterministically fail.🤖 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 `@tests/com.espressif.idf.core.test/src/com/espressif/idf/core/tools/test/EimZipExtractorTest.java` around lines 68 - 80, Update the assertions in EimZipExtractorTest to branch on Files.isSymbolicLink(eim) rather than the operating system, validating the symlink target when creation succeeds and the regular-file contents when fallback copying occurs. Keep the shared content and executable assertions appropriate to each outcome.bundles/com.espressif.idf.core/src/com/espressif/idf/core/tools/EimZipExtractor.java (1)
60-73: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftContain link resolution within
destDir.
normalize()is lexical only, whileFiles.copy,Files.isRegularFile, and subsequent symlink repair operations can follow links before or after thestartsWith()check. IfdestDiror an archive-created parent is a symlink, entries can be extracted or repaired outsidedestDir. Extract into a fresh real staging directory first, or validate every component and target withLinkOption.NOFOLLOW_LINKSand real-path containment before write/repair operations.Also applies to extraction at lines 60-80, repair targets at lines 120-141, and
looksLikeMaterializedSymlinkPayloadat lines 167-187.🤖 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 `@bundles/com.espressif.idf.core/src/com/espressif/idf/core/tools/EimZipExtractor.java` around lines 60 - 73, Harden EimZipExtractor so extraction and symlink repair cannot follow links outside destDir: use a fresh real staging directory for archive writes, or validate every path component and resolved target with NOFOLLOW_LINKS and real-path containment before any Files.copy, directory creation, or repair operation. Apply the same containment guarantees to the extraction flow, repair targets, and looksLikeMaterializedSymlinkPayload, preserving valid in-destination entries.
🤖 Prompt for all review comments with 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.
Inline comments:
In
`@bundles/com.espressif.idf.core/src/com/espressif/idf/core/tools/EimZipExtractor.java`:
- Around line 176-180: Update isSafeSymlinkTargetName to reject Windows-invalid
filename characters, including ?, *, ", <, >, and |, before any path resolution
occurs. Preserve the existing checks for empty names, newlines, carriage
returns, path separators, and "..", ensuring malformed archive symlink targets
are rejected safely.
---
Outside diff comments:
In
`@bundles/com.espressif.idf.core/src/com/espressif/idf/core/tools/EimZipExtractor.java`:
- Around line 60-73: Harden EimZipExtractor so extraction and symlink repair
cannot follow links outside destDir: use a fresh real staging directory for
archive writes, or validate every path component and resolved target with
NOFOLLOW_LINKS and real-path containment before any Files.copy, directory
creation, or repair operation. Apply the same containment guarantees to the
extraction flow, repair targets, and looksLikeMaterializedSymlinkPayload,
preserving valid in-destination entries.
In
`@tests/com.espressif.idf.core.test/src/com/espressif/idf/core/tools/test/EimZipExtractorTest.java`:
- Around line 68-80: Update the assertions in EimZipExtractorTest to branch on
Files.isSymbolicLink(eim) rather than the operating system, validating the
symlink target when creation succeeds and the regular-file contents when
fallback copying occurs. Keep the shared content and executable assertions
appropriate to each outcome.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 785bf035-c6ef-49ae-a96e-16d470af7965
📒 Files selected for processing (2)
bundles/com.espressif.idf.core/src/com/espressif/idf/core/tools/EimZipExtractor.javatests/com.espressif.idf.core.test/src/com/espressif/idf/core/tools/test/EimZipExtractorTest.java
| public static boolean isSafeSymlinkTargetName(String target) | ||
| { | ||
| return !target.isEmpty() && !target.contains("\n") && !target.contains("\r") //$NON-NLS-1$ //$NON-NLS-2$ | ||
| && !target.contains("/") && !target.contains("\\") && !target.contains(".."); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- locate file ---\n'
git ls-files | rg '^bundles/com\.espressif\.idf\.core/src/com/espressif/idf/core/tools/EimZipExtractor\.java$' || true
printf '\n--- outline ---\n'
ast-grep outline bundles/com.espressif.idf.core/src/com/espressif/idf/core/tools/EimZipExtractor.java --view expanded || true
printf '\n--- relevant source ---\n'
sed -n '1,240p' bundles/com.espressif.idf.core/src/com/espressif/idf/core/tools/EimZipExtractor.java
printf '\n--- search symlink handling ---\n'
rg -n "isSafeSymlinkTargetName|createSymbolicLink|Path\.of|resolve\(target\)|IOException|InvalidPathException" bundles/com.espressif.idf.core/src/com/espressif/idf/core/tools/EimZipExtractor.javaRepository: espressif/idf-eclipse-plugin
Length of output: 257
🌐 Web query:
Java Path.of Windows invalid path characters InvalidPathException documentation
💡 Result:
The java.nio.file.InvalidPathException is an unchecked exception thrown when a path string cannot be converted into a Path object [1][2]. On Windows, this commonly occurs when Path.of(String) or Paths.get(String) encounters characters that are illegal in a Windows file system path, or when the path format violates Windows-specific path conventions [1][3]. Key aspects of InvalidPathException and its behavior with Path.of on Windows include: 1. Nature of the Exception: It is a subclass of IllegalArgumentException [1]. It provides the input string, a reason for the rejection, and the index in the string where the error occurred [1][4]. 2. Common Causes on Windows: - Illegal Characters: Windows restricts characters such as <, >, :, ", |, ?, and * in file names (except in specific contexts like drive letters or UNC paths). An error such as "Illegal char <:> at index 2" often indicates a malformed path, such as an absolute path starting with a leading slash (e.g., /C:/...) which the Windows parser does not recognize as a valid root [3][5][6]. - Invisible Characters: Non-printing or zero-width Unicode characters (e.g., U+202A) can sometimes appear in paths copied from Windows UI elements, triggering this exception even if the path appears correct [3]. - Formatting Issues: While Path.of has been updated in newer JDKs to handle long path prefixes like \\?\ and \\?\UNC\ [7], incorrect manual construction of these strings or leading slashes on Windows paths frequently leads to InvalidPathException [6]. When troubleshooting this exception, examine the "index" provided in the exception message to identify exactly where the parser encountered the invalid character or format, and ensure that path strings do not contain unexpected leading slashes or hidden characters when operating on Windows [1][3].
Citations:
- 1: https://docs.oracle.com/en/java/javase/26/docs/api/java.base/java/nio/file/InvalidPathException.html
- 2: https://docs.oracle.com/en/java/javase/25/docs/api/java.base/java/nio/file/InvalidPathException.html
- 3: https://stackoverflow.com/questions/57197286/why-does-this-throw-java-nio-file-invalidpathexception
- 4: https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/nio/file/InvalidPathException.html
- 5: https://bugs.openjdk.org/browse/JDK-8314511
- 6:
PathMatchingResourcePatternResolvercan no longer resolve files on Windows spring-projects/spring-framework#29226 - 7: https://bugs.openjdk.org/browse/JDK-8302016
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- locate file ---'
git ls-files | rg '^bundles/com\.espressif\.idf\.core/src/com/espressif/idf/core/tools/EimZipExtractor\.java$' || true
printf '%s\n' '--- outline ---'
ast-grep outline bundles/com.espressif.idf.core/src/com/espressif/idf/core/tools/EimZipExtractor.java --view expanded || true
printf '%s\n' '--- relevant source ---'
sed -n '1,240p' bundles/com.espressif.idf.core/src/com/espressif/idf/core/tools/EimZipExtractor.java
printf '%s\n' '--- search symlink handling ---'
rg -n 'isSafeSymlinkTargetName|createSymbolicLink|Path\.of|resolve\(target\)|IOException|InvalidPathException' bundles/com.espressif.idf.core/src/com/espressif/idf/core/tools/EimZipExtractor.javaRepository: espressif/idf-eclipse-plugin
Length of output: 9782
🌐 Web query:
Java 9 Path.of source WindowsPathParser invalidPathCharacters IllegalChar
💡 Result:
The java.nio.file.InvalidPathException: Illegal char error, often seen with Path.of() or Paths.get() on Windows, occurs because the internal sun.nio.fs.WindowsPathParser enforces strict validation of path strings based on Windows file system naming constraints [1][2][3]. The WindowsPathParser uses a reserved set of characters—< > : " | ? *—as well as control characters (ASCII values below 32) that are considered illegal in Windows filenames [3]. When Path.of() or Paths.get() receives a string containing these characters, the parser throws an InvalidPathException [1][4]. Key points regarding this behavior include: 1. Nature of the Exception: The error occurs because the input string is being interpreted as a raw file path, and it contains characters that are invalid for Windows file system objects [3][4]. A common cause is passing a string that is not a valid file path, such as a URI (e.g., classpath:/...), a string with leading/trailing brackets (often from improper Arrays.toString() usage), or a device name (e.g., NUL:) that the parser does not recognize as a valid path prefix [5][6][4]. 2. Role of WindowsPathParser: This internal class is responsible for decomposing path strings into root, type, and path components [7][3]. The isInvalidPathChar method explicitly checks against the restricted character set [3]. 3. Path.of vs. Paths.get: Path.of() was introduced in Java 9 as a preferred, modern alternative to Paths.get() [8][9]. Both methods ultimately rely on the same underlying file system implementation (FileSystems.getDefault().getPath()) and are subject to the same validation logic in WindowsPathParser [10][6][4]. If you encounter this exception, ensure the input string does not contain reserved characters or formatting artifacts, and verify that you are not passing a URI or a non-file-system string directly into a method expecting a file system path [6][4]. If you need to convert a URI, use Path.of(URI.create(yourUriString)) instead of passing the URI string directly to the path constructor [11][9].
Citations:
- 1: https://bugs.openjdk.org/browse/JDK-8314511
- 2: https://bugs.openjdk.org/browse/JDK-8233294
- 3: https://github.com/frohoff/jdk8u-jdk/blob/master/src/windows/classes/sun/nio/fs/WindowsPathParser.java
- 4: https://stackoverflow.com/questions/56736003/why-am-i-getting-this-invalidpathexception
- 5: https://mail.openjdk.org/pipermail/core-libs-dev/2021-March/075383.html
- 6: API Spec cannot be read from classpath or JAR on *Windows* file system swagger-api/swagger-parser#1628
- 7: https://github.com/openjdk/jdk/blob/master/src/java.base/windows/classes/sun/nio/fs/WindowsPath.java
- 8: https://bugs.openjdk.org/browse/JDK-8199485
- 9: https://dev.java/learn/java-io/file-system/path/
- 10: https://tech-learn.dev/java/java-nio/java-nio-path
- 11: https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/nio/file/Path.html
🌐 Web query:
Java Path.of platform invalid names JavaPathParser InvalidPathException NUL reserved names
💡 Result:
In Java, the Path.of method (and the underlying Path API) utilizes the system-default FileSystem to parse path strings [1][2]. When a path string is provided, the parser validates it against platform-specific constraints [1]. If a string contains illegal characters or is otherwise invalid for the underlying file system, an InvalidPathException is thrown [1][3]. Regarding NUL characters (\u0000): The NUL character is strictly forbidden in path strings across supported platforms [1]. Java explicitly checks for this character during path parsing [4]. For example, in the Unix implementation, the parser performs a specific check for NUL (\u0000) and throws an InvalidPathException if one is encountered [4]. Regarding Reserved Names (e.g., NUL, CON, PRN): The behavior regarding Windows reserved names (such as NUL, CON, AUX, COMx, LPTx) is more nuanced than the handling of the NUL character [5]. 1. Path.of and Related API Validation: While Path.of will throw an InvalidPathException for illegal characters (like colons used in "NUL:" or invalid filename characters like <, >, |, etc.) [6][7][8], it does not automatically reject strings that are simply reserved Windows keywords (like "NUL") [5]. 2. File System Interactions: These reserved names are legacies of DOS/Windows device files [5]. Attempting to use them as file paths may succeed or fail depending on the specific file system and the exact context (e.g., creating a file vs. a directory) [5]. Because they are reserved for system devices, interactions with these paths at the operating system level can lead to "incorrect function" errors or other I/O exceptions rather than an InvalidPathException during the path parsing phase [9]. Summary: - InvalidPathException is primarily thrown for structural issues, such as illegal characters (including NUL bytes) or malformed path syntax [1][3]. - Reserved names (e.g., "NUL") are often accepted as valid Path objects because they are technically valid strings, but they may trigger OS-level errors when actual I/O operations are attempted [5][9].
Citations:
- 1: https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/nio/file/FileSystem.html
- 2: https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/nio/file/Path.html
- 3: https://developer.android.com/reference/java/nio/file/InvalidPathException
- 4: https://github.com/openjdk/jdk/blob/master/src/java.base/unix/classes/sun/nio/fs/UnixPath.java
- 5: https://stackoverflow.com/questions/75815313/mkdir-can-create-folders-with-windows-reserved-keyword-names
- 6: https://stackoverflow.com/questions/78579599/does-the-file-class-have-an-equivalent-of-the-invalidpathexception-of-the-p
- 7: https://stackoverflow.com/questions/66893799/java-serialization-invalidpathexception-illegal-char-at-index-13-2021-03
- 8: https://mail.openjdk.org/pipermail/core-libs-dev/2021-March/075383.html
- 9: https://mail.openjdk.org/pipermail/core-libs-dev/2021-March/075391.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- inspect repository file header/imports/style ---'
sed -n '1,80p' bundles/com.espressif.idf.core/src/com/espressif/idf/core/tools/EimZipExtractor.java | cat -n
printf '%s\n' '--- count tabs vs spaces in target method region ---'
sed -n '180,210p' bundles/com.espressif.idf.core/src/com/espressif/idf/core/tools/EimZipExtractor.java | cat -n | rg -n $'\t| ' || trueRepository: espressif/idf-eclipse-plugin
Length of output: 5186
Reject platform-invalid symlink target names before path resolution.
isSafeSymlinkTargetName accepts ?, *, ", <, >, |, and other platform-invalid Windows filename characters. On Windows, destDir.resolve(target).normalize() and Path.of(target) can throw an unchecked parse failure before the symlink-copy fallback runs, so a malformed archive payload aborts extraction. Reject platform-invalid characters in isSafeSymlinkTargetName, or catch InvalidPathException around path resolution and symlink creation.
🤖 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
`@bundles/com.espressif.idf.core/src/com/espressif/idf/core/tools/EimZipExtractor.java`
around lines 176 - 180, Update isSafeSymlinkTargetName to reject Windows-invalid
filename characters, including ?, *, ", <, >, and |, before any path resolution
occurs. Preserve the existing checks for empty names, newlines, carriage
returns, path separators, and "..", ensuring malformed archive symlink targets
are rejected safely.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with 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.
Inline comments:
In
`@bundles/com.espressif.idf.core/src/com/espressif/idf/core/tools/EimZipExtractor.java`:
- Around line 108-114: The repair flow in repairMaterializedEimSymlink must
verify that targetPath is also present in extractedFromArchive before restoring
the eim symlink or copy. When the target is absent, reject the payload or
exclude eim from launch candidates, and add a regression test covering an
archive referencing a pre-existing versioned file.
- Around line 56-59: Update EimZipExtractor.extract to inspect every existing
path component from destDir to each archive entry and reject any symbolic link
before Files.createDirectories or Files.copy executes; retain the existing
lexical traversal protection and fail extraction with an appropriate
IOException. Add a regression test using a pre-existing directory symlink
beneath the destination, assert extraction is rejected, and verify the symlink’s
external target remains unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1a392414-0b00-4094-b008-d8bab7f12527
📒 Files selected for processing (2)
bundles/com.espressif.idf.core/src/com/espressif/idf/core/tools/EimZipExtractor.javatests/com.espressif.idf.core.test/src/com/espressif/idf/core/tools/test/EimZipExtractorTest.java
| public static Path extract(Path zipPath, Path destDir) throws IOException | ||
| { | ||
| Files.createDirectories(destDir); | ||
| Set<Path> extractedFiles = new LinkedHashSet<>(); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Reject existing symlink components during extraction.
The lexical check does not stop Files.createDirectories and Files.copy from following an existing directory symlink below destDir. If destDir/plugins links outside destDir, an archive entry named plugins/file passes the check and writes outside the extraction directory.
Reject symlink path components before writing each entry. Add a regression test with a pre-existing directory symlink and verify that its external target remains unchanged.
🤖 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
`@bundles/com.espressif.idf.core/src/com/espressif/idf/core/tools/EimZipExtractor.java`
around lines 56 - 59, Update EimZipExtractor.extract to inspect every existing
path component from destDir to each archive entry and reject any symbolic link
before Files.createDirectories or Files.copy executes; retain the existing
lexical traversal protection and fail extraction with an appropriate
IOException. Add a regression test using a pre-existing directory symlink
beneath the destination, assert extraction is rejected, and verify the symlink’s
external target remains unchanged.
| static void repairMaterializedEimSymlink(Path destDir, Collection<Path> extractedFromArchive) throws IOException | ||
| { | ||
| Path eim = destDir.resolve("eim").normalize(); //$NON-NLS-1$ | ||
| if (!extractedFromArchive.contains(eim)) | ||
| { | ||
| return; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Require the symlink target to come from the current archive.
extractedFromArchive confirms only that eim came from the archive. It does not confirm that targetPath came from the archive. If the ZIP contains eim with payload eim_v0.17.4 and destDir retains an old eim_v0.17.4, the repair restores a link or copy of the old executable. Launch-path selection then returns that stable eim path.
Require extractedFromArchive.contains(targetPath) before repair. If the target is absent, reject the invalid payload or exclude eim from launch candidates. Add a regression test for an archived payload that references a pre-existing versioned file.
🤖 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
`@bundles/com.espressif.idf.core/src/com/espressif/idf/core/tools/EimZipExtractor.java`
around lines 108 - 114, The repair flow in repairMaterializedEimSymlink must
verify that targetPath is also present in extractedFromArchive before restoring
the eim symlink or copy. When the target is absent, reject the payload or
exclude eim from launch candidates, and add a regression test covering an
archive referencing a pre-existing versioned file.
|
Tested with latest changes from Peter in EIM 0.18 |
2 participants
Description
Bug
Opening ESP-IDF Terminal after installing EIM via Espressif-IDE could launch the EIM GUI on Linux.
Newer EIM zips ship eim_vX.Y.Z plus an eim symlink. IDE unzip (ZipInputStream) turned that symlink into a tiny text file. On later startup, PATH lookup gave it +x. The activation script then ran eim select …; because eim was not a real binary/symlink, args were not forwarded and the GUI opened by default.
Older zips with only a plain eim binary were fine.
Fix
After unzip, detect that broken eim payload and restore a real symlink to eim_vX.Y.Z (or copy the target if symlinks are unavailable). Prefer the stable eim path for launch. Legacy single-binary packages are left unchanged.
Fixes # (([~>EI-65397])](https://athena.espressif.cn:6565/w/xzzYSdgR))
Type of change
Please delete options that are not relevant.
How has this been tested?
Download the older eim version without eim_vX.Y.Z executable and repeat step 1
Test Configuration:
Dependent components impacted by this PR:
Checklist
Summary by CodeRabbit
Bug Fixes