Show file modification time

[asolntsev]

User description

🔗 Related Issues

Improves #9218

💥 What does this PR do?

This PR adds meta-info about the downloaded files to /se/files response:

1. File size
2. File last modification time

Previously, the response contained only file names. This is not enough to determine if the download is fully completed.

🔧 Implementation Notes

1. For backward compatibility, I left the previous response pieces as is (list of file names).
2. I am not sure where to put class DownloadedFile. Currently I added it as internal class to interface HasDownloads, but I am open for suggestions. :)

🔄 Types of changes

• New feature (non-breaking change which adds functionality and tests!)

---

PR Type

Enhancement

---

Description

• Add DownloadedFile class to expose file metadata (name, size, creation/modification time)

• Implement getDownloadedFiles() method returning list of DownloadedFile objects

• Refactor LocalNode.downloadFile() into three focused methods for GET/POST/DELETE operations

• Replace ImmutableMap.of() with Map.of() for Java 9+ compatibility

---

Diagram Walkthrough

flowchart LR
  A["HasDownloads Interface"] -->|adds| B["DownloadedFile Class"]
  B -->|contains| C["name, size, creation/modification time"]
  D["LocalNode.downloadFile()"] -->|refactored into| E["listDownloadedFiles()"]
  D -->|refactored into| F["getDownloadedFile()"]
  D -->|refactored into| G["deleteDownloadedFile()"]
  E -->|uses| B
  F -->|uses| B
  H["RemoteWebDriver"] -->|implements| I["getDownloadedFiles()"]
  I -->|returns| B

Loading

File Walkthrough

Relevant files

Enhancement

HasDownloads.java Add DownloadedFile class and metadata method                          java/src/org/openqa/selenium/HasDownloads.java Add new getDownloadedFiles() method to interface Create inner DownloadedFile class with name, creation time, last modified time, and size properties Include getter methods for all file metadata fields +37/-0    LocalNode.java Refactor downloads endpoint and add file metadata extraction java/src/org/openqa/selenium/grid/node/local/LocalNode.java Refactor monolithic downloadFile() method into three separate methods: listDownloadedFiles(), getDownloadedFile(), and deleteDownloadedFile() Add getFileInfo() helper method to extract file metadata using BasicFileAttributes Replace all ImmutableMap.of() calls with Map.of() for Java 9+ compatibility Update GET response to include both file names and DownloadedFile objects with metadata Add file metadata to POST response for individual file downloads +90/-52  RemoteWebDriver.java Implement getDownloadedFiles() client method                          java/src/org/openqa/selenium/remote/RemoteWebDriver.java Implement new getDownloadedFiles() method returning List Parse file metadata from server response and construct DownloadedFile objects Update getDownloadableFiles() to handle new response structure with metadata Update JavaDoc to clarify distinction between downloadable and downloaded files +24/-4

---

[qodo-merge-pro]

PR Compliance Guide 🔍

(Compliance updated until commit c5eab0f)

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	Insecure path handling Description: The code assumes the first entry of TemporaryFilesystem base dir exists and is the downloads directory without validating the array is non-empty or that the entry is a directory, which can lead to null pointer or path traversal issues if the filesystem layout is unexpected. LocalNode.java [742-754] Referred Code Optional.ofNullable(tempFS.getBaseDir().listFiles()).orElse(new File[] {})[0]; try { if (req.getMethod().equals(HttpMethod.GET)) { return listDownloadedFiles(downloadsDirectory); } if (req.getMethod().equals(HttpMethod.DELETE)) { return deleteDownloadedFile(downloadsDirectory); } return getDownloadedFile(req, downloadsDirectory); } catch (IOException e) { throw new UncheckedIOException(e); }
	Unsafe file deletion Description: Deleting all files in the downloads directory without validating file paths or restricting to regular files could allow unintended deletions if the directory contains symlinks pointing outside the intended location. LocalNode.java [825-832] Referred Code File[] files = Optional.ofNullable(downloadsDirectory.listFiles()).orElse(new File[] {}); for (File file : files) { FileHandler.delete(file); } Map<String, Object> toReturn = new HashMap<>(); toReturn.put("value", null); return new HttpResponse().setContent(asJson(toReturn)); }
Ticket Compliance
🟡	🎫 #9218 🔴 Allow retrieving downloaded files from browser containers to a host-accessible path, similar to how recorded videos are retrieved by mounting a host path.
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 Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. 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 auditing: New file listing/downloading/deleting endpoints are added but no explicit audit logging of who performed the action, when, and the outcome is evident in the new code. Referred Code String msg = "Cannot find downloads file system for session id: " + id + " — ensure downloads are enabled in the options class when requesting a session."; throw new WebDriverException(msg); } File downloadsDirectory = Optional.ofNullable(tempFS.getBaseDir().listFiles()).orElse(new File[] {})[0]; try { if (req.getMethod().equals(HttpMethod.GET)) { return listDownloadedFiles(downloadsDirectory); } if (req.getMethod().equals(HttpMethod.DELETE)) { return deleteDownloadedFile(downloadsDirectory); } return getDownloadedFile(req, downloadsDirectory); } catch (IOException e) { throw new UncheckedIOException(e); } } ... (clipped 78 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: Error context: While exceptions are thrown for missing/ambiguous files and IO errors are wrapped, the GET/DELETE/ZIP flows add little contextual logging and may not handle empty directory or concurrent file changes robustly. Referred Code try { if (req.getMethod().equals(HttpMethod.GET)) { return listDownloadedFiles(downloadsDirectory); } if (req.getMethod().equals(HttpMethod.DELETE)) { return deleteDownloadedFile(downloadsDirectory); } return getDownloadedFile(req, downloadsDirectory); } catch (IOException e) { throw new UncheckedIOException(e); } } /** User wants to list files that can be downloaded */ 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).collect(Collectors.toList()); Map<String, Object> data = ... (clipped 68 lines) 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: Verbose errors: User-facing WebDriverException messages include absolute directory paths which may reveal internal file system details. Referred Code if (allFiles.length == 0) { throw new WebDriverException( String.format( "Cannot find file [%s] in directory %s.", filename, downloadsDirectory.getAbsolutePath())); } 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: Input validation: The file name from the request is used to match files with minimal validation and absolute paths returned in errors; while filtered by exact name, additional normalization/validation and directory traversal safeguards are not clearly enforced in the new code. Referred Code throws IOException { String raw = string(req); if (raw.isEmpty()) { throw new WebDriverException( "Please specify file to download in payload as {\"name\": \"fileToDownload\"}"); } Map<String, Object> incoming = JSON.toType(raw, Json.MAP_TYPE); String filename = Optional.ofNullable(incoming.get("name")) .map(Object::toString) .orElseThrow( () -> new WebDriverException( "Please specify file to download in payload as {\"name\":" + " \"fileToDownload\"}")); File[] allFiles = Optional.ofNullable(downloadsDirectory.listFiles((dir, name) -> name.equals(filename))) .orElse(new File[] {}); if (allFiles.length == 0) { throw new WebDriverException( String.format( ... (clipped 15 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

---

Previous compliance checks

Compliance check up to commit 55e8fcf

Security Compliance
⚪	Information disclosure Description: The method getFileInfo wraps IOExceptions when reading file attributes into a RuntimeException, which can leak absolute file paths in error messages to clients, potentially disclosing internal filesystem structure. LocalNode.java [773-783] Referred Code try { BasicFileAttributes attributes = readAttributes(file.toPath(), BasicFileAttributes.class); return new DownloadedFile( file.getName(), attributes.creationTime().toMillis(), attributes.lastModifiedTime().toMillis(), attributes.size()); } catch (IOException e) { throw new RuntimeException("Failed to get file attributes: " + file.getAbsolutePath(), e); } }
Ticket Compliance
🟡	🎫 #9218 🔴 Provide support in Selenium 4 Dynamic Grid to access/download files saved in the browser container (e.g., /home/seluser/Downloads) to a host-accessible path, similar to how video assets are exposed via a mounted path.
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 Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. 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: New endpoints for listing, downloading, and deleting downloaded files perform sensitive actions without emitting audit logs that include user/session context, timestamps, and outcomes. Referred Code try { if (req.getMethod().equals(HttpMethod.GET)) { return listDownloadedFiles(downloadsDirectory); } if (req.getMethod().equals(HttpMethod.DELETE)) { return deleteDownloadedFile(downloadsDirectory); } return getDownloadedFile(req, downloadsDirectory); } catch (IOException e) { throw new UncheckedIOException(e); } } /** User wants to list files that can be downloaded */ 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).collect(Collectors.toList()); Map<String, Object> data = ... (clipped 69 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: Unhandled edge cases: File attribute retrieval wraps IOException in RuntimeException and listFiles()[0] assumes a directory exists, which may cause ungraceful failures without explicit handling of empty directories or permission issues. Referred Code Optional.ofNullable(tempFS.getBaseDir().listFiles()).orElse(new File[] {})[0]; try { if (req.getMethod().equals(HttpMethod.GET)) { return listDownloadedFiles(downloadsDirectory); } if (req.getMethod().equals(HttpMethod.DELETE)) { return deleteDownloadedFile(downloadsDirectory); } return getDownloadedFile(req, downloadsDirectory); } catch (IOException e) { throw new UncheckedIOException(e); } } /** User wants to list files that can be downloaded */ 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).collect(Collectors.toList()); ... (clipped 21 lines) 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: Detailed error paths: Error messages include absolute file system paths (e.g., in RuntimeException and WebDriverException) that could expose internal details if surfaced to end users. Referred Code return new DownloadedFile( file.getName(), attributes.creationTime().toMillis(), attributes.lastModifiedTime().toMillis(), attributes.size()); } catch (IOException e) { throw new RuntimeException("Failed to get file attributes: " + file.getAbsolutePath(), e); } } private HttpResponse getDownloadedFile(HttpRequest req, File downloadsDirectory) throws IOException { String raw = string(req); if (raw.isEmpty()) { throw new WebDriverException( "Please specify file to download in payload as {\"name\": \"fileToDownload\"}"); } Map<String, Object> incoming = JSON.toType(raw, Json.MAP_TYPE); String filename = Optional.ofNullable(incoming.get("name")) .map(Object::toString) ... (clipped 17 lines) 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: Input validation gaps: The download endpoint trusts the provided filename for matching without normalization or checks for traversal or unusual characters, and DELETE deletes all files without explicit authorization checks visible in the diff. Referred Code String filename = Optional.ofNullable(incoming.get("name")) .map(Object::toString) .orElseThrow( () -> new WebDriverException( "Please specify file to download in payload as {\"name\":" + " \"fileToDownload\"}")); File[] allFiles = Optional.ofNullable(downloadsDirectory.listFiles((dir, name) -> name.equals(filename))) .orElse(new File[] {}); if (allFiles.length == 0) { throw new WebDriverException( String.format( "Cannot find file [%s] in directory %s.", filename, downloadsDirectory.getAbsolutePath())); } if (allFiles.length != 1) { throw new WebDriverException( String.format("Expected there to be only 1 file. There were: %s.", allFiles.length)); } ... (clipped 20 lines) Learn more about managing compliance generic rules or creating your own custom rules

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	Consolidate redundant file download APIs The new getDownloadedFiles() method is a superset of the existing getDownloadableFiles(). To avoid API redundancy, the suggestion is to deprecate the old method in favor of the new one. Examples: java/src/org/openqa/selenium/HasDownloads.java [58-65] List<String> getDownloadableFiles(); /** * Gets all files downloaded by browser. * * @return a list of files with their name, size and time. */ List<DownloadedFile> getDownloadedFiles(); java/src/org/openqa/selenium/remote/RemoteWebDriver.java [683-708] public List<String> getDownloadableFiles() { requireDownloadsEnabled(capabilities); Response response = execute(DriverCommand.GET_DOWNLOADABLE_FILES); Map<String, Object> value = (Map<String, Object>) response.getValue(); return (List<String>) value.get("names"); } @Override @SuppressWarnings("unchecked") ... (clipped 16 lines) Solution Walkthrough: Before: // In interface HasDownloads public interface HasDownloads { // ... /** * @return a list of downloadable files for each key */ List<String> getDownloadableFiles(); /** * @return a list of files with their name, size and time. */ List<DownloadedFile> getDownloadedFiles(); // ... } After: // In interface HasDownloads public interface HasDownloads { // ... /** * @return a list of downloadable files for each key * @deprecated Use {@link #getDownloadedFiles()} instead. */ @Deprecated List<String> getDownloadableFiles(); /** * @return a list of files with their name, size and time. */ List<DownloadedFile> getDownloadedFiles(); // ... } Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies API redundancy and proposes deprecating the older getDownloadableFiles method, which is a standard practice for evolving an API without breaking backward compatibility, thus improving clarity and maintainability.	Medium
Possible issue	Use consistent exception type for errors In getFileInfo, throw UncheckedIOException instead of RuntimeException to maintain consistent exception handling with the calling downloadFile method. java/src/org/openqa/selenium/grid/node/local/LocalNode.java [772-783] 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 (IOException e) { - throw new RuntimeException("Failed to get file attributes: " + file.getAbsolutePath(), e); + throw new UncheckedIOException("Failed to get file attributes: " + file.getAbsolutePath(), e); } } Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly points out an inconsistency in exception handling and proposes changing RuntimeException to UncheckedIOException to align with the caller's error handling logic.	Medium
	Add null check to prevent crash Add a null check for the files list in getDownloadedFiles to prevent a NullPointerException if the response map does not contain the "files" key, returning an empty list instead. java/src/org/openqa/selenium/remote/RemoteWebDriver.java [698-707] List<Map<String, Object>> files = (List<Map<String, Object>>) value.get("files"); +if (files == null) { + return Collections.emptyList(); +} return files.stream() .map( file -> new DownloadedFile( (String) file.get("name"), (Long) file.get("creationTime"), (Long) file.get("lastModifiedTime"), (Long) file.get("size"))) .collect(Collectors.toUnmodifiableList()); Apply / Chat Suggestion importance[1-10]: 5 __ Why: The suggestion correctly identifies a potential NullPointerException and proposes a reasonable defensive null check, which improves the code's robustness against unexpected API responses.	Low
Update