You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@roller.apache.org by Greg Huber <gr...@gmail.com> on 2014/08/14 10:03:25 UTC
ampersand on search text
Glen,
When I do a search containing and ampersand, roller does not show correctly
the returned text.
eg
b&z
actually returns :b&amp;z
which renders as b&z
It should return b&z with no second ampersand for it to render
correctly.
Checking the method getTerm() it does a double escape, where the
StringEscapeUtils.escapeXml(..) adds the extra amp; causing it not to show
correctly :
SearchResultsModel():
public String getTerm() {
String query = searchRequest.getQuery();
return (query == null)
? "" : StringEscapeUtils.escapeXml(Utilities.escapeHTML(query));
}
Do we need the double escape? For XSS? StringEscapeUtils.escapeXml() or
how do we make it render correctly?
Cheers Greg.
Re: ampersand on search text
Posted by Glen Mazza <gl...@gmail.com>.
OK, I changed the code to:
public String getTerm() {
String query = searchRequest.getQuery();
return (query == null)
? "" : StringEscapeUtils.escapeXml10(query);
}
in SearchResultsModel, the double-escaping problem has gone away. The
"escapeXML" method is deprecated, so I replaced it with escapeXML10,
apparently still the dominant XML to use.
Regards,
Glen
On 08/20/2014 09:48 AM, Glen Mazza wrote:
> Oops, I note the code is escapeXML(escapeHTML), not
> escapeHTML(escapeHTML). Still checking...
>
> Glen
>
>
> On 08/19/2014 09:52 PM, Glen Mazza wrote:
>> Hi Dave, do you know why
>> "StringEscapeUtils.escapeXml(Utilities.escapeHTML(query)); " is not
>> just coded as "Utilities.escapeHTML(query); " ?
>>
>> On the basic theme search, if I do a search on "home run" (with
>> quotes), the search field gets repopulated as "home run" ,
>> if I search again, another pair of "s get added in, etc.
>>
>> I can test and remove that in a heartbeat, but I'm not sure if the
>> double escapeHTML was meant to guard against XSS attacks. Might that
>> have been the case? If so, maybe we can postpone a full solution to
>> past 5.1.
>>
>> (Incidentally, I just updated my own blog to the latest 5.1, and
>> found a couple of minor nitpicks, unneeded files, etc. that I've
>> since fixed in the code. I'm trying to keep changes minimal this
>> week, unless a known error.)
>>
>> Thanks,
>> Glen
>>
>>
>> On 08/14/2014 07:19 AM, Glen Mazza wrote:
>>> Hi Greg, that was done by Dave as part of this commit last August
>>> 13th: http://svn.apache.org/viewvc?view=revision&revision=1510000,
>>> which *may* have been part of the XSS security release Dave did the
>>> following November:
>>> http://rollerweblogger.org/project/entry/apache_roller_5_0_2.
>>>
>>> It may have been a copy and paste error, checking in feeds.vm in the
>>> above commit he does a escapeHTML(removeHTML) but in the other an
>>> escapeHTML(escapeHTML). One of the three files,
>>> SearchResultsModel() had no real changes, just the formatting was
>>> rearranged.
>>>
>>> I would say we don't need to allow searching on punctuation
>>> characters (does Google even?) but if Dave doesn't respond and
>>> removing one of the escapeHTML calls fixes things without breaking
>>> more important stuff, perhaps good to go ahead with the change.
>>> Certainly, if it needs to be reapplied, next time we can put in a
>>> comment saying why the consecutive escapeHTML() calls are necessary.
>>>
>>> Regards,
>>> Glen
>>>
>>> On 08/14/2014 04:03 AM, Greg Huber wrote:
>>>> Glen,
>>>>
>>>> When I do a search containing and ampersand, roller does not show
>>>> correctly
>>>> the returned text.
>>>>
>>>> eg
>>>>
>>>> b&z
>>>>
>>>> actually returns :b&amp;z
>>>>
>>>> which renders as b&z
>>>>
>>>> It should return b&z with no second ampersand for it to render
>>>> correctly.
>>>>
>>>> Checking the method getTerm() it does a double escape, where the
>>>> StringEscapeUtils.escapeXml(..) adds the extra amp; causing it not
>>>> to show
>>>> correctly :
>>>>
>>>> SearchResultsModel():
>>>>
>>>> public String getTerm() {
>>>> String query = searchRequest.getQuery();
>>>> return (query == null)
>>>> ? "" :
>>>> StringEscapeUtils.escapeXml(Utilities.escapeHTML(query));
>>>> }
>>>>
>>>> Do we need the double escape? For XSS?
>>>> StringEscapeUtils.escapeXml() or
>>>> how do we make it render correctly?
>>>>
>>>>
>>>> Cheers Greg.
>>>>
>>>
>>
>
Re: ampersand on search text
Posted by Glen Mazza <gl...@gmail.com>.
Oops, I note the code is escapeXML(escapeHTML), not
escapeHTML(escapeHTML). Still checking...
Glen
On 08/19/2014 09:52 PM, Glen Mazza wrote:
> Hi Dave, do you know why
> "StringEscapeUtils.escapeXml(Utilities.escapeHTML(query)); " is not
> just coded as "Utilities.escapeHTML(query); " ?
>
> On the basic theme search, if I do a search on "home run" (with
> quotes), the search field gets repopulated as "home run" ,
> if I search again, another pair of "s get added in, etc.
>
> I can test and remove that in a heartbeat, but I'm not sure if the
> double escapeHTML was meant to guard against XSS attacks. Might that
> have been the case? If so, maybe we can postpone a full solution to
> past 5.1.
>
> (Incidentally, I just updated my own blog to the latest 5.1, and found
> a couple of minor nitpicks, unneeded files, etc. that I've since fixed
> in the code. I'm trying to keep changes minimal this week, unless a
> known error.)
>
> Thanks,
> Glen
>
>
> On 08/14/2014 07:19 AM, Glen Mazza wrote:
>> Hi Greg, that was done by Dave as part of this commit last August
>> 13th: http://svn.apache.org/viewvc?view=revision&revision=1510000,
>> which *may* have been part of the XSS security release Dave did the
>> following November:
>> http://rollerweblogger.org/project/entry/apache_roller_5_0_2.
>>
>> It may have been a copy and paste error, checking in feeds.vm in the
>> above commit he does a escapeHTML(removeHTML) but in the other an
>> escapeHTML(escapeHTML). One of the three files, SearchResultsModel()
>> had no real changes, just the formatting was rearranged.
>>
>> I would say we don't need to allow searching on punctuation
>> characters (does Google even?) but if Dave doesn't respond and
>> removing one of the escapeHTML calls fixes things without breaking
>> more important stuff, perhaps good to go ahead with the change.
>> Certainly, if it needs to be reapplied, next time we can put in a
>> comment saying why the consecutive escapeHTML() calls are necessary.
>>
>> Regards,
>> Glen
>>
>> On 08/14/2014 04:03 AM, Greg Huber wrote:
>>> Glen,
>>>
>>> When I do a search containing and ampersand, roller does not show
>>> correctly
>>> the returned text.
>>>
>>> eg
>>>
>>> b&z
>>>
>>> actually returns :b&amp;z
>>>
>>> which renders as b&z
>>>
>>> It should return b&z with no second ampersand for it to render
>>> correctly.
>>>
>>> Checking the method getTerm() it does a double escape, where the
>>> StringEscapeUtils.escapeXml(..) adds the extra amp; causing it not
>>> to show
>>> correctly :
>>>
>>> SearchResultsModel():
>>>
>>> public String getTerm() {
>>> String query = searchRequest.getQuery();
>>> return (query == null)
>>> ? "" :
>>> StringEscapeUtils.escapeXml(Utilities.escapeHTML(query));
>>> }
>>>
>>> Do we need the double escape? For XSS? StringEscapeUtils.escapeXml() or
>>> how do we make it render correctly?
>>>
>>>
>>> Cheers Greg.
>>>
>>
>
Re: ampersand on search text
Posted by Glen Mazza <gl...@gmail.com>.
Hi Dave, do you know why
"StringEscapeUtils.escapeXml(Utilities.escapeHTML(query)); " is not just
coded as "Utilities.escapeHTML(query); " ?
On the basic theme search, if I do a search on "home run" (with quotes),
the search field gets repopulated as "home run" , if I search
again, another pair of "s get added in, etc.
I can test and remove that in a heartbeat, but I'm not sure if the
double escapeHTML was meant to guard against XSS attacks. Might that
have been the case? If so, maybe we can postpone a full solution to
past 5.1.
(Incidentally, I just updated my own blog to the latest 5.1, and found a
couple of minor nitpicks, unneeded files, etc. that I've since fixed in
the code. I'm trying to keep changes minimal this week, unless a known
error.)
Thanks,
Glen
On 08/14/2014 07:19 AM, Glen Mazza wrote:
> Hi Greg, that was done by Dave as part of this commit last August
> 13th: http://svn.apache.org/viewvc?view=revision&revision=1510000,
> which *may* have been part of the XSS security release Dave did the
> following November:
> http://rollerweblogger.org/project/entry/apache_roller_5_0_2.
>
> It may have been a copy and paste error, checking in feeds.vm in the
> above commit he does a escapeHTML(removeHTML) but in the other an
> escapeHTML(escapeHTML). One of the three files, SearchResultsModel()
> had no real changes, just the formatting was rearranged.
>
> I would say we don't need to allow searching on punctuation characters
> (does Google even?) but if Dave doesn't respond and removing one of
> the escapeHTML calls fixes things without breaking more important
> stuff, perhaps good to go ahead with the change. Certainly, if it
> needs to be reapplied, next time we can put in a comment saying why
> the consecutive escapeHTML() calls are necessary.
>
> Regards,
> Glen
>
> On 08/14/2014 04:03 AM, Greg Huber wrote:
>> Glen,
>>
>> When I do a search containing and ampersand, roller does not show
>> correctly
>> the returned text.
>>
>> eg
>>
>> b&z
>>
>> actually returns :b&amp;z
>>
>> which renders as b&z
>>
>> It should return b&z with no second ampersand for it to render
>> correctly.
>>
>> Checking the method getTerm() it does a double escape, where the
>> StringEscapeUtils.escapeXml(..) adds the extra amp; causing it not
>> to show
>> correctly :
>>
>> SearchResultsModel():
>>
>> public String getTerm() {
>> String query = searchRequest.getQuery();
>> return (query == null)
>> ? "" :
>> StringEscapeUtils.escapeXml(Utilities.escapeHTML(query));
>> }
>>
>> Do we need the double escape? For XSS? StringEscapeUtils.escapeXml() or
>> how do we make it render correctly?
>>
>>
>> Cheers Greg.
>>
>
Re: ampersand on search text
Posted by Glen Mazza <gl...@gmail.com>.
Hi Greg, that was done by Dave as part of this commit last August 13th:
http://svn.apache.org/viewvc?view=revision&revision=1510000, which *may*
have been part of the XSS security release Dave did the following
November: http://rollerweblogger.org/project/entry/apache_roller_5_0_2.
It may have been a copy and paste error, checking in feeds.vm in the
above commit he does a escapeHTML(removeHTML) but in the other an
escapeHTML(escapeHTML). One of the three files, SearchResultsModel()
had no real changes, just the formatting was rearranged.
I would say we don't need to allow searching on punctuation characters
(does Google even?) but if Dave doesn't respond and removing one of the
escapeHTML calls fixes things without breaking more important stuff,
perhaps good to go ahead with the change. Certainly, if it needs to be
reapplied, next time we can put in a comment saying why the consecutive
escapeHTML() calls are necessary.
Regards,
Glen
On 08/14/2014 04:03 AM, Greg Huber wrote:
> Glen,
>
> When I do a search containing and ampersand, roller does not show correctly
> the returned text.
>
> eg
>
> b&z
>
> actually returns :b&amp;z
>
> which renders as b&z
>
> It should return b&z with no second ampersand for it to render
> correctly.
>
> Checking the method getTerm() it does a double escape, where the
> StringEscapeUtils.escapeXml(..) adds the extra amp; causing it not to show
> correctly :
>
> SearchResultsModel():
>
> public String getTerm() {
> String query = searchRequest.getQuery();
> return (query == null)
> ? "" : StringEscapeUtils.escapeXml(Utilities.escapeHTML(query));
> }
>
> Do we need the double escape? For XSS? StringEscapeUtils.escapeXml() or
> how do we make it render correctly?
>
>
> Cheers Greg.
>