#16720 avoid failing because of temporary Chrome internal files

[asolntsev]

User description

... that Chrome downloads and immediately deletes.

🔗 Related Issues

Fixes #16720

🔄 Types of changes

• Bug fix (backwards compatible)

---

PR Type

Bug fix

---

Description

• Handle temporary Chrome files deleted during download listing

• Catch NoSuchFileException when file attributes cannot be read

• Filter out files with invalid modification times from results

---

Diagram Walkthrough

flowchart LR
  A["listDownloadedFiles"] --> B["stream files"]
  B --> C["getFileInfo"]
  C --> D["readAttributes"]
  D --> E{"File exists?"}
  E -->|Yes| F["return DownloadedFile"]
  E -->|No| G["catch NoSuchFileException"]
  G --> H["return DownloadedFile with -1 values"]
  F --> I["filter by lastModifiedTime > 0"]
  H --> I
  I --> J["collect results"]

Loading

File Walkthrough

Relevant files

Bug fix

LocalNode.java Handle temporary Chrome files in download listing                java/src/org/openqa/selenium/grid/node/local/LocalNode.java Added import for NoSuchFileException to handle missing files Modified getFileInfo() method to catch NoSuchFileException and return a DownloadedFile with -1 sentinel values Added filter in listDownloadedFiles() to exclude files with invalid modification times (value of -1) Prevents failures when Chrome temporarily downloads and deletes internal files during listing +7/-1

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Missing audit logs: The new handling for missing files and filtering does not emit any audit/log entry for these events, which may be considered critical for traceability of download processing. Referred Code private HttpResponse listDownloadedFiles(File downloadsDirectory) { File[] files = Optional.ofNullable(downloadsDirectory.listFiles()).orElse(new File[] {}); List<String> fileNames = Arrays.stream(files).map(File::getName).collect(Collectors.toList()); List<DownloadedFile> fileInfos = Arrays.stream(files) .map(this::getFileInfo) .filter(file -> file.getLastModifiedTime() > 0) .collect(Collectors.toList()); Map<String, Object> data = Map.of( "names", fileNames, "files", fileInfos); Map<String, Map<String, Object>> result = Map.of("value", data); return new HttpResponse().setContent(asJson(result)); } private DownloadedFile getFileInfo(File file) { try { BasicFileAttributes attributes = readAttributes(file.toPath(), BasicFileAttributes.class); return new DownloadedFile( ... (clipped 9 lines) Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Swallowed exception: The code catches NoSuchFileException and returns sentinel values without logging context, potentially hiding frequent or impactful errors from monitoring. Referred Code private DownloadedFile getFileInfo(File file) { try { BasicFileAttributes attributes = readAttributes(file.toPath(), BasicFileAttributes.class); return new DownloadedFile( file.getName(), attributes.creationTime().toMillis(), attributes.lastModifiedTime().toMillis(), attributes.size()); } catch (NoSuchFileException e) { return new DownloadedFile(file.getName(), -1, -1, -1); } catch (IOException e) { Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: No logging added: The new paths for missing files and filtering do not log structured events, reducing observability of file handling without assessing sensitive data exposure. Referred Code private HttpResponse listDownloadedFiles(File downloadsDirectory) { File[] files = Optional.ofNullable(downloadsDirectory.listFiles()).orElse(new File[] {}); List<String> fileNames = Arrays.stream(files).map(File::getName).collect(Collectors.toList()); List<DownloadedFile> fileInfos = Arrays.stream(files) .map(this::getFileInfo) .filter(file -> file.getLastModifiedTime() > 0) .collect(Collectors.toList()); Map<String, Object> data = Map.of( "names", fileNames, "files", fileInfos); Map<String, Map<String, Object>> result = Map.of("value", data); return new HttpResponse().setContent(asJson(result)); } private DownloadedFile getFileInfo(File file) { try { BasicFileAttributes attributes = readAttributes(file.toPath(), BasicFileAttributes.class); return new DownloadedFile( ... (clipped 9 lines) Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Ensure response lists are consistent To prevent inconsistencies in the JSON response, generate the fileNames list from the filtered fileInfos list, ensuring both lists have the same length. java/src/org/openqa/selenium/grid/node/local/LocalNode.java [792-807] private HttpResponse listDownloadedFiles(File downloadsDirectory) { File[] files = Optional.ofNullable(downloadsDirectory.listFiles()).orElse(new File[] {}); - List<String> fileNames = Arrays.stream(files).map(File::getName).collect(Collectors.toList()); List<DownloadedFile> fileInfos = Arrays.stream(files) .map(this::getFileInfo) .filter(file -> file.getLastModifiedTime() > 0) .collect(Collectors.toList()); + List<String> fileNames = fileInfos.stream().map(DownloadedFile::getName).collect(Collectors.toList()); Map<String, Object> data = Map.of( "names", fileNames, "files", fileInfos); Map<String, Map<String, Object>> result = Map.of("value", data); return new HttpResponse().setContent(asJson(result)); } [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a bug where fileNames and fileInfos can have different sizes, leading to an inconsistent JSON response and potential client-side errors.	High
General	Use Optional for cleaner error handling Refactor getFileInfo to return an Optional instead of using sentinel values for missing files, and update listDownloadedFiles to use flatMap for cleaner filtering. java/src/org/openqa/selenium/grid/node/local/LocalNode.java [795-822] List<DownloadedFile> fileInfos = Arrays.stream(files) .map(this::getFileInfo) - .filter(file -> file.getLastModifiedTime() > 0) + .flatMap(Optional::stream) .collect(Collectors.toList()); ... - private DownloadedFile getFileInfo(File file) { + private Optional<DownloadedFile> getFileInfo(File file) { try { BasicFileAttributes attributes = readAttributes(file.toPath(), BasicFileAttributes.class); - return new DownloadedFile( - file.getName(), - attributes.creationTime().toMillis(), - attributes.lastModifiedTime().toMillis(), - attributes.size()); + return Optional.of( + new DownloadedFile( + file.getName(), + attributes.creationTime().toMillis(), + attributes.lastModifiedTime().toMillis(), + attributes.size())); } catch (NoSuchFileException e) { - return new DownloadedFile(file.getName(), -1, -1, -1); + return Optional.empty(); } catch (IOException e) { throw new UncheckedIOException("Failed to get file attributes: " + file.getAbsolutePath(), e); } } [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 6 __ Why: This is a good suggestion for improving code quality by replacing sentinel values with Optional, which is a more robust and idiomatic Java practice for handling potentially absent values.	Low
More