Skip to content
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

Un escaping the HTML for the retired answer concepts #100

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mozzy11
Copy link
Member

@mozzy11 mozzy11 commented Jan 4, 2019

@dkayiwa
Copy link
Member

dkayiwa commented Jan 11, 2019

Can you include the ticket id in your commit message as advised at? https://wiki.openmrs.org/display/docs/Pull+Request+Tips

@mozzy11
Copy link
Member Author

mozzy11 commented Jan 14, 2019

ok let me do that

@samuelmale
Copy link
Member

Are you still working on this?

@mozzy11
Copy link
Member Author

mozzy11 commented Feb 8, 2019

samwel , as far as i understood the ticket, i dont see any more work to be done ...the retired answer concepts no longer show html code..i just un-escaped the html genarated from the server side

@mozzy11
Copy link
Member Author

mozzy11 commented Feb 8, 2019

if u look at the last image i attached ..i seem to have solved the problem.. unless there are any other comments from u but i think i finished this ..

@samuelmale
Copy link
Member

Did you see @dkayiwa 's comments?

@mozzy11
Copy link
Member Author

mozzy11 commented Feb 8, 2019

you mean including the ticket id in my commit messages? yes i did that

@mozzy11
Copy link
Member Author

mozzy11 commented Feb 8, 2019

i did that a49cc47

@@ -498,7 +498,7 @@
<tr>
<td valign="top">
<select class="largeWidth" size="6" id="answerNames" multiple="multiple" onKeyUp="listKeyPress('answerNames', 'answerIds', ' ', event)">
<c:forEach items="${command.conceptAnswers}" var="answer">
<c:forEach items="${command.conceptAnswers}" var="answer" >
Copy link
Member

Choose a reason for hiding this comment

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

Do we need this change?

Copy link
Member Author

Choose a reason for hiding this comment

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

sorry , that was un intended

@@ -9,4 +9,4 @@ legacyui.manageuser.noProviderIdentifier=No Identifier Specified

${project.parent.artifactId}.Location.purgeLocation=Permanently Delete Location
${project.parent.artifactId}.Location.confirmDelete=Are you sure you want to delete this Location? It will be permanently removed from the system.
${project.parent.artifactId}.Location.purgedSuccessfully=Location deleted successfully
Copy link
Member

Choose a reason for hiding this comment

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

Do we need this change?

Copy link
Member Author

Choose a reason for hiding this comment

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

that was un intended , i cant even figure out the change (:

@mozzy11
Copy link
Member Author

mozzy11 commented Feb 25, 2019

i removed the unnecesary changes

@ibacher
Copy link
Member

ibacher commented Nov 9, 2020

@mozzy11 Is there another approach we can take here rather than just not escaping XML, since this makes XSS attacks possible through the concept name field?

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.

4 participants