Skip to content

Commit ceddad3

Browse files
committed
Improve escaping of strings in webapp.
Util.java: Improve formQuoteEscape() to also escape '&' characters, and clarify that it's for use in HTML attribute values. UtilTest.java: Add test for the new behaviour of formQuoteEscape(). menu.jspf: Use correct form of escaping for file type selection. search.jspf: Escape previously unescaped strings.
1 parent e8ab6d6 commit ceddad3

File tree

4 files changed

+22
-13
lines changed

4 files changed

+22
-13
lines changed

src/org/opensolaris/opengrok/web/Util.java

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -580,7 +580,8 @@ public static String URIEncode(String q) {
580580
return q == null ? "" : URLEncoder.encode(q, "UTF-8");
581581
} catch (UnsupportedEncodingException e) {
582582
// Should not happen. UTF-8 must be supported by JVMs.
583-
Logger.getLogger(EftarFileReader.class.getName()).log(Level.WARNING, "Failed to URL-encode UTF-8: ", e);
583+
Logger.getLogger(Util.class.getName()).log(
584+
Level.WARNING, "Failed to URL-encode UTF-8: ", e);
584585
}
585586
return null;
586587
}
@@ -649,8 +650,8 @@ public static String URIEncodePath(String path) {
649650
}
650651

651652
/**
652-
* Replace all quote characters (ASCI 0x22) with the corresponding html
653-
* entity (").
653+
* Escape a string for use as in an HTML attribute value. The returned
654+
* value is not enclosed in double quotes. The caller needs to add those.
654655
* @param q string to escape.
655656
* @return an empty string if a parameter is {@code null}, the mangled
656657
* string otherwise.
@@ -665,6 +666,8 @@ public static String formQuoteEscape(String q) {
665666
c = q.charAt(i);
666667
if (c == '"') {
667668
sb.append(""");
669+
} else if (c == '&') {
670+
sb.append("&");
668671
} else {
669672
sb.append(c);
670673
}

test/org/opensolaris/opengrok/web/UtilTest.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,7 @@ public void formQuoteEscape() {
179179
assertEquals("", Util.formQuoteEscape(null));
180180
assertEquals("abc", Util.formQuoteEscape("abc"));
181181
assertEquals(""abc"", Util.formQuoteEscape("\"abc\""));
182+
assertEquals("å", Util.formQuoteEscape("å"));
182183
}
183184

184185
@Test

web/menu.jspf

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,7 @@ information: Portions Copyright [yyyy] [name of copyright owner]
1818

1919
CDDL HEADER END
2020

21-
Copyright 2007 Sun Microsystems, Inc. All rights reserved.
22-
Use is subject to license terms.
21+
Copyright (c) 2007, 2013, Oracle and/or its affiliates. All rights reserved.
2322

2423
Portions Copyright 2011 Jens Elkner.
2524

@@ -111,11 +110,11 @@ org.opensolaris.opengrok.web.Util"
111110
<option value="">Any</option><%
112111
for (Map.Entry<String, String> d : SearchHelper.getFileTypeDescirptions()) {
113112
%>
114-
<option value="<%= d.getKey() %>"<%
113+
<option value="<%= Util.formQuoteEscape(d.getKey()) %>"<%
115114
if (d.getKey().equals(selection)) {
116115
%> selected="selected"<%
117116
}
118-
%>><%= Util.formQuoteEscape(d.getValue()) %></option><%
117+
%>><%= Util.htmlize(d.getValue()) %></option><%
119118
}
120119
%>
121120
</select>

web/search.jsp

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ information: Portions Copyright [yyyy] [name of copyright owner]
1818
1919
CDDL HEADER END
2020
21-
Copyright (c) 2005, 2011, Oracle and/or its affiliates. All rights reserved.
21+
Copyright (c) 2005, 2013, Oracle and/or its affiliates. All rights reserved.
2222
Portions Copyright 2011 Jens Elkner.
2323
2424
--%><%@page session="false" errorPage="error.jsp" import="
@@ -134,18 +134,23 @@ include file="menu.jspf"
134134
for (Suggestion hint : hints) {
135135
%><p><font color="#cc0000">Did you mean (for <%= hint.name %>)</font>:<%
136136
for (String word : hint.freetext) {
137-
%> <a href=search?q=<%= word %>><%= word %></a> &nbsp; <%
137+
%> <a href="search?q=<%= Util.URIEncode(word) %>"><%=
138+
Util.htmlize(word) %></a> &nbsp; <%
138139
}
139140
for (String word : hint.refs) {
140-
%> <a href=search?refs=<%= word %>><%= word %></a> &nbsp; <%
141+
%> <a href="search?refs=<%= Util.URIEncode(word) %>"><%=
142+
Util.htmlize(word) %></a> &nbsp; <%
141143
}
142144
for (String word : hint.defs) {
143-
%> <a href=search?defs=<%= word %>><%= word %></a> &nbsp; <%
145+
%> <a href="search?defs=<%= Util.URIEncode(word) %>"><%=
146+
Util.htmlize(word) %></a> &nbsp; <%
144147
}
145148
%></p><%
146149
}
147150
%>
148-
<p> Your search <b><%= searchHelper.query %></b> did not match any files.
151+
<p> Your search <b><%
152+
Util.htmlize(searchHelper.query.toString(), out); %></b>
153+
did not match any files.
149154
<br/> Suggestions:<br/>
150155
</p>
151156
<ul>
@@ -198,7 +203,8 @@ include file="menu.jspf"
198203
thispage = totalHits - start;
199204
}
200205
%>
201-
<p class="pagetitle">Searched <b><%= searchHelper.query
206+
<p class="pagetitle">Searched <b><%
207+
Util.htmlize(searchHelper.query.toString(), out);
202208
%></b> (Results <b> <%= start + 1 %> - <%= thispage + start
203209
%></b> of <b><%= totalHits %></b>) sorted by <%=
204210
searchHelper.order.getDesc() %></p><%

0 commit comments

Comments
 (0)