Bug: Attachment upload leaks file handles on partial failure #6

New issue

Closed

opened 2026-05-09 05:18:42 +00:00 by Mark · 1 comment

Mark commented

2026-05-09 05:18:42 +00:00

Owner

Severity: Low-Medium

In vikunjatool/api/client.py (and duplicated in vikunja_cli/api/client.py), the upload_task_attachments method opens files and appends them to a list before entering the try block. If one of the open() calls raises an exception (e.g., file not found), the previously opened file handles in the list are never closed.

def upload_task_attachments(self, task_id, file_paths):
    url = f"{self.api_url}/tasks/{task_id}/attachments"
    files = []
    for path in file_paths:
        files.append(('files', (os.path.basename(path), open(path, 'rb'))))  # <-- opened BEFORE try
    try:
        response = self.session.request('PUT', url, files=files)
        ...
    finally:
        for _, file_tuple in files:
            file_tuple[1].close()  # only closes files that were appended

If file_paths = ["valid1.pdf", "nonexistent.pdf", "valid2.pdf"], opening nonexistent.pdf raises FileNotFoundError, but valid1.pdf's handle is never closed.

Fix: Use a context manager (open() inside the try block) or use with statements. Open files lazily or one by one, ensuring cleanup on any exception path.

**Severity: Low-Medium** In `vikunjatool/api/client.py` (and duplicated in `vikunja_cli/api/client.py`), the `upload_task_attachments` method opens files and appends them to a list before entering the try block. If one of the `open()` calls raises an exception (e.g., file not found), the previously opened file handles in the list are never closed. ```python def upload_task_attachments(self, task_id, file_paths): url = f"{self.api_url}/tasks/{task_id}/attachments" files = [] for path in file_paths: files.append(('files', (os.path.basename(path), open(path, 'rb')))) # <-- opened BEFORE try try: response = self.session.request('PUT', url, files=files) ... finally: for _, file_tuple in files: file_tuple[1].close() # only closes files that were appended ``` If file_paths = ["valid1.pdf", "nonexistent.pdf", "valid2.pdf"], opening nonexistent.pdf raises FileNotFoundError, but valid1.pdf's handle is never closed. **Fix:** Use a context manager (`open()` inside the try block) or use `with` statements. Open files lazily or one by one, ensuring cleanup on any exception path.

Mark commented

2026-05-13 15:59:56 +00:00

Author

Owner

Fixed in commit. Closing.