Resulting string format of token_replace(..., array('sanitize' => FALSE)) is undefined

Changes like this are definitely wrong. When $sanitize is FALSE, then the caller is asking for the original, raw token values, not to be output, but used in other contexts.

I think that depends on whether $sanitize == FALSE is supposed to indicate 1) that the output is plain text, or 2) that the output is unsanitized HTML (that should be passed through filter_xss_admin(), not check_plain()). I thought there was consensus about case 1 (i.e. the behaviour is similar to the CHECK_PLAIN/PASS_THROUGH option for drupal_set_title()).

In case 2 we should run decode_entities(strip_tags()) on tokens that may contain HTML in order to convert them to plain text. We can do that, but that actually destroys information. I don't think “sanitize” is the right word to describe this process.

Or, 3) we can have a three-state option, so the function may return either sanitized HTML, unsanitized HTML or plain-text.

Can we write down a concrete use-case, in which the current $sanitize option fails?

I'm not sure we understand each other correctly. My point is that if users just gets the raw token values, he doesn't know which format they are in.

For instance, if he calls token_replace('Our slogan is [site:slogan]', array(), array('sanitize' => FALSE)), he will get an HTML string. If he need to use that in a plain-text context (e.g. in a log file or an email subject), he will need to convert it to plain text using decode_entities(strip_tags()). On the other hand, if calls calls token_replace('This is [site:name]', array(), array('sanitize' => FALSE)), he will get a plain-text string that can be used as is. If he calls token_replace($text, array(), array('sanitize' => FALSE)) where $text is some user-supplied value, he has no way of knowing whether he needs to translate the string to plain-text or not. Calling decode_entities(strip_tags()) on a string that is already plain text will not always work.

Calling token_replace('Our slogan is [site:slogan]', array(), array('sanitize' => FALSE)) for usage in a HTML context is wrong. If you want to output HTML, then you use the default value for $sanitize (TRUE).

I was talking about a non-HTML context, e.g. in a log file or an email subject.

But let's consider the example with the file field, and let's assume that file_field_widget_uri() actually passes array('sanitize' => FALSE) to token_replace() (it currently does not, but I assume it's supposed to - I have added this change to the patch):

Assume that I have a site with geek humour with the witty name “<jokes>” and the witty slogan “and <stuff>”, i.e. that's what the users will see. On the admin form I have to enter the slogan as and <stuff>, because the slogan field is supposed to contain HTML and is not check_plain()'ed before being displayed on the site. I have configured a file field to store its files in the directory [site:name]-[site:slogan]. This will store the files in http://example.com/sites/default/files/%3Cjokes%3E-and%20%26lt%3Bstuff%3E/foo.txt - in pretty-print (the text displayed in the status bar of modern browser when you hover over the link) http://example.com/sites/default/files/<jokes>-and &lt;stuff>/foo.txt.

What the user sees is the raw values, i.e. exactly what was input on the Site information page. From an end-user point of view it does not make sense, that < appears verbatim in the URL - he would expect to see the < character be encoded the same way for the name and the slogan, i.e. so that the URL would pretty-print as http://example.com/sites/default/files/<jokes>-and <stuff>/foo.txt.

Do you agree?

> The example for using the site slogan isn't really valid in this case. Although it *can*
> be used it's not a realistic example and this direction is not acceptable to me.
Some other examples: Putting the output from token_replace() in a mail subject, in a CSV file or other text file, or in a message posted on IRC.

In these cases we want the result in plain-text, not in HTML, even though some tokens contain HTML and some contain just plain text. Currently, the only way to do this is decode_entities(strip_tags(token_replace($foo, $context, array('sanitize' => TRUE))).

Example:
Create a node with the title “foo &copy; bar” and the body “bar &copy; <strong>foo”. On the node view page, the title appears as “foo &copy; bar” (i.e. exactly as entered) and the body as “bar © foo” (i.e. interpreted as HTML).

Now consider this code used to generate a text file:

$string = "Title: [node:title]\nBody: [node:body]";
$node = node_load(1);
$context = array('node' => $node);
file_put_contents('/tmp/node-a.txt', decode_entities(strip_tags(token_replace($string, $context, array('sanitize' => TRUE)))));
file_put_contents('/tmp/node-b.txt', token_replace($string, $context, array('sanitize' => FALSE)));

node-a.txt contains this:

Title: foo © bar
Body: bar © foo

This is a plain-text representation of what was shown on the node view page, so this is good.

node-b.txt contains this:

Title: foo © bar
Body: bar &copy; <strong>foo</strong>

Here the title is right, but the body contains the raw HTML source code, i.e. the resulting text file contains a mix between HTML and plain text. This is not really useful, except perhaps for generating debug output. In most other real-world use-cases you want to convert the result into some specific format, either plain-text or HTML (or some other format) independent of how the value is represented internally in Drupal.

@Damian, thank you for the clear summary.

I don't have strong opinions on whether it should be (2)+(4) or (3)+(4). My patch is implementing (3)+(4), but I'd be happy to make a new one implementing (2)+(4). My main concern is that we don't do (1).

(One year later ...)

@Damian: I agree, (2)+(4) (or posibly (2)+(3)) makes more sense.

This patch implements (2) and (4). Text passed through _text_sanitize() or check_markup() is considered safe enough (is this how we usually do?).

I also eliminated renamed the option “sanitize” to favour “html” (similar to the option for l()). I think this wording is more precise.

Reroll.

Reroll.