fix: fixing bug with unpacking new eim version on linux #1498
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,257 @@ | ||
| /******************************************************************************* | ||
| * Copyright 2026 Espressif Systems (Shanghai) PTE LTD. All rights reserved. | ||
| * Use is subject to license terms. | ||
| *******************************************************************************/ | ||
| package com.espressif.idf.core.tools; | ||
|
|
||
| import java.io.IOException; | ||
| import java.io.InputStream; | ||
| import java.nio.charset.StandardCharsets; | ||
| import java.nio.file.Files; | ||
| import java.nio.file.Path; | ||
| import java.nio.file.StandardCopyOption; | ||
| import java.util.Collection; | ||
| import java.util.Comparator; | ||
| import java.util.LinkedHashSet; | ||
| import java.util.Optional; | ||
| import java.util.Set; | ||
| import java.util.zip.ZipEntry; | ||
| import java.util.zip.ZipInputStream; | ||
|
|
||
| import org.eclipse.core.runtime.Platform; | ||
|
|
||
| import com.espressif.idf.core.logging.Logger; | ||
|
|
||
| /** | ||
| * Extracts EIM release zip archives. | ||
| * <p> | ||
| * Newer Linux/macOS packages ship a versioned binary ({@code eim_vX.Y.Z}) plus an {@code eim} | ||
| * symlink. {@link ZipInputStream} materializes that symlink as a tiny regular file whose contents | ||
| * are the link target. Running that file with arguments (e.g. {@code eim select …}) does not | ||
| * forward argv to the real binary and can open the GUI. This extractor detects that case and | ||
| * recreates a real symlink (or copies the target if symlinks are unsupported). | ||
| * <p> | ||
| * Older packages ship only a plain {@code eim} binary — left unchanged. | ||
| * <p> | ||
| * Launch-path selection considers only files from the current archive, so leftover binaries in | ||
| * {@code destDir} from a previous install are not chosen. | ||
| */ | ||
| public final class EimZipExtractor | ||
| { | ||
| /** | ||
| * Symlink targets stored as ZIP entry payloads are short path strings (e.g. {@code eim_v0.17.4}). | ||
| * Real EIM binaries are multi‑MB; keep this well below any plausible binary size. | ||
| */ | ||
| private static final long MAX_SYMLINK_PAYLOAD_BYTES = 512; | ||
|
|
||
| private EimZipExtractor() | ||
| { | ||
| } | ||
|
|
||
| /** | ||
| * Extracts {@code zipPath} into {@code destDir} and returns the preferred EIM launch path | ||
| * ({@code eim} / {@code eim.exe} when present in this archive, otherwise a versioned | ||
| * {@code eim_v*} binary from this archive). | ||
| */ | ||
| public static Path extract(Path zipPath, Path destDir) throws IOException | ||
| { | ||
| Files.createDirectories(destDir); | ||
| Set<Path> extractedFiles = new LinkedHashSet<>(); | ||
| Path firstRegularFile = null; | ||
|
|
||
| try (InputStream fileIn = Files.newInputStream(zipPath); ZipInputStream zis = new ZipInputStream(fileIn)) | ||
| { | ||
| ZipEntry entry; | ||
| while ((entry = zis.getNextEntry()) != null) | ||
| { | ||
| Path newPath = destDir.resolve(entry.getName()).normalize(); | ||
| if (!newPath.startsWith(destDir.normalize())) | ||
| { | ||
| throw new IOException("ZIP entry is outside target dir: " + entry.getName()); //$NON-NLS-1$ | ||
| } | ||
|
|
||
| if (entry.isDirectory()) | ||
| { | ||
| Files.createDirectories(newPath); | ||
| continue; | ||
| } | ||
|
|
||
| Files.createDirectories(newPath.getParent()); | ||
| Files.copy(zis, newPath, StandardCopyOption.REPLACE_EXISTING); | ||
| extractedFiles.add(newPath); | ||
| if (shouldMarkExecutable(newPath)) | ||
| { | ||
| newPath.toFile().setExecutable(true); | ||
| } | ||
| if (firstRegularFile == null) | ||
| { | ||
| firstRegularFile = newPath; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| repairMaterializedEimSymlink(destDir, extractedFiles); | ||
| return resolvePreferredLaunchPath(destDir, extractedFiles, firstRegularFile); | ||
| } | ||
|
|
||
| /** | ||
| * If {@code eim} was extracted from this archive as a tiny text file whose content names an | ||
| * existing sibling binary (the usual result of extracting a ZIP symlink with | ||
| * {@link ZipInputStream}), replace it with a real symlink or a copy of that target. | ||
| */ | ||
| public static void repairMaterializedEimSymlink(Path destDir) throws IOException | ||
| { | ||
| Path eim = destDir.resolve("eim").normalize(); //$NON-NLS-1$ | ||
| repairMaterializedEimSymlink(destDir, Set.of(eim)); | ||
| } | ||
|
|
||
| static void repairMaterializedEimSymlink(Path destDir, Collection<Path> extractedFromArchive) throws IOException | ||
| { | ||
| Path eim = destDir.resolve("eim").normalize(); //$NON-NLS-1$ | ||
| if (!extractedFromArchive.contains(eim)) | ||
| { | ||
| return; | ||
| } | ||
|
Comment on lines
+108
to
+114
There was a problem hiding this comment. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win Require the symlink target to come from the current archive.
Require 🤖 Prompt for AI Agents |
||
| if (!Files.isRegularFile(eim) || Files.isSymbolicLink(eim)) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| long size = Files.size(eim); | ||
| if (size == 0 || size > MAX_SYMLINK_PAYLOAD_BYTES) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| byte[] bytes = Files.readAllBytes(eim); | ||
| if (containsNullByte(bytes)) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| String target = new String(bytes, StandardCharsets.UTF_8).trim(); | ||
| if (!isSafeSymlinkTargetName(target)) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| Path targetPath = destDir.resolve(target).normalize(); | ||
| if (!targetPath.startsWith(destDir.normalize()) || !Files.isRegularFile(targetPath) | ||
| || Files.isSameFile(eim, targetPath)) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| if (!looksLikeEimBinaryName(targetPath.getFileName().toString())) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| Files.delete(eim); | ||
| try | ||
| { | ||
| Files.createSymbolicLink(eim, Path.of(target)); | ||
| Logger.log("Restored EIM symlink " + eim + " -> " + target); //$NON-NLS-1$ //$NON-NLS-2$ | ||
| } | ||
| catch (UnsupportedOperationException | IOException e) | ||
| { | ||
| Logger.log("Could not create EIM symlink; copying target instead: " + e.getMessage()); //$NON-NLS-1$ | ||
| Files.copy(targetPath, eim, StandardCopyOption.REPLACE_EXISTING); | ||
| eim.toFile().setExecutable(true); | ||
| } | ||
| } | ||
|
|
||
| public static boolean looksLikeMaterializedSymlinkPayload(Path file, Path destDir) throws IOException | ||
| { | ||
| if (!Files.isRegularFile(file) || Files.isSymbolicLink(file)) | ||
| { | ||
| return false; | ||
| } | ||
| long size = Files.size(file); | ||
| if (size == 0 || size > MAX_SYMLINK_PAYLOAD_BYTES) | ||
| { | ||
| return false; | ||
| } | ||
| byte[] bytes = Files.readAllBytes(file); | ||
| if (containsNullByte(bytes)) | ||
| { | ||
| return false; | ||
| } | ||
| String target = new String(bytes, StandardCharsets.UTF_8).trim(); | ||
| if (!isSafeSymlinkTargetName(target)) | ||
| { | ||
| return false; | ||
| } | ||
| Path targetPath = destDir.resolve(target).normalize(); | ||
| return targetPath.startsWith(destDir.normalize()) && Files.isRegularFile(targetPath) | ||
| && looksLikeEimBinaryName(targetPath.getFileName().toString()); | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| } | ||
|
|
||
| /** | ||
| * Shared validation for ZIP-symlink payloads: reject empty names, path separators, parent | ||
| * references, and stray newlines so detection and repair stay aligned. | ||
| */ | ||
| 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$ | ||
| } | ||
|
Comment on lines
+176
to
+180
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:
💡 Result: The Citations:
🏁 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:
💡 Result: The Citations:
🌐 Web query:
💡 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:
🏁 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.
🤖 Prompt for AI Agents |
||
|
|
||
| private static boolean shouldMarkExecutable(Path extractedFile) | ||
| { | ||
| if (Platform.OS_WIN32.equals(Platform.getOS())) | ||
| { | ||
| return false; | ||
| } | ||
| return looksLikeEimBinaryName(extractedFile.getFileName().toString()); | ||
| } | ||
|
|
||
| private static boolean looksLikeEimBinaryName(String name) | ||
| { | ||
| String lower = name.toLowerCase(); | ||
| return lower.equals("eim") || lower.startsWith("eim_v") || lower.startsWith("eim-"); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ | ||
| } | ||
|
|
||
| private static boolean containsNullByte(byte[] bytes) | ||
| { | ||
| for (byte b : bytes) | ||
| { | ||
| if (b == 0) | ||
| { | ||
| return true; | ||
| } | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| private static Path resolvePreferredLaunchPath(Path destDir, Collection<Path> extractedFromArchive, | ||
| Path firstRegularFile) | ||
| { | ||
| Path stableEim = destDir.resolve("eim").normalize(); //$NON-NLS-1$ | ||
| if (extractedFromArchive.contains(stableEim) && Files.exists(stableEim)) | ||
| { | ||
| return stableEim; | ||
| } | ||
|
|
||
| Path stableEimExe = destDir.resolve("eim.exe").normalize(); //$NON-NLS-1$ | ||
| if (extractedFromArchive.contains(stableEimExe) && Files.exists(stableEimExe)) | ||
| { | ||
| return stableEimExe; | ||
| } | ||
|
|
||
| Optional<Path> versioned = findVersionedEimBinary(extractedFromArchive); | ||
| if (versioned.isPresent()) | ||
| { | ||
| return versioned.get(); | ||
| } | ||
|
|
||
| return firstRegularFile != null ? firstRegularFile : destDir; | ||
| } | ||
|
|
||
| private static Optional<Path> findVersionedEimBinary(Collection<Path> extractedFromArchive) | ||
| { | ||
| return extractedFromArchive.stream().filter(Files::isRegularFile) | ||
| .filter(p -> p.getFileName().toString().matches("(?i)eim_v.+")) //$NON-NLS-1$ | ||
| .sorted(Comparator.comparing(p -> p.getFileName().toString())).findFirst(); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Reject existing symlink components during extraction.
The lexical check does not stop
Files.createDirectoriesandFiles.copyfrom following an existing directory symlink belowdestDir. IfdestDir/pluginslinks outsidedestDir, an archive entry namedplugins/filepasses 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