There is a system that allows users to work with files in the browser. The stack is standard: Java, Spring, React, Postgres. Files are stored in the file system on the backend, file metadata in the database. The team implemented a feature - renaming a file. This method is essentially key for renaming a file on the backend. A review is needed to identify potential issues in the existing code and suggest ways to fix them. @Transactional public void process(String oldName, String newName) { Long id = exec("select id from file where name='" + oldName + "'"); processFile(oldName, newName); // renaming the file on disk exec("update file set name='" + newName + "' where id = " + id); }
sobes.tech AI
Answer from AI
The presented method has several issues:
-
SQL Injection: Using string concatenation to build SQL queries (
"select id from file where name='" + oldName + "'") is vulnerable to SQL injection. -
Lack of error handling: If a file with the name
oldNameis not found, theexecmethod may return null or throw an error, which is not handled. -
Potential desynchronization: Renaming the file on disk and updating the database record are not atomic, despite the
@Transactionalannotation, because the filesystem is not part of the database transaction. -
No check for successful renaming: If
processFilefails, the database will already be updated, leading to desynchronization. -
Unsafe filename update: There is no check to ensure that
newNamedoes not conflict with existing names.
Recommendations for fixing:
- Use parameterized queries or ORM (e.g., JPA) for safe database access.
- Add checks for file existence and error handling.
- Make renaming the file and updating the database atomic using compensation mechanisms or deferred updates.
- Check that the new name is not already taken.
- Log operations and errors.
Example of corrected code using JPA and error handling:
@Transactional
public void process(String oldName, String newName) {
Optional<FileEntity> fileOpt = fileRepository.findByName(oldName);
if (!fileOpt.isPresent()) {
throw new FileNotFoundException("File with name " + oldName + " not found");
}
if (fileRepository.existsByName(newName)) {
throw new IllegalArgumentException("File name " + newName + " already exists");
}
FileEntity file = fileOpt.get();
try {
processFile(oldName, newName); // renaming on disk
} catch (IOException e) {
throw new RuntimeException("Failed to rename file on disk", e);
}
file.setName(newName);
fileRepository.save(file);
}
This approach improves the safety and reliability of the operation.