Skip to content

Commit 14101d8

Browse files
committed
Fix security, threading, and code quality issues
- Fix XSS vulnerability in HTML export by escaping content and title - Fix thread safety and memory leak issues with proper cleanup - Extract magic numbers to named constants for maintainability - Fix type hints for better Python version compatibility - Move module imports to top level (re, datetime) - Add error handling to styling method - Add docstrings to key methods - Remove redundant widget styling (palette + stylesheet)
1 parent 62d6aea commit 14101d8

File tree

2 files changed

+86
-59
lines changed

2 files changed

+86
-59
lines changed

main.py

Lines changed: 70 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
QTextBrowser,
2121
)
2222
from PyQt5.QtCore import Qt, QSize, QTimer, QSettings, QThread, pyqtSignal
23-
from PyQt5.QtGui import QIcon, QKeySequence, QFont, QColor
23+
from PyQt5.QtGui import QIcon, QKeySequence, QFont
2424
from datetime import datetime
2525

2626
# Import our modules
@@ -45,7 +45,13 @@
4545
OutputSyntaxHighlighter,
4646
)
4747

48+
# UI Constants
4849
DEFAULT_FONT = "Menlo" # macOS native monospace font
50+
DEFAULT_FONT_SIZE = 11
51+
DEFAULT_BUTTON_HEIGHT = 40
52+
MAX_RECENT_FILES = 10
53+
STATUS_MESSAGE_TIMEOUT_SHORT = 3000 # milliseconds
54+
STATUS_MESSAGE_TIMEOUT_LONG = 5000 # milliseconds
4955

5056

5157
class TemplateRenderer(QThread):
@@ -90,7 +96,7 @@ def _load_settings(self):
9096

9197
def _save_settings(self):
9298
"""Save application settings."""
93-
self._settings.setValue("recent_files", self._recent_files[:10]) # Keep last 10
99+
self._settings.setValue("recent_files", self._recent_files[:MAX_RECENT_FILES])
94100
self._settings.setValue("window_geometry", self.saveGeometry())
95101
self._settings.setValue("splitter_sizes", self.mainSplitter.saveState())
96102

@@ -127,56 +133,40 @@ def initUI(self):
127133
self.statusBar().showMessage("Ready")
128134

129135
def _create_widgets(self):
130-
"""Create and configure UI widgets."""
136+
"""Create and configure all UI widgets including editors, buttons, and viewers."""
131137
# Template editor with syntax highlighting
132138
self.templateEditor = QTextEdit()
133139
self.templateEditor.setPlaceholderText(
134140
"Enter your Velocity Template (.vm) code here..."
135141
)
136142
self.templateEditor.setAcceptRichText(False)
137-
self.templateEditor.setFont(QFont(DEFAULT_FONT, 11))
143+
self.templateEditor.setFont(QFont(DEFAULT_FONT, DEFAULT_FONT_SIZE))
138144
self.templateHighlighter = VelocitySyntaxHighlighter(
139145
self.templateEditor.document()
140146
)
141-
# Explicit palette for text color
142-
pal = self.templateEditor.palette()
143-
pal.setColor(self.templateEditor.backgroundRole(), QColor("#ffffff"))
144-
pal.setColor(self.templateEditor.foregroundRole(), QColor("#222222"))
145-
self.templateEditor.setPalette(pal)
146-
self.templateEditor.setStyleSheet("color: #222; background: #fff;")
147147

148148
# Data editor with JSON syntax highlighting
149149
self.dataEditor = QTextEdit()
150150
self.dataEditor.setPlaceholderText(
151151
'Enter JSON data for the template here...\nExample: {"name": "World", "items": [1, 2, 3]}'
152152
)
153153
self.dataEditor.setAcceptRichText(False)
154-
self.dataEditor.setFont(QFont(DEFAULT_FONT, 11))
154+
self.dataEditor.setFont(QFont(DEFAULT_FONT, DEFAULT_FONT_SIZE))
155155
self.jsonHighlighter = JSONSyntaxHighlighter(self.dataEditor.document())
156-
pal = self.dataEditor.palette()
157-
pal.setColor(self.dataEditor.backgroundRole(), QColor("#ffffff"))
158-
pal.setColor(self.dataEditor.foregroundRole(), QColor("#222222"))
159-
self.dataEditor.setPalette(pal)
160-
self.dataEditor.setStyleSheet("color: #222; background: #fff;")
161156

162157
# Output viewer with better formatting
163158
self.outputViewer = QTextBrowser()
164159
self.outputViewer.setPlaceholderText("Rendered output will appear here.")
165-
self.outputViewer.setFont(QFont(DEFAULT_FONT, 11))
160+
self.outputViewer.setFont(QFont(DEFAULT_FONT, DEFAULT_FONT_SIZE))
166161
self.outputViewer.setOpenExternalLinks(True)
167162
self.outputHighlighter = OutputSyntaxHighlighter(self.outputViewer.document())
168-
pal = self.outputViewer.palette()
169-
pal.setColor(self.outputViewer.backgroundRole(), QColor("#ffffff"))
170-
pal.setColor(self.outputViewer.foregroundRole(), QColor("#222222"))
171-
self.outputViewer.setPalette(pal)
172-
self.outputViewer.setStyleSheet("color: #222; background: #fff;")
173163

174164
# Buttons with better styling
175165
self.renderButton = QPushButton("Render Template")
176166
self.renderButton.setIcon(
177167
self._get_stock_icon(QApplication.style().SP_MediaPlay)
178168
)
179-
self.renderButton.setMinimumHeight(40)
169+
self.renderButton.setMinimumHeight(DEFAULT_BUTTON_HEIGHT)
180170

181171
self.clearDataButton = QPushButton("Clear Data")
182172
self.clearDataButton.setIcon(
@@ -188,7 +178,7 @@ def _create_widgets(self):
188178
self.progressBar.setVisible(False)
189179

190180
def _create_layouts(self):
191-
"""Create the main layout with improved organization."""
181+
"""Create the main layout with splitters and organize widgets into tabs."""
192182
# Main horizontal splitter
193183
self.mainSplitter = QSplitter(Qt.Horizontal)
194184

@@ -274,6 +264,14 @@ def _create_status_bar(self):
274264

275265
def _apply_styling(self):
276266
"""Apply modern styling to the application."""
267+
try:
268+
self._apply_styling_unsafe()
269+
except Exception as e:
270+
# If styling fails, log the error but continue with default styling
271+
print(f"Warning: Failed to apply custom styling: {e}")
272+
273+
def _apply_styling_unsafe(self):
274+
"""Apply modern styling to the application (unsafe version without error handling)."""
277275
if self._dark_mode:
278276
self.setStyleSheet(
279277
"""
@@ -387,6 +385,7 @@ def _get_stock_icon(self, standard_icon_enum) -> QIcon:
387385
return self.style().standardIcon(standard_icon_enum)
388386

389387
def _create_menu_bar(self):
388+
"""Create the menu bar with File, Edit, Tools, and Help menus."""
390389
menuBar = self.menuBar()
391390

392391
# File menu
@@ -487,6 +486,7 @@ def _create_menu_bar(self):
487486
helpMenu.addAction(aboutAction)
488487

489488
def _create_toolbar(self):
489+
"""Create the main toolbar with quick access buttons."""
490490
toolbar = QToolBar("Main Toolbar")
491491
toolbar.setIconSize(QSize(16, 16)) # Smaller icons for toolbar
492492
self.addToolBar(toolbar)
@@ -497,6 +497,7 @@ def _create_toolbar(self):
497497
toolbar.addAction(self.renderAction)
498498

499499
def _connect_signals(self):
500+
"""Connect all UI signals to their respective handler methods."""
500501
# File actions
501502
self.openTemplateAction.triggered.connect(self.open_template_file)
502503
self.openDataAction.triggered.connect(self.open_data_file)
@@ -539,12 +540,12 @@ def open_template_file(self):
539540
self._current_template_file_path = fileName
540541
self._update_window_title()
541542
self._add_to_recent_files(fileName)
542-
self.statusBar().showMessage(f"Template '{fileName}' loaded.", 5000)
543+
self.statusBar().showMessage(f"Template '{fileName}' loaded.", STATUS_MESSAGE_TIMEOUT_LONG)
543544
except IOError as e:
544545
QMessageBox.critical(
545546
self, "Error Opening File", f"Could not open template file:\n{e}"
546547
)
547-
self.statusBar().showMessage(f"Error opening template: {e}", 5000)
548+
self.statusBar().showMessage(f"Error opening template: {e}", STATUS_MESSAGE_TIMEOUT_LONG)
548549

549550
def open_data_file(self):
550551
fileName, _ = QFileDialog.getOpenFileName(
@@ -555,12 +556,12 @@ def open_data_file(self):
555556
with open(fileName, "r", encoding="utf-8") as file:
556557
self.dataEditor.setText(file.read())
557558
self._current_data_file_path = fileName
558-
self.statusBar().showMessage(f"Data file '{fileName}' loaded.", 5000)
559+
self.statusBar().showMessage(f"Data file '{fileName}' loaded.", STATUS_MESSAGE_TIMEOUT_LONG)
559560
except IOError as e:
560561
QMessageBox.critical(
561562
self, "Error Opening File", f"Could not open data file:\n{e}"
562563
)
563-
self.statusBar().showMessage(f"Error opening data file: {e}", 5000)
564+
self.statusBar().showMessage(f"Error opening data file: {e}", STATUS_MESSAGE_TIMEOUT_LONG)
564565

565566
def _save_template_to_path(self, file_path: str, silent: bool = False) -> bool:
566567
if not file_path:
@@ -571,13 +572,13 @@ def _save_template_to_path(self, file_path: str, silent: bool = False) -> bool:
571572
self._current_template_file_path = file_path
572573
self._update_window_title()
573574
if not silent:
574-
self.statusBar().showMessage(f"Template saved to '{file_path}'.", 5000)
575+
self.statusBar().showMessage(f"Template saved to '{file_path}'.", STATUS_MESSAGE_TIMEOUT_LONG)
575576
return True
576577
except IOError as e:
577578
QMessageBox.critical(
578579
self, "Error Saving File", f"Could not save template file:\n{e}"
579580
)
580-
self.statusBar().showMessage(f"Error saving template: {e}", 5000)
581+
self.statusBar().showMessage(f"Error saving template: {e}", STATUS_MESSAGE_TIMEOUT_LONG)
581582
return False
582583

583584
def save_template_file(self):
@@ -608,32 +609,32 @@ def save_output_file(self):
608609
try:
609610
with open(fileName, "w", encoding="utf-8") as file:
610611
file.write(self.outputViewer.toPlainText())
611-
self.statusBar().showMessage(f"Output saved to '{fileName}'.", 5000)
612+
self.statusBar().showMessage(f"Output saved to '{fileName}'.", STATUS_MESSAGE_TIMEOUT_LONG)
612613
except IOError as e:
613614
QMessageBox.critical(
614615
self, "Error Saving File", f"Could not save output file:\n{e}"
615616
)
616-
self.statusBar().showMessage(f"Error saving output: {e}", 5000)
617+
self.statusBar().showMessage(f"Error saving output: {e}", STATUS_MESSAGE_TIMEOUT_LONG)
617618

618619
def clear_data_editor(self):
619620
self.dataEditor.clear()
620-
self.statusBar().showMessage("Data editor cleared.", 3000)
621+
self.statusBar().showMessage("Data editor cleared.", STATUS_MESSAGE_TIMEOUT_SHORT)
621622

622623
def render_template(self):
623624
"""Render the template with the provided data."""
624625
template_str = self.templateEditor.toPlainText()
625626
data_str = self.dataEditor.toPlainText()
626627
if not template_str.strip():
627628
self.outputViewer.setText("Template is empty. Nothing to render.")
628-
self.statusBar().showMessage("Render attempted with empty template.", 3000)
629+
self.statusBar().showMessage("Render attempted with empty template.", STATUS_MESSAGE_TIMEOUT_SHORT)
629630
return
630631
# Validate JSON data
631632
is_valid, error_message, context_data = validate_json_data(data_str)
632633
if not is_valid:
633634
self.outputViewer.setText(
634635
format_error_message(error_message, "JSON Data Error")
635636
)
636-
self.statusBar().showMessage("JSON Data Error.", 5000)
637+
self.statusBar().showMessage("JSON Data Error.", STATUS_MESSAGE_TIMEOUT_LONG)
637638
return
638639
if context_data is None:
639640
context_data = {}
@@ -663,6 +664,11 @@ def format_date(fmt):
663664
"skills": [],
664665
"experience": [],
665666
}
667+
668+
# Clean up any existing thread before starting a new one
669+
if self._renderer_thread is not None and self._renderer_thread.isRunning():
670+
self._renderer_thread.wait()
671+
666672
# Show progress and disable render button
667673
self.progressBar.setVisible(True)
668674
self.progressBar.setRange(0, 0) # Indeterminate progress
@@ -677,21 +683,33 @@ def format_date(fmt):
677683

678684
def _on_template_rendered(self, rendered_text: str):
679685
"""Handle successful template rendering."""
680-
self.outputViewer.setText(rendered_text)
681-
self.statusLabel.setText("Template rendered successfully")
682-
self.statusBar().showMessage("Template rendered successfully.", 3000)
686+
try:
687+
self.outputViewer.setText(rendered_text)
688+
self.statusLabel.setText("Template rendered successfully")
689+
self.statusBar().showMessage("Template rendered successfully.", STATUS_MESSAGE_TIMEOUT_SHORT)
690+
except Exception as e:
691+
self.statusLabel.setText(f"Error displaying rendered output: {e}")
683692

684693
def _on_template_error(self, error_message: str):
685694
"""Handle template rendering error."""
686-
self.outputViewer.setText(error_message)
687-
self.statusLabel.setText("Rendering failed")
688-
self.statusBar().showMessage("Template Rendering Error.", 5000)
695+
try:
696+
self.outputViewer.setText(error_message)
697+
self.statusLabel.setText("Rendering failed")
698+
self.statusBar().showMessage("Template Rendering Error.", STATUS_MESSAGE_TIMEOUT_LONG)
699+
except Exception as e:
700+
self.statusLabel.setText(f"Error displaying error message: {e}")
689701

690702
def _on_render_finished(self):
691703
"""Handle render thread completion."""
692-
self.progressBar.setVisible(False)
693-
self.renderButton.setEnabled(True)
694-
self._renderer_thread = None
704+
try:
705+
self.progressBar.setVisible(False)
706+
self.renderButton.setEnabled(True)
707+
# Properly clean up the thread reference
708+
if self._renderer_thread is not None:
709+
self._renderer_thread.wait()
710+
self._renderer_thread = None
711+
except Exception as e:
712+
self.statusLabel.setText(f"Error finishing render: {e}")
695713

696714
def validate_template(self):
697715
"""Validate the current template syntax."""
@@ -731,7 +749,7 @@ def export_as_html(self):
731749
html_content = create_html_export(self.outputViewer.toPlainText())
732750
with open(fileName, "w", encoding="utf-8") as file:
733751
file.write(html_content)
734-
self.statusBar().showMessage(f"HTML exported to '{fileName}'.", 5000)
752+
self.statusBar().showMessage(f"HTML exported to '{fileName}'.", STATUS_MESSAGE_TIMEOUT_LONG)
735753
except IOError as e:
736754
QMessageBox.critical(
737755
self, "Export Error", f"Could not export HTML file:\n{e}"
@@ -740,12 +758,12 @@ def export_as_html(self):
740758
def clear_template_editor(self):
741759
"""Clear the template editor."""
742760
self.templateEditor.clear()
743-
self.statusBar().showMessage("Template editor cleared.", 3000)
761+
self.statusBar().showMessage("Template editor cleared.", STATUS_MESSAGE_TIMEOUT_SHORT)
744762

745763
def clear_output_viewer(self):
746764
"""Clear the output viewer."""
747765
self.outputViewer.clear()
748-
self.statusBar().showMessage("Output viewer cleared.", 3000)
766+
self.statusBar().showMessage("Output viewer cleared.", STATUS_MESSAGE_TIMEOUT_SHORT)
749767

750768
def _update_recent_files_menu(self):
751769
"""Update the recent files menu."""
@@ -779,7 +797,7 @@ def _open_recent_file(self, file_path: str):
779797
self._current_template_file_path = file_path
780798
self._update_window_title()
781799
self._add_to_recent_files(file_path)
782-
self.statusBar().showMessage(f"Recent file '{file_path}' loaded.", 5000)
800+
self.statusBar().showMessage(f"Recent file '{file_path}' loaded.", STATUS_MESSAGE_TIMEOUT_LONG)
783801
except IOError as e:
784802
QMessageBox.critical(
785803
self, "Error Opening File", f"Could not open recent file:\n{e}"
@@ -790,7 +808,7 @@ def _add_to_recent_files(self, file_path: str):
790808
if file_path in self._recent_files:
791809
self._recent_files.remove(file_path)
792810
self._recent_files.insert(0, file_path)
793-
self._recent_files = self._recent_files[:10] # Keep only last 10
811+
self._recent_files = self._recent_files[:MAX_RECENT_FILES]
794812
self._update_recent_files_menu()
795813

796814
def _clear_recent_files(self):
@@ -820,6 +838,9 @@ def _show_about(self):
820838

821839
def closeEvent(self, event):
822840
"""Handle application close event."""
841+
# Clean up any running threads before closing
842+
if self._renderer_thread is not None and self._renderer_thread.isRunning():
843+
self._renderer_thread.wait()
823844
self._save_settings()
824845
event.accept()
825846

0 commit comments

Comments
 (0)