Skip to content

fix: resolve JSP syntax errors in multiple files#169

Merged
yingbull merged 8 commits intodevelop/coyotefrom
158-jsp-with-compile-time-issues-breaking-test-pipeline-on-branches
May 12, 2025
Merged

fix: resolve JSP syntax errors in multiple files#169
yingbull merged 8 commits intodevelop/coyotefrom
158-jsp-with-compile-time-issues-breaking-test-pipeline-on-branches

Conversation

@AlexSilveira1
Copy link
Copy Markdown

@AlexSilveira1 AlexSilveira1 commented May 9, 2025

Summary by Sourcery

Resolve JSP syntax errors in multiple pages and enhance the admin backup download page with null-safe session handling, JSTL tag usage, and XSS-safe file listing

Bug Fixes:

  • Fix malformed expression quoting in ChooseAllergy2.jsp image source and MultiPageDocDisplay.jsp calendar script tag
  • Correct missing newline at end of HTML file in ChooseAllergy2.jsp

Enhancements:

  • Add null-safe session checks and redirect to login in adminbackupdownload.jsp
  • Switch to JSTL taglibs for internationalization and URL generation in adminbackupdownload.jsp
  • Escape file names to prevent XSS and improve error handling for missing or empty backup path

@sourcery-ai
Copy link
Copy Markdown

sourcery-ai bot commented May 9, 2025

Reviewer's Guide

This PR resolves JSP syntax errors and overhauls adminbackupdownload.jsp to add error directives, null-safe session and security checks, cleaner file listing with JSTL and XSS protection, and corrects taglib misuse in ChooseAllergy2.jsp and MultiPageDocDisplay.jsp.

File-Level Changes

Change Details Files
Overhaul admin backup download JSP with improved error handling, security checks, and file listing
  • Add page directives for contentType, errorPage, and imports
  • Implement null-safe session attribute retrieval with redirect to login
  • Reformat security tag block and early exit on unauthorized access
  • Load backup_path, set in session, validate path and existence with forwarding or exception
  • Use JSTL c:set, c:url, fmt:message for calendar i18n
  • Escape file names to prevent XSS and generate sorted file list with descriptive errors
src/main/webapp/admin/adminbackupdownload.jsp
Fix mismatched quotes in JSP EL ternary expression for image src
  • Correct quotation marks around 'collapser' and 'expander' to valid JSP syntax
src/main/webapp/oscarRx/ChooseAllergy2.jsp
Correct taglib usage in calendar script inclusion
  • Remove incorrect fmt:setBundle inside src attribute and use fmt:message directly
src/main/webapp/documentManager/MultiPageDocDisplay.jsp

Possibly linked issues

  • Relative URL 404 Errors #103: The PR fixes specific JSP syntax errors, including <%= typos and incorrect tag usage, in files mentioned in the issue report.
  • #0: The PR fixes JSP syntax and tag errors in adminbackupdownload, ChooseAllergy2, and MultiPageDocDisplay.jsp as reported.

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@AlexSilveira1
Copy link
Copy Markdown
Author

@sourcery-ai review

Copy link
Copy Markdown

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @AlexSilveira1 - I've reviewed your changes and they look great!

Here's what I looked at during the review
  • 🟡 General issues: 3 issues found
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

AlexSilveira1 and others added 2 commits May 12, 2025 10:35
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
@AlexSilveira1
Copy link
Copy Markdown
Author

@sourcery-ai review

Copy link
Copy Markdown

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @AlexSilveira1 - I've reviewed your changes and found some issues that need to be addressed.

Blocking issues:

  • Escape file names before rendering (link)
Here's what I looked at during the review
  • 🟡 General issues: 1 issue found
  • 🔴 Security: 1 blocking issue, 1 other issue
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@AlexSilveira1
Copy link
Copy Markdown
Author

@sourcery-ai review

@AlexSilveira1 AlexSilveira1 linked an issue May 12, 2025 that may be closed by this pull request
Copy link
Copy Markdown

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @AlexSilveira1 - I've reviewed your changes - here's some feedback:

  • Consider replacing the heavy use of scriptlets (loops, out.println, session logic) with JSTL <c:forEach> and EL to make the JSP more readable and maintainable.
  • Unify your error handling strategy—you have both errorPage="/errorpage.jsp" and forwards to “/error.jsp”, plus thrown exceptions; pick one consistent approach and path.
  • Consider moving authentication and security checks out of the JSP into a servlet filter instead of inline security:oscarSec scriptlets for cleaner separation of concerns.
Here's what I looked at during the review
  • 🟡 General issues: 2 issues found
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@yingbull yingbull merged commit add5fdc into develop/coyote May 12, 2025
10 checks passed
@yingbull yingbull deleted the 158-jsp-with-compile-time-issues-breaking-test-pipeline-on-branches branch May 12, 2025 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

jsp with compile time issues breaking test pipeline on branches

2 participants