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

Basic UI: escape HTML characters #3749

Merged
merged 1 commit into from Jul 3, 2017

Conversation

Projects
None yet
4 participants
@lolodomo
Copy link
Contributor

commented Jun 26, 2017

Fix #3744

Signed-off-by: Laurent Garnier lg.hc@free.fr

@lolodomo

This comment has been minimized.

Copy link
Contributor Author

commented Jun 26, 2017

@resetnow : this one is for you ,)
All my tests are positive.

@sjka sjka added the UI-BasicUI label Jun 27, 2017

@resetnow
Copy link
Contributor

left a comment

Please see my remark, otherwise LGTM

@@ -82,7 +82,8 @@ public boolean canRender(Widget w) {
String button = getSnippet("button");
button = StringUtils.replace(button, "%item%", w.getItem());
button = StringUtils.replace(button, "%cmd%", mapping.getCmd());

This comment has been minimized.

Copy link
@resetnow

resetnow Jun 27, 2017

Contributor

I think we need escape here as well. Test case:

Switch item=DemoString mappings=["value__1"="description_1", "value_\"_2"="description_2"]

This comment has been minimized.

Copy link
@resetnow

resetnow Jun 27, 2017

Contributor

Also, using StringEscapeUtils.escapeHTML here is probably not the right choice — only quotes and backslashes need to be prepended with \ so resulting HTML attribute has correct value when it's being read from Javascript.

This comment has been minimized.

Copy link
@lolodomo

lolodomo Jun 28, 2017

Author Contributor

I thought about mappings from number or ON/OFF/OPEN/CLOSE... for example but not from a general string.
You're right, it has to be tested and fixed for general string commands too.

Basic UI: escape HTML characters
Fix #3744

Signed-off-by: Laurent Garnier <lg.hc@free.fr>

@lolodomo lolodomo force-pushed the lolodomo:basic_escape branch from dd84797 to ff98449 Jul 2, 2017

@lolodomo

This comment has been minimized.

Copy link
Contributor Author

commented Jul 2, 2017

Fixed.

@resetnow
Copy link
Contributor

left a comment

LGTM

@sjka sjka merged commit becd5fe into eclipse:master Jul 3, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
ip-validation
Details
@sjka

This comment has been minimized.

Copy link
Contributor

commented Jul 3, 2017

Thanks!

@lolodomo lolodomo deleted the lolodomo:basic_escape branch Jul 3, 2017

@kaikreuzer kaikreuzer added this to the 0.9.0 milestone Nov 30, 2017

@kaikreuzer kaikreuzer added the bug label Dec 15, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.