ampersand on search text

2014-08-14 Thread Greg Huber
Glen,

When I do a search containing and ampersand, roller does not show correctly
the returned text.

eg

bz

actually returns :bamp;amp;z

which renders  as bamp;z

It should return bamp;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

2014-08-14 Thread Glen Mazza
Hi Greg, that was done by Dave as part of this commit last August 13th: 
http://svn.apache.org/viewvc?view=revisionrevision=151, 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

bz

actually returns :bamp;amp;z

which renders  as bamp;z

It should return bamp;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: simplify requiredWeblogPermissionActions() and requiredGlobalPermissionActions()?

2014-08-14 Thread Glen Mazza
Does anyone else on the team have a view?  #5 below I'm no longer sure 
on so I'm withdrawing that proposal but feedback on #1-#4 below is most 
welcome.


Glen

On 08/13/2014 04:27 PM, Dave wrote:

I don't see the need for this change and I would leave those permissions in
place. They existed to support and may still be used to support real uses
cases like private blogging, where only registered users can see blogs and
only those with special permissions can comment.

Even if they do not work fully now, they give people a way to hook their
own rules into their own custom versions of Roller, perhaps by adding new
code, ServletFilters, etc.  And they are a starting point for people who
want private blogging to be fully supported in Roller.

- Dave




On Wed, Aug 13, 2014 at 3:51 PM, Glen Mazza glen.ma...@gmail.com wrote:


OK, checking Global Permission, we have these three levels:

 /** Allowed to login and edit profile */
 public static final String LOGIN  = login;

 /** Allowed to login and do weblogging */
 public static final String WEBLOG = weblog;

 /** Allowed to login and do everything, including site-wide admin */
 public static final String ADMIN  = admin;

We don't use weblog though, we save it as editor in the userrole
table.  We also don't use login for anything other than to make it the
minimum required setting on pages that don't require an ADMIN setting.  All
newly registered users are given editor as a minimum, meaning we could
raise minimum from login to editor and do away with the login role
without any difference in application behavior.

On top of this, we allow the roles additional subroles per the
roller.properties file:

# Role name to global permission action mappings
role.names=editor,admin
role.action.editor=login,comment,weblog
role.action.admin=login,comment,weblog,admin

comment is also never used in the application, further, in the above
list we're inconsistently assigning admin to admin but weblog to editor.
  Since the permissions are all Russian doll (login  comment 
weblog/editor  admin), it's sufficient to just store the highest role, as
the lower ones are all implied, i.e., we don't need these properties.

My proposal is to:

1.) Replace the above LOGIN/WEBLOG/ADMIN strings with a two-value
enumeration, EDITOR and ADMIN.  Later, if we have user demand for LOGIN and
COMMENT, and somebody actually coding in logic that uses those values, we
can easily add in the enumerations for them.  (I don't like LOGIN much,
however, if we don't trust them not to blog they shouldn't be lurking
around the UI.)

2.) The userrole database table will be dropped, replaced with a new
varchar column ROLE in the Roller_User table.  I'll update the migration
script to copy the user's highest role into that column.

3.) The three properties role.name, role.action.editor, and
role.action.admin will be removed.

4.) ListString requiredGlobalPermissionActions() will return a single
enumeration constant instead (EDITOR or ADMIN), stating the minimum
accepted value.

5.) WeblogPermissions looks fine, except I'll just switch the string array
of EDIT_DRAFT, ADMIN, POST, to an enumeration constant with the same values
and have requiredWeblogPermissionActions() return an enumeration constant
instead.

How does this sound?  I have other things to work on so I'll wait 72 hours
before proceeding to give time for others to evaluate this change.

Regards,
Glen



On 08/13/2014 08:33 AM, Glen Mazza wrote:


If the methods return just a single permission instead of a collection of
permissions, at least for GlobalPermissionActions, that means we can move
to Russian doll type role levels, where each permission level includes
all the permission levels below it (de facto the way Roller runs now).  If
we can officially be on that, that means we can toss out the userrole table
and just place a single column rolename (indicating the highest role a
person has) in the roller_user table, a very sleek change.  (I'm not
talking about Roller_Permission, i.e., permissions a user has on each blog
-- that table is still needed, but the userrole table indicating whether
one's a global admin or not.)

Glen

On 08/13/2014 07:54 AM, Dave wrote:


I don't have a strong opinion, but this seems like change just for the
sake
of change. I doubt that impacts performance in any significant way,
especially when compared to all the database calls that are made during
JSP
or page template processing.

- Dave


On Tue, Aug 12, 2014 at 9:15 PM, Glen Mazza glen.ma...@gmail.com
wrote:

  Hi team, one or both of these methods are heavily called within the

application, indeed for almost every action run:

  public ListString requiredWeblogPermissionActions() {
  return Collections.singletonList(WeblogPermission.x);
  }

  public ListString requiredGlobalPermissionActions() {
  return Collections.singletonList(GlobalPermission.xx);
  }

I've checked every implementation of both