Fix: Rx note giving an error if pharmacy name contains double quotes#144
Merged
Conversation
Resolved Conflicts: src/main/webapp/oscarRx/printRx/PreviewContent.jsp
Reviewer's GuideEnsures pharmacy/provider names are stored raw in the action and then safely rendered in both JavaScript onClick handlers and HTML hidden inputs using context-appropriate OWASP encoding to fix Rx print/fax buttons breaking when names contain double quotes. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
setupAdditionalAttributes, the null-coalescing and trimming for pharmacy name/fax is open-coded; consider extracting a small helper to normalize these values so you don’t duplicate this pattern if other pharmacy fields are added later. - In
PreviewContent.jsp,patientDOBnow usesEncode.forHtmlwhile neighboring fields still useStringEscapeUtils.escapeHtml; it would be clearer and less error-prone to standardize on a single encoding library for all these patient-related attributes.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `setupAdditionalAttributes`, the null-coalescing and trimming for pharmacy name/fax is open-coded; consider extracting a small helper to normalize these values so you don’t duplicate this pattern if other pharmacy fields are added later.
- In `PreviewContent.jsp`, `patientDOB` now uses `Encode.forHtml` while neighboring fields still use `StringEscapeUtils.escapeHtml`; it would be clearer and less error-prone to standardize on a single encoding library for all these patient-related attributes.
## Individual Comments
### Comment 1
<location path="src/main/webapp/oscarRx/printRx/PreviewContent.jsp" line_range="121-122" />
<code_context>
+ <input type="hidden" name="patientDOB"
</code_context>
<issue_to_address>
**🚨 issue (security):** Use `Encode.forHtmlAttribute` for `patientDOB` since the value is in an HTML attribute context.
Since this value is rendered in an HTML attribute (`value="..."`), it should use attribute-safe encoding to prevent malformed attributes or potential XSS if the string ever contains quotes or other special characters. `Encode.forHtmlAttribute` is designed for this context, unlike `Encode.forHtml`, which targets body content.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
b056deb
into
MagentaHealth:release/2026-03-17
7 of 9 checks passed
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
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
A double quote in a pharmacy name broke the Print & Add to encounter note button —
clicking it did nothing. The fix applies context-appropriate OWASP encoding at each JSP
output point so pharmacy/provider values are safe in both the JavaScript
onClickhandlersand the HTML hidden inputs that get submitted to the prescription PDF servlet.
Files changed:
src/main/java/ca/openosp/openo/rx/RxPrintPreview2Action.javasrc/main/webapp/oscarRx/printRx/PrintPreview.jspsrc/main/webapp/oscarRx/printRx/PreviewContent.jspOriginal PR: openo-beta#2459
Problem
When a patient's preferred pharmacy name contained a double quote (
"), clickingPrint & Add to encounter note (and the Fax + paste button) did nothing — neither the
print dialog nor the encounter window opened.
Root cause: in
PrintPreview.jsp, the pharmacy name was interpolated raw into adouble-quoted
onClickHTML attribute:onClick="printPasteToParent('${ctx}', ..., '${requestScope.prefPharmacy}', ...);">The value reaching the JSP was only escaped for single quotes in the action
(
pharmacy.getName().replace("'", "\\'")), so a"passed through raw, closed theonClick="..."attribute early, and left a truncated, malformed handler — making the buttona no-op.
A second, related defect surfaced during review:
pharmacyNameandpharmacyFaxaredual-context — they are used in the
onClick(JavaScript string) and in hidden inputsin
PreviewContent.jspthat are submitted toFrmCustomedPDFServlet(pharmaName/pharmaFax).JavaScript-escaping these at the Java source would submit escaped literals
(e.g.
Test \x22Quote\x22 Pharmacy) to the PDF servlet instead of the real name.Solution
Encode at the output, per context, rather than at the Java source — matching the pattern
already used by the sibling hidden inputs (
patientName→Encode.forHtml,pharmacyInfo→Encode.forHtmlAttribute).RxPrintPreview2Action.java— store the raw (null-guarded) values; no source-sideencoding:
providerName,prefPharmacy,pharmacyName,pharmacyFaxPrintPreview.jsp— JavaScript-string context (both the print and faxonClickhandlers): wrap each of the four values in
${e:forJavaScript(...)}.forJavaScriptescapes"(→\x22),',\,/,<,>,&, so the value is safe as a JS string literal andcannot break out of the surrounding double-quoted attribute. The browser decodes the escapes
back to the literal characters at runtime, so the encounter note shows the real name.
PreviewContent.jsp— HTML-attribute context (hidden inputs submitted to the PDF servlet):pharmaName/pharmaFaxuse<%= Encode.forHtmlAttribute(...) %>. HTML-attribute encodingdecodes back to the real value on form submit, so
FrmCustomedPDFServletreceivesTest "Quote" Pharmacy, not the escaped literal.Why output-side encoding
Encoding at the Java source is the wrong layer for a value rendered into more than one context:
the same string was JS-escaped once but consumed as both JavaScript and an HTML attribute, so
one of the two contexts was always wrong. Encoding at each output point lets every consumer
apply the encoding its context requires. Values are coalesced to
""before being stored sothe request attributes are never null.
Summary by Sourcery
Apply context-appropriate output encoding for provider and pharmacy data in Rx print preview to prevent broken buttons and ensure safe values in both JavaScript handlers and PDF form submissions.
Bug Fixes:
Enhancements: