-
Notifications
You must be signed in to change notification settings - Fork 0
Enhance font management with Google Fonts integration, automatic clea… #1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
bcd622c
0da2741
a2f8cf9
565f759
a0338f9
6a33096
9fe20df
2e3d671
d73f490
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -3,48 +3,215 @@ | |||||||||||||||||||||
| import uuid | ||||||||||||||||||||||
| import atexit | ||||||||||||||||||||||
| import signal | ||||||||||||||||||||||
| from flask import Flask, render_template, request, send_file, after_this_request | ||||||||||||||||||||||
| from video_engine import generate_video_from_web # Import from video_engine.py | ||||||||||||||||||||||
| import requests | ||||||||||||||||||||||
| import json | ||||||||||||||||||||||
| import tempfile | ||||||||||||||||||||||
| from flask import Flask, render_template, request, send_file, after_this_request, jsonify | ||||||||||||||||||||||
| from video_engine import generate_video_from_web, install_google_font # Import from video_engine.py | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| app = Flask(__name__) | ||||||||||||||||||||||
| app.config['UPLOAD_FOLDER'] = 'temp_uploads' | ||||||||||||||||||||||
| app.config['OUTPUT_FOLDER'] = 'temp_outputs' | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # Create temporary folders if they don't exist | ||||||||||||||||||||||
| os.makedirs(app.config['UPLOAD_FOLDER'], exist_ok=True) | ||||||||||||||||||||||
| os.makedirs(app.config['OUTPUT_FOLDER'], exist_ok=True) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| def cleanup_temp_folders(): | ||||||||||||||||||||||
| """Clean up all temporary folders when server shuts down""" | ||||||||||||||||||||||
| print("\n🧹 Cleaning up temporary folders...") | ||||||||||||||||||||||
| """Clean up all temporary folders and cache when server shuts down""" | ||||||||||||||||||||||
| print("\n🧹 Cleaning up temporary files and caches...") | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| folders_to_remove = [ | ||||||||||||||||||||||
| app.config['UPLOAD_FOLDER'], | ||||||||||||||||||||||
| app.config['OUTPUT_FOLDER'], | ||||||||||||||||||||||
| '__pycache__', | ||||||||||||||||||||||
| '.pytest_cache', | ||||||||||||||||||||||
| ] | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| try: | ||||||||||||||||||||||
| if os.path.exists(app.config['UPLOAD_FOLDER']): | ||||||||||||||||||||||
| shutil.rmtree(app.config['UPLOAD_FOLDER']) | ||||||||||||||||||||||
| print(f" ✓ Removed {app.config['UPLOAD_FOLDER']}") | ||||||||||||||||||||||
| if os.path.exists(app.config['OUTPUT_FOLDER']): | ||||||||||||||||||||||
| shutil.rmtree(app.config['OUTPUT_FOLDER']) | ||||||||||||||||||||||
| print(f" ✓ Removed {app.config['OUTPUT_FOLDER']}") | ||||||||||||||||||||||
| print("✓ Cleanup complete!") | ||||||||||||||||||||||
| # Remove temp folders | ||||||||||||||||||||||
| for folder in folders_to_remove: | ||||||||||||||||||||||
| if os.path.exists(folder): | ||||||||||||||||||||||
| if os.path.isdir(folder): | ||||||||||||||||||||||
| shutil.rmtree(folder) | ||||||||||||||||||||||
| print(f" ✓ Removed {folder}/") | ||||||||||||||||||||||
| else: | ||||||||||||||||||||||
| os.remove(folder) | ||||||||||||||||||||||
| print(f" ✓ Removed {folder}") | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # Remove .pyc files recursively | ||||||||||||||||||||||
| for root, dirs, files in os.walk('.'): | ||||||||||||||||||||||
| for file in files: | ||||||||||||||||||||||
| if file.endswith('.pyc') or file.endswith('.pyo'): | ||||||||||||||||||||||
| filepath = os.path.join(root, file) | ||||||||||||||||||||||
| try: | ||||||||||||||||||||||
| os.remove(filepath) | ||||||||||||||||||||||
| except: | ||||||||||||||||||||||
| pass | ||||||||||||||||||||||
| print(" ✓ Removed .pyc and .pyo files") | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # Remove moviepy temp files | ||||||||||||||||||||||
| temp_dir = tempfile.gettempdir() | ||||||||||||||||||||||
| for file in os.listdir(temp_dir): | ||||||||||||||||||||||
| if 'mpy' in file.lower(): | ||||||||||||||||||||||
| try: | ||||||||||||||||||||||
| filepath = os.path.join(temp_dir, file) | ||||||||||||||||||||||
| if os.path.isfile(filepath): | ||||||||||||||||||||||
| os.remove(filepath) | ||||||||||||||||||||||
|
Comment on lines
+54
to
+59
|
||||||||||||||||||||||
| if 'mpy' in file.lower(): | |
| try: | |
| filepath = os.path.join(temp_dir, file) | |
| if os.path.isfile(filepath): | |
| os.remove(filepath) | |
| filepath = os.path.join(temp_dir, file) | |
| # Only remove files that match MoviePy's known temp filename prefix | |
| if os.path.isfile(filepath) and file.lower().startswith('moviepy_'): | |
| try: | |
| os.remove(filepath) |
Copilot
AI
Jan 18, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The bare except clauses silently catch all exceptions including KeyboardInterrupt and SystemExit. This can make debugging difficult and could hide serious errors. Replace with specific exception types such as 'except OSError:' or at minimum 'except Exception:'.
Copilot
AI
Jan 19, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bare except: clauses at lines 48 and 60 silently swallow all exceptions including KeyboardInterrupt and SystemExit. This makes debugging difficult and can mask serious errors. Consider catching specific exceptions like OSError or PermissionError instead, or at minimum use except Exception: to avoid catching system-level exceptions.
Copilot
AI
Jan 18, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Google Fonts metadata endpoint is called on every search request without any caching. This could cause performance issues and rate limiting. Consider implementing a caching mechanism (e.g., cache the font list for a reasonable duration like 24 hours) to reduce external API calls and improve response times.
Copilot
AI
Jan 19, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The font search endpoint lacks error handling for network issues. If requests.get() raises an exception (lines 94-101), it returns a 502 status with a generic error message. However, the function doesn't validate the JSON response structure before accessing nested keys. If Google changes their API response format, the code could crash with KeyError or AttributeError. Consider adding validation after parsing the JSON to ensure data has the expected structure.
Copilot
AI
Jan 18, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The font download endpoint returns the absolute local filesystem path to the frontend. This exposes internal server paths to clients. Consider returning just the filename or a relative path instead, and handle path resolution on the server side when processing the font selection.
| return jsonify({'success': True, 'path': path, 'name': name}) | |
| # Return a relative path instead of an absolute filesystem path | |
| relative_path = os.path.relpath(path, start=os.getcwd()) | |
| return jsonify({'success': True, 'path': relative_path, 'name': name}) |
Copilot
AI
Jan 18, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The uploaded filenames are used directly to save files without sanitization. This could allow directory traversal attacks if a malicious filename like '../../sensitive.mp3' is uploaded. Sanitize filenames using a function like werkzeug.utils.secure_filename() before saving.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot open a new pull request to apply changes based on this feedback
Copilot
AI
Jan 19, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The cleanup logic in the finally block (lines 169-173) uses a bare except Exception: that silently ignores all cleanup failures. While this prevents the endpoint from crashing, it could lead to accumulation of temporary files if cleanup consistently fails. Consider logging the exception to help diagnose persistent cleanup issues.
| except Exception: | |
| pass | |
| except Exception as cleanup_error: | |
| print(f"Cleanup error for {session_upload_path}: {cleanup_error}") |
Copilot
AI
Jan 19, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The path traversal validation logic has an issue. On Windows, os.path.sep is typically \, so the check at line 194 if not preview_path.startswith(output_folder + os.path.sep) would fail for the last file in the directory if output_folder doesn't end with a separator. A safer approach is to use os.path.commonpath() or ensure both paths are normalized with trailing separators. Additionally, os.path.altsep is None on many platforms, so the check if os.path.altsep and os.path.altsep in filename could be simplified.
Copilot
AI
Jan 19, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The /preview endpoint (lines 126-173) and /get_preview/<filename> endpoint (lines 175-198) are defined but don't appear to be called from the frontend HTML. Additionally, these endpoints call generate_video_from_web instead of the new generate_video_preview function that was added to video_engine.py. This suggests either: 1) the endpoints are incomplete/untested, 2) they're intended for future use but shouldn't be in this release, or 3) the frontend integration is missing. Consider either completing the feature integration or removing these endpoints until they're fully implemented.
Copilot
AI
Jan 19, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to the cleanup handler, this code uses a bare except: clause at line 210 that catches all exceptions including system-level ones. Use except Exception: or specific exception types like OSError instead.
| except: | |
| except OSError: |
Copilot
AI
Jan 18, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The bare except clause silently catches all exceptions. Replace with specific exception types such as 'except OSError:' or at minimum 'except Exception:'.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,4 +6,5 @@ numpy>=1.24.0 | |
| tqdm>=4.66.0 | ||
| librosa>=0.10.0 | ||
| pillow-heif>=0.13.0 | ||
| requests>=2.31.0 | ||
| setuptools>=65.0.0 | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The bare except clause silently catches all exceptions including KeyboardInterrupt and SystemExit. Replace with specific exception types such as 'except OSError:' or at minimum 'except Exception:'.