The specification for hook_filter() provides for filters to define a 'prepare' procedure as well as 'process'. That way they can get the text ready and can escape some things important to them so that other filters which come first do not destroy them.
In the function check_markup(), we see that hook_filter() is invoked with $op='prepare' before being invoked again with $op='process':
// Give filters the chance to escape HTML-like data such as code or formulas.
foreach ($filters as $filter) {
$text = module_invoke($filter->module, 'filter', 'prepare', $filter->delta, $format, $text, $cache_id);
}
// Perform filtering.
foreach ($filters as $filter) {
$text = module_invoke($filter->module, 'filter', 'process', $filter->delta, $format, $text, $cache_id);
}
taken from filter.module, function check_markup()
So when CKEditor applies the security filters it should do the same. At the moment it does just this:
$text = module_invoke($module, 'filter', 'process', $delta, $format, $text);
taken from ckeditor.page.inc, function ckeditor_filter_xss()
This prevents some filters from working as they were designed to do.
Shall I roll a patch?
Comment | File | Size | Author |
---|---|---|---|
#1 | ckeditor-1278062.patch | 1.65 KB | martin_q |
Comments
Comment #1
martin_qI went ahead and rolled a patch.
Comment #2
Fabianx CreditAttribution: Fabianx commentedPatch looks fine.
I had another problem here before, which I moved to a seperate issue.
Best Wishes,
Fabian
Comment #3
mkesicki CreditAttribution: mkesicki commented@Fabianx thx for notice this and patch review.
@martin_q thx for patch.
We will check this patch.
EDIT:
After check patch looks fine. One more time thx.
Comment #4
martin_qWho can we get to review and provide a third opinion?
Comment #5
duozerskThis patch looks fine to me. Not that I had some issues with security filters working wrong way, but doing it the way the core does it should be the right way to go.
Comment #6
martin_q@duozersk Thanks!
@michal_cksource There's your third. What happens now?
Comment #7
mkesicki CreditAttribution: mkesicki commentedWe will add this patch to future versions.
Comment #8
dczepierga CreditAttribution: dczepierga commentedHi,
Ok i review this patch and rewrite this, because we doesn't need to rewrite hole loop there and add new one.
All changes i commit to GIT, pls check last DEV does it works as u expect.
Greetings
Comment #9
dczepierga CreditAttribution: dczepierga commentedSry, I make a little mistake, i need add some more changes...
Comment #10
dczepierga CreditAttribution: dczepierga commentedOk i add new changes.
Pls check now last DEV version.
Greetings
Comment #11
mkesicki CreditAttribution: mkesicki commentedComment #11.0
mkesicki CreditAttribution: mkesicki commentedAdded spacing and indicated source files for the two code snippets.