Project template - #6
Closed
LuongTranDuc03 wants to merge 13 commits into
Closed
Conversation
xóa smart tomcat
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (5)
src/main/webapp/assets/css/layout.css (1)
886-911: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer kebab-case for keyframe names.
CSS linters recommend using kebab-case for keyframe animation names (e.g.,
slide-downinstead ofslideDown) to maintain consistent styling conventions.♻️ Proposed refactor
- animation: slideDown 0.3s ease; + animation: slide-down 0.3s ease; } .alert-banner.success { @@ -898,7 +898,7 @@ border: 1px solid `#fca5a5`; } -@keyframes slideDown { +@keyframes slide-down { from { transform: translateY(-10px); opacity: 0;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/webapp/assets/css/layout.css` around lines 886 - 911, Rename the CSS keyframe animation `slideDown` to the kebab-case name `slide-down`, and update the corresponding animation declaration to reference the renamed keyframe.Source: Linters/SAST tools
src/main/webapp/assets/css/admin.css (1)
1-9: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer string literals over
url()for@import.Modern CSS conventions and linters (like Stylelint) recommend using a direct string literal for
@importstatements to keep the syntax concise.♻️ Proposed refactor
-@import url('layout.css'); -@import url('sidebar.css'); +@import 'layout.css'; +@import 'sidebar.css'; /* Feature-based stylesheets */ -@import url('products/product.css'); -@import url('invoices/invoice.css'); -@import url('employees/employee.css'); -@import url('coupons/coupon.css'); -@import url('customers/customer.css'); +@import 'products/product.css'; +@import 'invoices/invoice.css'; +@import 'employees/employee.css'; +@import 'coupons/coupon.css'; +@import 'customers/customer.css';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/webapp/assets/css/admin.css` around lines 1 - 9, Update all `@import` declarations in the admin stylesheet to use direct quoted string literals instead of the url() wrapper, preserving each existing stylesheet path and import order.Source: Linters/SAST tools
src/main/java/project/duan1_sd21301/controller/admin/luong/ProductValidator.java (1)
333-450: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNear-duplicate of the per-variant validation in
validateProduct.
validateSingleVariantre-implements almost the same field-by-field checks (color/size/style/prices/stock/dimensions/images) as the loop body invalidateProduct(lines 211-328). Extracting a shared per-variant validator would prevent the two from silently diverging.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/project/duan1_sd21301/controller/admin/luong/ProductValidator.java` around lines 333 - 450, The per-variant checks in validateSingleVariant duplicate the validation logic inside validateProduct and can diverge. Extract the shared color, size, style, price, stock, dimension, and image validation into one reusable helper, then have both validateProduct and validateSingleVariant call it while preserving their existing inputs and returned errors.src/main/java/project/duan1_sd21301/controller/admin/luong/ProductController.java (1)
410-413: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove empty
try {} catch {}blocks. Both are dead stubs left behind after the promo-price parsing was removed; they do nothing and obscure intent.
src/main/java/project/duan1_sd21301/controller/admin/luong/ProductController.java#L410-L413: delete the empty try/catch in theupdateVariantfield-setting sequence.src/main/java/project/duan1_sd21301/controller/admin/luong/ProductController.java#L564-L567: delete the empty try/catch in theaddVariantfield-setting sequence.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/project/duan1_sd21301/controller/admin/luong/ProductController.java` around lines 410 - 413, Remove the empty try/catch blocks from the field-setting sequences in ProductController.updateVariant (src/main/java/project/duan1_sd21301/controller/admin/luong/ProductController.java:410-413) and ProductController.addVariant (src/main/java/project/duan1_sd21301/controller/admin/luong/ProductController.java:564-567), leaving the surrounding field-setting logic intact.src/main/webapp/WEB-INF/views/admin/luong/product-list.jsp (1)
888-953: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove dead legacy filter code.
applyFilters_old,resetFilters_old, and theDOMContentLoaded_oldlistener are unreachable:'DOMContentLoaded_old'is not a real event, and neither_oldfunction is referenced anywhere. They also reference elements that no longer exist (sortSelect,.cat-pill), so they would throw if ever called. Deleting them reduces confusion.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/webapp/WEB-INF/views/admin/luong/product-list.jsp` around lines 888 - 953, Remove the unreachable legacy functions applyFilters_old and resetFilters_old, along with the document listener registered for DOMContentLoaded_old. Do not alter the active applyFilters/resetFilters implementation or other current filtering behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pom.xml`:
- Around line 21-26: Update the mssql-jdbc dependency version in the Maven
dependency declaration to a fixed 12.6.x release, using 12.6.5 or newer and
retaining the jre11 classifier.
In `@src/main/webapp/WEB-INF/views/admin/luong/product-add.jsp`:
- Around line 571-575: HTML-encode all user-controlled JSP output before
rendering. In src/main/webapp/WEB-INF/views/admin/luong/product-add.jsp lines
571-575 and the related fields, escape id, name, brand, origin,
careInstructions, and description values in inputs and textareas; in
src/main/webapp/WEB-INF/views/admin/luong/product-list.jsp lines 671-686, escape
data-* attributes and table text for id, name, category, and brand; in
src/main/webapp/WEB-INF/views/admin/luong/variant-list.jsp lines 388-409, escape
data-* attributes and cells for productId, color, size, and style, using
encoding that handles &, <, >, quotes, and apostrophes rather than only
replacing double quotes.
In `@src/main/webapp/WEB-INF/views/admin/luong/product-detail.jsp`:
- Around line 700-707: Escape the values rendered by getCareInstructions() and
getDescription() in the corresponding readonly textareas so markup cannot break
out of the textarea; preserve each existing null fallback. Use the JSP project’s
established XML/HTML escaping mechanism, and update ProductValidator only if its
existing validation patterns support applying equivalent checks to both fields.
- Around line 58-64: Update the action=edit branch in product-detail.jsp to
forward via request.getRequestDispatcher to
/WEB-INF/views/admin/luong/product-add.jsp, keeping the existing product lookup,
pageTitle assignment, and return behavior unchanged.
In `@src/main/webapp/WEB-INF/views/admin/luong/variant-list.jsp`:
- Around line 442-449: Add a hidden row with id noResultsRow inside the variant
table’s tbody, alongside the existing server-side empty-state row, using
colspan="9" and the filter-specific Vietnamese message so applyFilters can
toggle feedback when no client-side results match.
---
Nitpick comments:
In
`@src/main/java/project/duan1_sd21301/controller/admin/luong/ProductController.java`:
- Around line 410-413: Remove the empty try/catch blocks from the field-setting
sequences in ProductController.updateVariant
(src/main/java/project/duan1_sd21301/controller/admin/luong/ProductController.java:410-413)
and ProductController.addVariant
(src/main/java/project/duan1_sd21301/controller/admin/luong/ProductController.java:564-567),
leaving the surrounding field-setting logic intact.
In
`@src/main/java/project/duan1_sd21301/controller/admin/luong/ProductValidator.java`:
- Around line 333-450: The per-variant checks in validateSingleVariant duplicate
the validation logic inside validateProduct and can diverge. Extract the shared
color, size, style, price, stock, dimension, and image validation into one
reusable helper, then have both validateProduct and validateSingleVariant call
it while preserving their existing inputs and returned errors.
In `@src/main/webapp/assets/css/admin.css`:
- Around line 1-9: Update all `@import` declarations in the admin stylesheet to
use direct quoted string literals instead of the url() wrapper, preserving each
existing stylesheet path and import order.
In `@src/main/webapp/assets/css/layout.css`:
- Around line 886-911: Rename the CSS keyframe animation `slideDown` to the
kebab-case name `slide-down`, and update the corresponding animation declaration
to reference the renamed keyframe.
In `@src/main/webapp/WEB-INF/views/admin/luong/product-list.jsp`:
- Around line 888-953: Remove the unreachable legacy functions applyFilters_old
and resetFilters_old, along with the document listener registered for
DOMContentLoaded_old. Do not alter the active applyFilters/resetFilters
implementation or other current filtering behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4e063650-55a9-4007-aa66-911431f74b15
⛔ Files ignored due to path filters (2)
Project1-SD21301-main+email.raris excluded by!**/*.rarProject1-SD21301-main.raris excluded by!**/*.rar
📒 Files selected for processing (50)
.gitignore.smarttomcat/duan1_sd21301/conf/catalina.policy.smarttomcat/duan1_sd21301/conf/catalina.properties.smarttomcat/duan1_sd21301/conf/context.xml.smarttomcat/duan1_sd21301/conf/jaspic-providers.xml.smarttomcat/duan1_sd21301/conf/jaspic-providers.xsd.smarttomcat/duan1_sd21301/conf/logging.properties.smarttomcat/duan1_sd21301/conf/server.xml.smarttomcat/duan1_sd21301/conf/tomcat-users.xml.smarttomcat/duan1_sd21301/conf/tomcat-users.xsd.smarttomcat/duan1_sd21301/conf/web.xmlpom.xmlsrc/main/java/project/duan1_sd21301/controller/admin/CouponController.javasrc/main/java/project/duan1_sd21301/controller/admin/DashboardController.javasrc/main/java/project/duan1_sd21301/controller/admin/HomeController.javasrc/main/java/project/duan1_sd21301/controller/admin/InvoiceController.javasrc/main/java/project/duan1_sd21301/controller/admin/ha/CustomerController.javasrc/main/java/project/duan1_sd21301/controller/admin/huy/EmployeeController.javasrc/main/java/project/duan1_sd21301/controller/admin/luong/ProductController.javasrc/main/java/project/duan1_sd21301/controller/admin/luong/ProductValidator.javasrc/main/java/project/duan1_sd21301/controller/admin/luong/VariantController.javasrc/main/java/project/duan1_sd21301/controller/admin/phuc/InvoiceController.javasrc/main/java/project/duan1_sd21301/controller/admin/tuan/CouponController.javasrc/main/java/project/duan1_sd21301/model/Coupon.javasrc/main/java/project/duan1_sd21301/model/Invoice.javasrc/main/java/project/duan1_sd21301/model/luong/Product.javasrc/main/java/project/duan1_sd21301/model/luong/ProductDetail.javasrc/main/java/project/duan1_sd21301/repository/CouponRepository.javasrc/main/resources/luong/mock_data.jsonsrc/main/resources/luong/mock_data.sqlsrc/main/webapp/WEB-INF/views/admin/coupon-list.jspsrc/main/webapp/WEB-INF/views/admin/dashboard.jspsrc/main/webapp/WEB-INF/views/admin/invoice-list.jspsrc/main/webapp/WEB-INF/views/admin/luong/product-add.jspsrc/main/webapp/WEB-INF/views/admin/luong/product-detail.jspsrc/main/webapp/WEB-INF/views/admin/luong/product-list.jspsrc/main/webapp/WEB-INF/views/admin/luong/variant-list.jspsrc/main/webapp/WEB-INF/views/admin/product-add.jspsrc/main/webapp/WEB-INF/views/admin/product-detail.jspsrc/main/webapp/WEB-INF/views/admin/product-list.jspsrc/main/webapp/WEB-INF/views/layout/sidebar.jspsrc/main/webapp/WEB-INF/views/layout/toast.jspsrc/main/webapp/assets/css/admin.csssrc/main/webapp/assets/css/coupons/coupon.csssrc/main/webapp/assets/css/customers/customer.csssrc/main/webapp/assets/css/employees/employee.csssrc/main/webapp/assets/css/invoices/invoice.csssrc/main/webapp/assets/css/layout.csssrc/main/webapp/assets/css/products/product.csssrc/main/webapp/assets/css/sidebar.css
💤 Files with no reviewable changes (20)
- .smarttomcat/duan1_sd21301/conf/server.xml
- .smarttomcat/duan1_sd21301/conf/tomcat-users.xml
- .smarttomcat/duan1_sd21301/conf/logging.properties
- .smarttomcat/duan1_sd21301/conf/tomcat-users.xsd
- src/main/java/project/duan1_sd21301/model/Coupon.java
- .smarttomcat/duan1_sd21301/conf/context.xml
- .smarttomcat/duan1_sd21301/conf/catalina.policy
- src/main/webapp/WEB-INF/views/admin/dashboard.jsp
- .smarttomcat/duan1_sd21301/conf/jaspic-providers.xml
- .smarttomcat/duan1_sd21301/conf/catalina.properties
- src/main/java/project/duan1_sd21301/controller/admin/InvoiceController.java
- src/main/java/project/duan1_sd21301/repository/CouponRepository.java
- src/main/webapp/WEB-INF/views/admin/invoice-list.jsp
- src/main/webapp/WEB-INF/views/admin/coupon-list.jsp
- .smarttomcat/duan1_sd21301/conf/jaspic-providers.xsd
- src/main/java/project/duan1_sd21301/model/Invoice.java
- src/main/webapp/WEB-INF/views/admin/product-add.jsp
- src/main/java/project/duan1_sd21301/controller/admin/CouponController.java
- src/main/webapp/WEB-INF/views/admin/product-detail.jsp
- src/main/webapp/WEB-INF/views/admin/product-list.jsp
Comment on lines
+21
to
+26
| <!-- Thư viện kết nối SQL Server --> | ||
| <dependency> | ||
| <groupId>com.microsoft.sqlserver</groupId> | ||
| <artifactId>mssql-jdbc</artifactId> | ||
| <version>12.6.1.jre11</version> | ||
| </dependency> |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
mssql-jdbc CVE-2025-59250 fixed version
💡 Result:
CVE-2025-59250 is a spoofing vulnerability in the Microsoft JDBC Driver for SQL Server caused by improper input validation [1][2]. To mitigate this vulnerability, you must update your driver to one of the following fixed versions (or later) corresponding to your current version branch [3][1][4]: - 10.2 branch: Update to 10.2.4 or later [3][4] - 11.2 branch: Update to 11.2.4 or later [3][4] - 12.2 branch: Update to 12.2.1 or later [3][4] - 12.4 branch: Update to 12.4.3 or later [3][4] - 12.6 branch: Update to 12.6.5 or later [3][4] - 12.8 branch: Update to 12.8.2 or later [3][1][4] - 12.10 branch: Update to 12.10.2 or later [3][4] - 13.2 branch: Update to 13.2.1 or later [1][4][5] Official release notes from Microsoft confirm that these updates address the vulnerability [1]. Always ensure you are using the latest stable release for your specific driver branch [1].
Citations:
- 1: https://learn.microsoft.com/en-us/sql/connect/jdbc/release-notes-for-the-jdbc-driver?view=sql-server-ver17
- 2: https://nvd.nist.gov/vuln/detail/cve-2025-59250
- 3: https://osv.dev/vulnerability/CVE-2025-59250
- 4: https://app.opencve.io/cve/CVE-2025-59250
- 5: Addressed CVE-2025-59250 and version updates for 13.2.1 microsoft/mssql-jdbc#2801
Bump mssql-jdbc past the vulnerable 12.6.1 release. 12.6.1.jre11 is affected by CVE-2025-59250; Microsoft lists 12.6.5+ as the fixed 12.6 branch release.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pom.xml` around lines 21 - 26, Update the mssql-jdbc dependency version in
the Maven dependency declaration to a fixed 12.6.x release, using 12.6.5 or
newer and retaining the jre11 classifier.
Comment on lines
+571
to
+575
| <input type="text" id="id" name="id" class="form-input" placeholder="Ví dụ: SP001" value="<%= product != null && product.getId() != null ? product.getId() : "" %>" <%= isEdit ? "readonly style='background-color: #f1f5f9; cursor: not-allowed;'" : "" %> required> | ||
| </div> | ||
| <div class="form-group"> | ||
| <label class="form-label" for="name">Tên sản phẩm <span style="color: red;">*</span></label> | ||
| <input type="text" id="name" name="name" class="form-input" placeholder="Nhập tên sản phẩm..." value="<%= product != null && product.getName() != null ? product.getName() : "" %>" required> |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- product-add.jsp excerpt ---'
sed -n '560,670p' src/main/webapp/WEB-INF/views/admin/luong/product-add.jsp
echo
echo '--- product-list.jsp excerpt ---'
sed -n '660,705p' src/main/webapp/WEB-INF/views/admin/luong/product-list.jsp
echo
echo '--- variant-list.jsp excerpt ---'
sed -n '380,425p' src/main/webapp/WEB-INF/views/admin/luong/variant-list.jsp
echo
echo '--- search for escaping helpers/usages ---'
rg -n "escapeXml|stringEscapeUtils|escapeHtml|fn:escapeXml|c:out" src/main/webapp/WEB-INF/views/admin/luong -SRepository: LuongTranDuc03/Project1-SD21301
Length of output: 23638
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for f in \
src/main/webapp/WEB-INF/views/admin/luong/product-add.jsp \
src/main/webapp/WEB-INF/views/admin/luong/product-list.jsp \
src/main/webapp/WEB-INF/views/admin/luong/variant-list.jsp
do
echo "=== $f ==="
nl -ba "$f" | sed -n '560,670p;660,705p;380,425p' 2>/dev/null || true
done
echo '=== escaping usage ==='
rg -n "escapeXml|stringEscapeUtils|escapeHtml|fn:escapeXml|c:out" src/main/webapp/WEB-INF/views/admin/luong -S || trueRepository: LuongTranDuc03/Project1-SD21301
Length of output: 503
Escape these JSP outputs before rendering
src/main/webapp/WEB-INF/views/admin/luong/product-add.jsp: encode the rawvalue/textareaoutputs (id,name,brand,origin,careInstructions,description) before writing them into HTML.src/main/webapp/WEB-INF/views/admin/luong/product-list.jsp: encode thedata-*values and table-cell text forid,name,category, andbrand.src/main/webapp/WEB-INF/views/admin/luong/variant-list.jsp: encode thedata-*values and cells forproductId,color,size, andstyle; replacing only"with"still leaves<and&injection paths open.
📍 Affects 3 files
src/main/webapp/WEB-INF/views/admin/luong/product-add.jsp#L571-L575(this comment)src/main/webapp/WEB-INF/views/admin/luong/product-list.jsp#L671-L686src/main/webapp/WEB-INF/views/admin/luong/variant-list.jsp#L388-L409
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/webapp/WEB-INF/views/admin/luong/product-add.jsp` around lines 571 -
575, HTML-encode all user-controlled JSP output before rendering. In
src/main/webapp/WEB-INF/views/admin/luong/product-add.jsp lines 571-575 and the
related fields, escape id, name, brand, origin, careInstructions, and
description values in inputs and textareas; in
src/main/webapp/WEB-INF/views/admin/luong/product-list.jsp lines 671-686, escape
data-* attributes and table text for id, name, category, and brand; in
src/main/webapp/WEB-INF/views/admin/luong/variant-list.jsp lines 388-409, escape
data-* attributes and cells for productId, color, size, and style, using
encoding that handles &, <, >, quotes, and apostrophes rather than only
replacing double quotes.
Comment on lines
+58
to
+64
| String requestAction = request.getParameter("action"); | ||
| if ("edit".equals(requestAction)) { | ||
| Product prodObj = (Product) request.getAttribute("product"); | ||
| request.setAttribute("pageTitle", "Chỉnh sửa sản phẩm " + (prodObj != null ? prodObj.getId() : "")); | ||
| request.getRequestDispatcher("/WEB-INF/views/admin/product-add.jsp").forward(request, response); | ||
| return; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm whether the legacy path still exists and whether this branch is actually reachable.
fd -a product-add.jsp
rg -n "product-add.jsp" -g '*.jsp' -g '*.java'Repository: LuongTranDuc03/Project1-SD21301
Length of output: 246
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- relevant lines in product-detail.jsp ---'
sed -n '50,70p' src/main/webapp/WEB-INF/views/admin/luong/product-detail.jsp
echo
echo '--- all product-add.jsp references ---'
rg -n "product-add\.jsp|action=edit|/admin/products" src/main/webapp src/main/java
echo
echo '--- all product-add.jsp files ---'
fd -a 'product-add.jsp' src/main/webappRepository: LuongTranDuc03/Project1-SD21301
Length of output: 6980
Forward the edit branch to the luong JSP
product-detail.jsp sends action=edit to /WEB-INF/views/admin/product-add.jsp, but the actual admin edit view is /WEB-INF/views/admin/luong/product-add.jsp, so this branch will fail when reached.
Proposed fix
- request.getRequestDispatcher("/WEB-INF/views/admin/product-add.jsp").forward(request, response);
+ request.getRequestDispatcher("/WEB-INF/views/admin/luong/product-add.jsp").forward(request, response);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| String requestAction = request.getParameter("action"); | |
| if ("edit".equals(requestAction)) { | |
| Product prodObj = (Product) request.getAttribute("product"); | |
| request.setAttribute("pageTitle", "Chỉnh sửa sản phẩm " + (prodObj != null ? prodObj.getId() : "")); | |
| request.getRequestDispatcher("/WEB-INF/views/admin/product-add.jsp").forward(request, response); | |
| return; | |
| } | |
| String requestAction = request.getParameter("action"); | |
| if ("edit".equals(requestAction)) { | |
| Product prodObj = (Product) request.getAttribute("product"); | |
| request.setAttribute("pageTitle", "Chỉnh sửa sản phẩm " + (prodObj != null ? prodObj.getId() : "")); | |
| request.getRequestDispatcher("/WEB-INF/views/admin/luong/product-add.jsp").forward(request, response); | |
| return; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/webapp/WEB-INF/views/admin/luong/product-detail.jsp` around lines 58
- 64, Update the action=edit branch in product-detail.jsp to forward via
request.getRequestDispatcher to /WEB-INF/views/admin/luong/product-add.jsp,
keeping the existing product lookup, pageTitle assignment, and return behavior
unchanged.
Comment on lines
+700
to
+707
| <div class="form-group full-width"> | ||
| <label class="form-label" for="careInstructions">Hướng dẫn bảo quản</label> | ||
| <textarea id="careInstructions" class="form-textarea" readonly style="background-color: #f1f5f9; cursor: not-allowed;"><%= prod.getCareInstructions() != null ? prod.getCareInstructions() : "Chưa có hướng dẫn bảo quản cho sản phẩm này." %></textarea> | ||
| </div> | ||
| <div class="form-group full-width"> | ||
| <label class="form-label" for="description">Mô tả sản phẩm</label> | ||
| <textarea id="description" class="form-textarea" readonly style="background-color: #f1f5f9; cursor: not-allowed;"><%= prod.getDescription() != null ? prod.getDescription() : "Chưa có mô tả chi tiết cho sản phẩm này." %></textarea> | ||
| </div> |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Unescaped description/careInstructions rendered inside a <textarea> — stored XSS risk.
ProductValidator only blocks dangerous characters (<, >, %) for the product name; description and care instructions have no such restriction, and here they're written with raw <%= %> instead of an escaping mechanism. A value containing </textarea><script>...</script> would break out and execute on every future load of this page.
🛡️ Proposed fix
- <textarea id="careInstructions" class="form-textarea" readonly style="background-color: `#f1f5f9`; cursor: not-allowed;"><%= prod.getCareInstructions() != null ? prod.getCareInstructions() : "Chưa có hướng dẫn bảo quản cho sản phẩm này." %></textarea>
+ <textarea id="careInstructions" class="form-textarea" readonly style="background-color: `#f1f5f9`; cursor: not-allowed;"><c:out value="${product.careInstructions}" default="Chưa có hướng dẫn bảo quản cho sản phẩm này."/></textarea>Apply the same treatment (e.g., <c:out>/fn:escapeXml) to description at line 706, and consider adding a length/dangerous-character check for these two fields in ProductValidator as defense in depth.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/webapp/WEB-INF/views/admin/luong/product-detail.jsp` around lines
700 - 707, Escape the values rendered by getCareInstructions() and
getDescription() in the corresponding readonly textareas so markup cannot break
out of the textarea; preserve each existing null fallback. Use the JSP project’s
established XML/HTML escaping mechanism, and update ProductValidator only if its
existing validation patterns support applying equivalent checks to both fields.
Comment on lines
+442
to
+449
| %> | ||
| <tr> | ||
| <td colspan="9" style="text-align: center; padding: 40px; color: #9ca3af;">Không có dữ liệu biến thể.</td> | ||
| </tr> | ||
| <% | ||
| } | ||
| %> | ||
| </tbody> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Missing "no results" row for client-side filtering.
applyFilters toggles an element with id noResultsRow (Line 736-738), but this table never renders one — only the server-side "Không có dữ liệu biến thể" row when the whole list is empty. When a filter/search matches nothing, users see an empty table with no feedback. Add a hidden noResultsRow like product-list.jsp does.
💡 Suggested addition inside <tbody>
<tr id="noResultsRow" class="no-results-row" style="display:none;">
<td colspan="9" style="text-align: center; padding: 48px; color: `#94a3b8`;">
Không tìm thấy biến thể phù hợp với bộ lọc hiện tại.
</td>
</tr>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/webapp/WEB-INF/views/admin/luong/variant-list.jsp` around lines 442
- 449, Add a hidden row with id noResultsRow inside the variant table’s tbody,
alongside the existing server-side empty-state row, using colspan="9" and the
filter-specific Vietnamese message so applyFilters can toggle feedback when no
client-side results match.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Updates
Chores