Sobes.tech
Middle+

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:

  1. SQL Injection: Using string concatenation to build SQL queries ("select id from file where name='" + oldName + "'") is vulnerable to SQL injection.

  2. Lack of error handling: If a file with the name oldName is not found, the exec method may return null or throw an error, which is not handled.

  3. Potential desynchronization: Renaming the file on disk and updating the database record are not atomic, despite the @Transactional annotation, because the filesystem is not part of the database transaction.

  4. No check for successful renaming: If processFile fails, the database will already be updated, leading to desynchronization.

  5. Unsafe filename update: There is no check to ensure that newName does 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.