Fix: Using double quotes " breaks prescriptions#2483
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Snapshot WarningsEnsure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice. Scanned FilesNone |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideEscapes prescription text values when rendering them into hidden HTML input attributes so that embedded double quotes no longer truncate faxed/printed prescriptions or encounter-note text. Sequence diagram for fax/print prescription flow with encoded hidden inputsequenceDiagram
participant PreviewContent_jsp as PreviewContent_jsp
participant Browser as Browser
participant FrmCustomedPDFServlet as FrmCustomedPDFServlet
PreviewContent_jsp->PreviewContent_jsp: Encode.forHtmlAttribute(strRx)
PreviewContent_jsp->Browser: render rx hidden input with encoded value
Browser->FrmCustomedPDFServlet: submit preview2Form with parameter rx
FrmCustomedPDFServlet->FrmCustomedPDFServlet: getParameter(rx)
FrmCustomedPDFServlet->FrmCustomedPDFServlet: generate PDF with full prescription text
Sequence diagram for encounter note creation using encoded rx_no_newlinessequenceDiagram
participant PreviewContent_jsp as PreviewContent_jsp
participant Browser as Browser
participant EncounterNote as EncounterNote
PreviewContent_jsp->PreviewContent_jsp: Encode.forHtmlAttribute(strRxNoNewLines)
PreviewContent_jsp->Browser: render rx_no_newlines hidden input with encoded value
Browser->Browser: document.getElementById(rx_no_newlines).value
Browser->EncounterNote: printPaste2Parent inserts full prescription text into note
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
@SourceryAI review |
|
@coderabbitai review |
✅ Action performedReview finished.
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request improves security in PreviewContent.jsp by replacing direct EL expressions for the 'rx' and 'rx_no_newlines' hidden input fields with JSP scriptlets that HTML-attribute-encode the values using Encode.forHtmlAttribute(), thereby mitigating potential Cross-Site Scripting (XSS) vulnerabilities. There are no review comments, and I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
📝 WalkthroughWalkthroughIn ChangesPrescription Hidden Input Source Fix
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Consider applying the same
Encode.forHtmlAttributetreatment toadditNotes(and any other hidden inputs carrying user/content data) for consistency and to avoid similar truncation/escaping issues in other fields. - The new scriptlet-based encoding for
strRxandstrRxNoNewLinesintroduces mixed styles with the surrounding EL-based attributes; if possible, consolidating on a single approach (e.g., a custom EL function for attribute encoding) would keep the JSP more uniform and easier to maintain.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider applying the same `Encode.forHtmlAttribute` treatment to `additNotes` (and any other hidden inputs carrying user/content data) for consistency and to avoid similar truncation/escaping issues in other fields.
- The new scriptlet-based encoding for `strRx` and `strRxNoNewLines` introduces mixed styles with the surrounding EL-based attributes; if possible, consolidating on a single approach (e.g., a custom EL function for attribute encoding) would keep the JSP more uniform and easier to maintain.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/main/webapp/oscarRx/printRx/PreviewContent.jsp`:
- Line 321: The hidden input field with name attribute `additNotes` has the same
encoding vulnerability as the prescription text fields fixed at lines 319-320.
Apply the same encoding/escaping pattern to the `${requestScope.comment}` value
in the value attribute of the `additNotes` input element to ensure that double
quote characters and other special characters are properly escaped and do not
prematurely close the attribute value, which would cause data truncation when
the form is submitted.
🪄 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
Run ID: db558b97-65ef-49c1-a8ef-55201f17563b
📒 Files selected for processing (1)
src/main/webapp/oscarRx/printRx/PreviewContent.jsp
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
2 participants
Summary
A double quote (
") in a drug name, instructions, or special instructions silentlytruncated the faxed/printed prescription and the "Add to encounter note" text from the
first
"onward. The Rx body was interpolated raw into two double-quoted HTML hidden-inputattributes, so the
"closed the attribute early and everything after it was lost. The fixapplies context-appropriate OWASP encoding at those two output points. This is the same class
of bug — and the same fix pattern — as #2451 / PR #2459, which handled the pharmacy name; this
change covers the prescription text itself.
Files changed:
src/main/webapp/oscarRx/printRx/PreviewContent.jspFixes #2482
Problem
When a drug name or its (special) instructions contained a
", e.g.Tylenol "extra":".button) was cut off at the first
".The on-screen preview looked correct, which masked the problem.
Root cause: in
PreviewContent.jsp, the prescription text was rendered raw into twodouble-quoted HTML hidden inputs:
Both values come from
RxPrescriptionData.getFullOutLine()(drug name + instructions + specialinstructions, concatenated). A
"in that text closed thevalue="..."attribute early, so:name="rx"— submitted viapreview2FormtoFrmCustomedPDFServlet(
/form/createcustomedpdf) for the print/fax PDF. The form posted a truncated value, sothe PDF was cut off. (The servlet splits the text on newlines/
<br>, never on", so thetruncation is purely the browser closing the attribute at submit time.)
name="rx_no_newlines"— read in JS byprintPaste2Parent()asdocument.getElementById("rx_no_newlines").value. The DOM only contained text up to the first", so the encounter note was truncated.The on-screen preview was unaffected because it renders the Rx in an HTML body context
(
<td>${fullOutLine}</td>), where a literal"is harmless — the bug only manifests in theattribute context of the hidden inputs.
This also explains the reporter's observation that the fax issue was recent while the
encounter note issue was long-standing: the fax-via-PDF-servlet path and the
rx_no_newlinesinput were introduced in the recent Rx print-preview refactor, whereas theencounter-note paste has emitted Rx text into an unescaped attribute/JS context for far longer.
In the current code both symptoms share the one root cause above, so a single change resolves
both.
Solution
Encode the two hidden inputs at the output point, per context, instead of leaving them raw —
matching the already-correct sibling inputs in this same file (
pharmaName,pharmaFax,pharmacyInfo) that were fixed in PR #2459:Encode.forHtmlAttributeturns"into", so it can no longer break out of thevalue="..."attribute. The encoding is transparent to both consumers — the browser decodesit back to a literal
":FrmCustomedPDFServlet.getParameter("rx")receives the real value, so theprinted/faxed PDF shows the full line.
getElementById("rx_no_newlines").valuereturns the real value, so theencounter note contains the full line.
Summary by Sourcery
Escape prescription text when rendering into hidden HTML inputs to prevent truncation when it contains double quotes.
Bug Fixes:
Summary by CodeRabbit