ctools_context_entity_convert() sanitize (process plaintext) tokens.

So, the problem here is that it's very unclear what the result of ctools_context_keyword_substitute() is: an HTML string or plaintext?

In the current situation, ctools_context_entity_convert() calls token_generate() without 'sanitize' => FALSE, which means that the result is going to be returned as an HTML string, and then potentially double encoded when used in a title or in the breadcrumb links generated by panels_breadcrumb.

In the patch attached in the original post, @bennetteson suggests that ctools_context_keyword_substitute() should return a plain-text string by default, but supports an option that allow the caller to decide it wants to process the string as plaintext.

Could we get a review on the principle of this patch?

Ok, the principle of the patch is okay.

We have to be very careful in checking all uses of this function to see if there is a potential security issue, which is going to be somewhat time consuming, but luckily I think it is not actually called in that many places so it hopefully will not be a big deal. And I believe most of the places we are seeing this we will see it already sanitized. The idea is that anything that can have a token in it is user input, and therefore MUST be sanitized after the token replacement. This is perfectly valid behavior and I think is the correct behavior.

In a minor nit, this patch unnecessarily removes whitespace that I prefer. I tend to add a lot of extra CRs between code blocks in order to facilitate readability.

Actually, I now realize this is not the proper approach.

The token system in Drupal 7 does *not* support returning plaintext tokens. It only support two things:

• HTML-encoded strings (when 'sanitize' = TRUE)
• Raw tokens (when 'sanitize' = FALSE), which is *not* plaintext but rather "whatever form the source thing is stored in")

Which means that if you want plaintext, you don't have any other solution then calling token_replace() with 'sanitize' = TRUE and you need to convert the resulting HTML to plaintext yourself (whether using drupal_html_to_text() or strip_tags(decode_entities($text))).

Hello,

using the "Menu Token" module, i have the same problem with my user menu.
This menu is something like : "Hello Bob El'roy".

Could this patch do something for me or must I look in another way?

Thanks.

I'm not sure to understand.

After aplying the patch in #9 to ctools, I still need to find a patch for Menu Token?

I actually need this patch for other reasons, but Damien is right. Just using 'sanitize' => FALSE is not necessarily what you want. Interesting.

@@ -265,8 +265,9 @@ function ctools_context_entity_convert($context, $type) {
   }
 
   $tokens = token_info();
+  $options += array('sanitize' => TRUE);
 
-  $values = token_generate($token, array($type => $type), array($token => $context->data));
+  $values = token_generate($token, array($type => $type), array($token => $context->data), $options);
   if (isset($values[$type])) {
     return $values[$type];
   }

The $options addition is unnecessary there because token_generate already has that exact same line. So it is completely redundant in every case unless token_generate changes its default in the future, which it would never do because of the security implications.

Otherwise I think I am going to use this, as it is valuable.

Committed and pushed with the change I made above.

No no. What I was saying was that the $options += array('sanitize' => TRUE) is unnecessary -- token_generate has that exact line of code. So omitting the line is the exact same result.

Is this what I committed?

http://drupalcode.org/project/ctools.git/commit/e33eb1da18f81ae275eeb6ce...