Weceem

Editors don't work in projects with a default codec other than "none"

Details

  • Type: Bug Bug
  • Status: Committed Committed
  • Priority: Minor Minor
  • Resolution: Unresolved
  • Affects Version/s: 0.9
  • Fix Version/s: 1.2
  • Component/s: None
  • Labels:
    None
  • Environment:
    Linux with Firefox 3.5.1
  • Request Controller:
    Please Select
  • External Supervisor:
    Please select
  • Executing Programmer:
    Please select

Description

Weceem assumes that projects have grails.views.default.codec="none", but in some cases this is not a good option for security reasons. If projects define grails.views.default.codec="html" for instance, then the rich editor and code editor don't display their content correctly (html characters are escaped).

  1. WCM-447.diff
    29/Jan/10 8:44 AM
    1 kB
    Geli Crick
  2. WCM-447-extra.diff
    04/Feb/10 11:15 AM
    4 kB
    Geli Crick

Activity

Hide
Geli Crick added a comment - 29/Jan/10 8:44 AM

Here is a patch based on revision 58895 of trunk. It changes the rich editor and the code editor so that their content is inserted using <%= %> rather than ${ }. This way no matter what codec a project defines, the editors will never receive escaped html. In theory this could have been done by placing <%@page defaultCodec="none" %> in the gsp, but I couldn't get this to work (see http://stackoverflow.com/questions/1337464/overriding-grails-views-default-codechtml-config-back-to-none).

As far as security is concerned, I don't think this changes much for weceem, since it defined a default codec of "none" anyway. Also, there are probably other places I didn't find where this could be changed.

Show
Geli Crick added a comment - 29/Jan/10 8:44 AM Here is a patch based on revision 58895 of trunk. It changes the rich editor and the code editor so that their content is inserted using <%= %> rather than ${ }. This way no matter what codec a project defines, the editors will never receive escaped html. In theory this could have been done by placing <%@page defaultCodec="none" %> in the gsp, but I couldn't get this to work (see http://stackoverflow.com/questions/1337464/overriding-grails-views-default-codechtml-config-back-to-none). As far as security is concerned, I don't think this changes much for weceem, since it defined a default codec of "none" anyway. Also, there are probably other places I didn't find where this could be changed.
Hide
Marc Palmer added a comment - 29/Jan/10 11:44 AM

Thanks for this. Frankly in my experience the grails default codec setting is just bad news. Problems like this will always occur with plugins. That's why we are explicit about our codec usage.

We'll have to set the codec to none in all GSPs I think. Personally I would not use default codecs in my app though, as in non-trivial apps (eg this case too) you have to turn it off in pages anyway. Better to turn ON a default codec in just the GSPs that have lots of expansion and no complications that might arise in accidental double-encoding.

Show
Marc Palmer added a comment - 29/Jan/10 11:44 AM Thanks for this. Frankly in my experience the grails default codec setting is just bad news. Problems like this will always occur with plugins. That's why we are explicit about our codec usage. We'll have to set the codec to none in all GSPs I think. Personally I would not use default codecs in my app though, as in non-trivial apps (eg this case too) you have to turn it off in pages anyway. Better to turn ON a default codec in just the GSPs that have lots of expansion and no complications that might arise in accidental double-encoding.
Hide
Geli Crick added a comment - 29/Jan/10 12:28 PM

Yep, it does seem rather problematic for plugins because there isn't a way to override it at the plugin level (at least that I know of). I think you're right, that the only way to make sure the weceem plugin works right with any app is to set it explicitly in the GSPs that need it- none for things like fckeditor, and html encoded for things like comments.

In our app we actually find being able to set the default encoding very useful. It is a very large app, that includes a lot of user generated content, so there are a lot of opportunities to make a mistake and allow cross site scripting. For us it's much better to have a double-encoding accident than a security accident Inside the weceem administration it's not so much of a problem because in theory only trusted users have access.

Show
Geli Crick added a comment - 29/Jan/10 12:28 PM Yep, it does seem rather problematic for plugins because there isn't a way to override it at the plugin level (at least that I know of). I think you're right, that the only way to make sure the weceem plugin works right with any app is to set it explicitly in the GSPs that need it- none for things like fckeditor, and html encoded for things like comments. In our app we actually find being able to set the default encoding very useful. It is a very large app, that includes a lot of user generated content, so there are a lot of opportunities to make a mistake and allow cross site scripting. For us it's much better to have a double-encoding accident than a security accident Inside the weceem administration it's not so much of a problem because in theory only trusted users have access.
Hide
Geli Crick added a comment - 04/Feb/10 11:15 AM

I found a few more places where accented characters were getting double encoded in the administration view. Here's another patch with just the extra changes, also from revision 58895 of trunk.

Show
Geli Crick added a comment - 04/Feb/10 11:15 AM I found a few more places where accented characters were getting double encoded in the administration view. Here's another patch with just the extra changes, also from revision 58895 of trunk.

People

Vote (0)
Watch (0)

Dates

  • Created:
    29/Jan/10 8:31 AM
    Updated:
    25/Aug/11 5:16 PM