Create target directories before copying file

[MohabMohie]

User description

Fixes #16738

🔄 Types of changes

• Bug fix (backwards compatible)

---

PR Type

Bug fix

---

Description

• Ensure parent directories exist before file download

• Prevents IOException when target directory missing

---

Diagram Walkthrough

flowchart LR
  A["downloadFile method"] --> B["Files.createDirectories"]
  B --> C["Files.copy"]
  C --> D["File saved successfully"]

Loading

File Walkthrough

	Relevant files
Bug fix	RemoteWebDriver.java Add directory creation before file download                            java/src/org/openqa/selenium/remote/RemoteWebDriver.java Added Files.createDirectories(targetLocation) call before file copy operation Ensures parent directories exist to prevent IOException when downloading files to non-existent paths Maintains backwards compatibility with existing code +1/-0

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	Path traversal Description: The method uses unvalidated path components (targetLocation and fileName) to create directories and write a file (Files.createDirectories(targetLocation) and targetLocation.resolve(fileName)), which can enable path traversal or arbitrary file write if either value can be influenced to include .. segments or an absolute path (e.g., fileName like ../../.ssh/authorized_keys). RemoteWebDriver.java [730-739] Referred Code public void downloadFile(String fileName, Path targetLocation) throws IOException { requireDownloadsEnabled(capabilities); Response response = execute(DriverCommand.GET_DOWNLOADED_FILE, Map.of("name", fileName)); Contents.Supplier content = (Contents.Supplier) response.getValue(); try (InputStream fileContent = content.get()) { Files.createDirectories(targetLocation); Files.copy(new BufferedInputStream(fileContent), targetLocation.resolve(fileName)); }
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: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation 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: 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: 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: No audit logging: The new file-system action (creating directories before saving a downloaded file) is not accompanied by any audit log context (actor, action, outcome), which may be required depending on whether downloads are considered a critical action in this project. Referred Code Files.createDirectories(targetLocation); Files.copy(new BufferedInputStream(fileContent), targetLocation.resolve(fileName)); 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

[MohabMohie]

@diemol @asolntsev for your review kindly.

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Create parent directory of destination file Instead of creating the targetLocation as a directory, create the parent directory of the final destination path (targetLocation.resolve(fileName)). This makes the method more robust if targetLocation is a file path instead of a directory. java/src/org/openqa/selenium/remote/RemoteWebDriver.java [737-738] -Files.createDirectories(targetLocation); -Files.copy(new BufferedInputStream(fileContent), targetLocation.resolve(fileName)); +Path destination = targetLocation.resolve(fileName); +Files.createDirectories(destination.getParent()); +Files.copy(new BufferedInputStream(fileContent), destination); Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that if targetLocation is a path to a file, the PR's change will incorrectly create it as a directory. The proposed change to create the parent directory of the resolved path is more robust and handles this edge case correctly, improving the code's reliability.	Medium
Learned best practice	Validate file paths before copying Defensively validate fileName and ensure the resolved destination stays within targetLocation to prevent ../ traversal or absolute-path writes; then create directories and copy to the validated path. java/src/org/openqa/selenium/remote/RemoteWebDriver.java [730-740] public void downloadFile(String fileName, Path targetLocation) throws IOException { requireDownloadsEnabled(capabilities); Response response = execute(DriverCommand.GET_DOWNLOADED_FILE, Map.of("name", fileName)); + Path baseDir = Require.nonNull("Target location", targetLocation).toAbsolutePath().normalize(); + String safeName = Require.nonNull("File name", fileName).trim(); + if (safeName.isEmpty()) { + throw new IllegalArgumentException("File name must not be blank"); + } + + Path destination = baseDir.resolve(safeName).normalize(); + if (!destination.startsWith(baseDir) || destination.getFileName() == null) { + throw new IllegalArgumentException("Invalid file name: " + fileName); + } + Contents.Supplier content = (Contents.Supplier) response.getValue(); try (InputStream fileContent = content.get()) { - Files.createDirectories(targetLocation); - Files.copy(new BufferedInputStream(fileContent), targetLocation.resolve(fileName)); + Files.createDirectories(baseDir); + Files.copy(new BufferedInputStream(fileContent), destination); } } [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Validate and sanitize external inputs (e.g., names/paths) before using them in filesystem operations to avoid path traversal and invalid paths.	Low
More

[asolntsev]

Hi. As I said in the ticket, I think that the end user is responsible for creating targetLocation, not Selenium.

Selenium could just throw an exception like "I cannot download the file because
targetLocation is not found ".

[asolntsev]

Decided to merge it because this argumentation was strong: #16738 (comment) :)