-
Notifications
You must be signed in to change notification settings - Fork 72
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
3.1.1.8 Resolve Manage Patient Identifier Sources from XSS attack #123
Merged
Merged
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
2c855e5
3.1.1.8 Resolve Manage Patient Identifier Sources from XSS attack
slubwama ae1af38
add firstIdentifierBase
slubwama 5ddab58
keep it consistent
slubwama c384991
making sure display is also covered
slubwama 4f0a363
remove already existing file
slubwama 40dc494
match method to where is being used
slubwama 86101d6
renaming method
slubwama File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,21 @@ | |
<%@ include file="/WEB-INF/template/header.jsp"%> | ||
<%@ include file="localHeader.jsp"%> | ||
|
||
<script type="text/javascript"> | ||
function beforeSubmit() { | ||
const baseCharacterSet = document.getElementById('baseCharacterSet'); | ||
const prefix = document.getElementById('prefix'); | ||
const suffix = document.getElementById('suffix'); | ||
const firstIdentifierBase = document.getElementById('firstIdentifierBase'); | ||
|
||
baseCharacterSet.value=html_sanitize(baseCharacterSet.value); | ||
prefix.value=html_sanitize(prefix.value); | ||
suffix.value=html_sanitize(suffix.value); | ||
firstIdentifierBase.value=html_sanitize(firstIdentifierBase.value); | ||
return true; | ||
} | ||
</script> | ||
|
||
<style> | ||
.requiredField {font-weight:bold; color:red;} | ||
</style> | ||
|
@@ -22,7 +37,7 @@ | |
<spring:message code="idgen.general.for"/> <c:out value='${source.identifierType.name}'/> | ||
</h3> | ||
|
||
<frm:form modelAttribute="source" method="post" action="saveIdentifierSource.form"> | ||
<frm:form modelAttribute="source" method="post" action="saveIdentifierSource.form" onsubmit="return sanitizeAndSubmit()"> | ||
<frm:errors path="*" cssClass="error"/><br/> | ||
<table> | ||
<tr> | ||
|
@@ -69,12 +84,12 @@ | |
<span class="requiredField">*</span> | ||
<spring:message code="idgen.baseCharacterSet" />: | ||
</th> | ||
<td><frm:input path="baseCharacterSet" size="80" /><frm:errors path="baseCharacterSet" cssClass="error" /></td> | ||
<td><frm:input id="baseCharacterSet" path="baseCharacterSet" size="80" /><frm:errors path="baseCharacterSet" cssClass="error" /></td> | ||
</tr> | ||
<tr> | ||
<th align="right" valign="top"> | ||
<span class="requiredField">*</span> | ||
<spring:message code="idgen.firstIdentifierBase" />: | ||
<spring:message id="baseCharacterSet" code="idgen.firstIdentifierBase" />: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So, this seems to produce the error:
|
||
</th> | ||
<td> | ||
<c:choose> | ||
|
@@ -84,18 +99,18 @@ | |
<spring:message code="idgen.inUseUnableToModify" /> | ||
</c:when> | ||
<c:otherwise> | ||
<frm:input path="firstIdentifierBase" size="20" /><frm:errors path="firstIdentifierBase" cssClass="error" /> | ||
<frm:input id="firstIdentifierBase" path="firstIdentifierBase" size="20" /><frm:errors path="firstIdentifierBase" cssClass="error" /> | ||
</c:otherwise> | ||
</c:choose> | ||
</td> | ||
</tr> | ||
<tr> | ||
<th align="right"><spring:message code="idgen.prefix" />:</th> | ||
<td><frm:input path="prefix" size="10" /><frm:errors path="prefix" cssClass="error" /></td> | ||
<td><frm:input id="prefix" path="prefix" size="10" /><frm:errors path="prefix" cssClass="error" /></td> | ||
</tr> | ||
<tr> | ||
<th align="right"><spring:message code="idgen.suffix" />:</th> | ||
<td><frm:input path="suffix" size="10" /><frm:errors path="suffix" cssClass="error" /></td> | ||
<td><frm:input id="suffix" path="suffix" size="10" /><frm:errors path="suffix" cssClass="error" /></td> | ||
</tr> | ||
<tr> | ||
<th align="right"><spring:message code="idgen.minLength" />:</th> | ||
|
This file contains 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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you expect this to be named
sanitizeAndSubmit
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dkayiwa I have just changed the method call to beforeSubmit to allow flexibility in the future. In case there is a need of more client side controls on the form before submission
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The original method name was better because it described what exactly the method is doing.
It is not a good practice to sacrifice clarity for a possible future which in most cases turns out not to happen. 😊
If that future comes and we find that we need to rename/refactor, let it happen then, but not now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dkayiwa corrected to original
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The other question is, did you actually test this? The reason i am asking is simply because the method name was different from the one you had for the onSubmit attribute. So i would have expected you to get errors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dkayiwa I had tested it, and before pushing, I renamed the method to beforeSubmit. That’s how it ended up breaking. I forgot that this wouldn’t rename properly, since the method call in the form tag was being recognized as a string rather than an actual method call in IntelliJ. As a result, the automatic rename didn’t work.