I am using views_data_export to create a CSV download with the Exposed Filters form in a block. One of the exposed filters is a Select widget (for content type). I get an error message at the top of my file when I download it.

I think the problem is that the value of the Select widget is an array, and the code expects a string (as for a text box/area). It passes that value through check_plain(), which hands it off to htmlspecialchars(), generating the error.

I will supply a patch as soon as I have an issue number.

Update (2014-06-20):

The original patch (#1) was committed to the 7.x-3.x branch (#2) and backported to the 6.x-2.x branch (#4).

The original patch assumes that the option is a string or an array of strings. Later comments point out that it can get worse than that. The patch in #12 deals with more cases and was marked RTBC (#13, #14, #16). The patch in #15 aims to deal with any level of arrays, but (according to #16) has trouble with simple strings.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

benjifisher’s picture

Status: Active » Needs review
FileSize
1.1 KB

The attached patch fixes the problem for me. I am not sure what effect it will have on other file formats, or if the Exposed Filter form is not in a block. My guess is that the file format will not matter, since add_http_headers() adds the filter information as tokens in the 'Content-Disposition' header, which (I guess) is independent of the format.

I am also not sure whether the option can be anything other than a string or an array. At least, I think this patch is a step in the right direction.

Steven Jones’s picture

Status: Needs review » Fixed

Thanks very much for the patch, fixed in 7.x-3.x.

Steven Jones’s picture

Version: 7.x-3.x-dev » 6.x-2.x-dev
Status: Fixed » Patch (to be ported)

Needs backport

Steven Jones’s picture

Status: Patch (to be ported) » Fixed

Pushed in 6.x-2.x

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

sinn’s picture

Status: Closed (fixed) » Needs work

#1 doesn't work with the between-date-exposed-filter because option in that case is like

array('value' => array('date' => '2013-06-28'),
    'min' => array('date' => '2013-06-28'),
    'max' => array('date' => '2013-06-28'),
);

Code:

  $option = implode('--', $option);

causes error in that case: "Array to string conversion in views_data_export_plugin_style_export->add_http_headers() (line 330 of D:\xampp54\htdocs\specsavers.localhost\brand2\code\sites\all\modules\contrib\views_data_export\plugins\views_data_export_plugin_style_export.inc)."

https://drupal.org/node/1298814#comment-6870686 works in that case.

benjifisher’s picture

Status: Needs work » Needs review
FileSize
1.48 KB

@sinn, thanks for pointing this out. I guess if I had looked harder in the issues queue, I would have found #1298814: PHP warning when exposed filters contain non-scalar data. (now closed as a duplicate of this issue) instead of opening this one.

I am not too surprised that the patch in #1 fails to cover some cases: see my comment there.

The latest patch in the other issue has some funkiness:

            foreach($filter_data as $key=>$data) {
              $data = is_array($data) ? implode('-', $data) : $data;
              $filter_data = $key . '-' . $data;
            }

Even if PHP allows it, I do not like replacing an array while iterating over it, but surely this snippet does not do what we want: the effect is to set $filter_data to $key . '-' . $data for the last entry of the original array, discarding the results of earlier entries. Also (and the first thing I noticed) it violates Drupal coding standards: => should have a single space on either side.

Please test the attached patch. It could be more concise, but I think it is pretty clear. I tested it on my site, but not with a date-range filter.

Note that my patch ignores $key unless $data is an array. I do not feel strongly about this: if you think the key should be included, then I can roll a new patch.

Are we confident that "array of array of strings" is as bad as it can get? Maybe we should do something with array_walk_recursive() in order to future-proof this bit of code.

JvE’s picture

Version: 6.x-2.x-dev » 7.x-3.x-dev
Status: Needs review » Needs work

I am using patch #7 on one of my sites now and it does fix the notice.

I do not know if the resulting $tokens string is useable though.

Example:
One date field "field_date" and one date-range field "field_date_range".

$tokens['%exposed'] with only range filled:
field_date_value_value----field_date_range_value_min-2011-7-9--max-2012-5-16
$tokens['%exposed'] with only date filled:
field_date_value_value-2012-6-10-field_date_range_value_min-----max---
$tokens['%exposed'] with neither filled:
field_date_value_value----field_date_range_value_min-----max---

Perhaps an array_filter() is needed on $data before implosion?

koppie’s picture

Status: Needs work » Reviewed & tested by the community

Patch in #7 works for me. (Patch in #1 did not.) Note that the new patch needs to be run against the dev version of the module. I'm marking this RTBC and switching the version back to 7.x-3.x.

Any word on when we can expect to see a new stable version?

Thanks!

koppie’s picture

Whoops sorry - our ticket updates crossed in the blogosphere. Can I leave this RTBC or does it need more work?

benjifisher’s picture

If I remember correctly, the token is used to create a default file name for the download. It is configurable to some extent. It is not useful for me.

If that is right, then the problem in #8 is an annoyance, not a serious problem. Unless you need to be able to parse the file name ...

JvE’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
1.49 KB

The same patch from #7 with a filter for empty values.

$tokens['%exposed'] with only range filled:
field_date_value_value--field_date_range_value_min-2011-7-9--max-2012-5-16
$tokens['%exposed'] with only date filled:
field_date_value_value-2012-6-10-field_date_range_value_min---max-
$tokens['%exposed'] with neither filled:
field_date_value_value--field_date_range_value_min---max-

Jelle_S’s picture

Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

Patch #12 works for me

NancyDru’s picture

Seems to take care of me.

michfuer’s picture

I feel like recursion is appropriate here since I never can anticipate the depth of a particular field/filter used by Views. This patch doesn't do anything the others don't, just takes a recursive approach.

drenton’s picture

Patch #12 works for me.
#15 does not work for string only filters.

benjifisher’s picture

Issue summary: View changes

I am updating the issue summary to summarize the recent activity.

@drenton, can you give steps to reproduce the problem using the patch in #15?

drenton’s picture

Any string only exposed filter should reproduce the problem.

I created a filter for :

Content: Author uid (exposed)

Here is the view export :


$view = new view();
$view->name = 'test';
$view->description = '';
$view->tag = 'default';
$view->base_table = 'node';
$view->human_name = 'test';
$view->core = 7;
$view->api_version = '3.0';
$view->disabled = FALSE; /* Edit this to true to make a default view disabled initially */

/* Display: Master */
$handler = $view->new_display('default', 'Master', 'default');
$handler->display->display_options['title'] = 'test';
$handler->display->display_options['use_more_always'] = FALSE;
$handler->display->display_options['access']['type'] = 'perm';
$handler->display->display_options['cache']['type'] = 'none';
$handler->display->display_options['query']['type'] = 'views_query';
$handler->display->display_options['exposed_form']['type'] = 'basic';
$handler->display->display_options['pager']['type'] = 'full';
$handler->display->display_options['pager']['options']['items_per_page'] = '10';
$handler->display->display_options['style_plugin'] = 'table';
$handler->display->display_options['style_options']['columns'] = array(
  'title' => 'title',
);
$handler->display->display_options['style_options']['default'] = '-1';
$handler->display->display_options['style_options']['info'] = array(
  'title' => array(
    'sortable' => 0,
    'default_sort_order' => 'asc',
    'align' => '',
    'separator' => '',
    'empty_column' => 0,
  ),
);
/* Field: Content: Title */
$handler->display->display_options['fields']['title']['id'] = 'title';
$handler->display->display_options['fields']['title']['table'] = 'node';
$handler->display->display_options['fields']['title']['field'] = 'title';
$handler->display->display_options['fields']['title']['label'] = '';
$handler->display->display_options['fields']['title']['alter']['word_boundary'] = FALSE;
$handler->display->display_options['fields']['title']['alter']['ellipsis'] = FALSE;
/* Filter criterion: Content: Author uid */
$handler->display->display_options['filters']['uid']['id'] = 'uid';
$handler->display->display_options['filters']['uid']['table'] = 'node';
$handler->display->display_options['filters']['uid']['field'] = 'uid';
$handler->display->display_options['filters']['uid']['value'] = '';
$handler->display->display_options['filters']['uid']['exposed'] = TRUE;
$handler->display->display_options['filters']['uid']['expose']['operator_id'] = 'uid_op';
$handler->display->display_options['filters']['uid']['expose']['label'] = 'Author uid';
$handler->display->display_options['filters']['uid']['expose']['operator'] = 'uid_op';
$handler->display->display_options['filters']['uid']['expose']['identifier'] = 'uid';

/* Display: Page */
$handler = $view->new_display('page', 'Page', 'page');
$handler->display->display_options['path'] = 'test';

/* Display: Data export */
$handler = $view->new_display('views_data_export', 'Data export', 'views_data_export_1');
$handler->display->display_options['pager']['type'] = 'some';
$handler->display->display_options['style_plugin'] = 'views_data_export_csv';
$handler->display->display_options['style_options']['provide_file'] = 1;
$handler->display->display_options['style_options']['parent_sort'] = 0;
$handler->display->display_options['style_options']['quote'] = 1;
$handler->display->display_options['style_options']['trim'] = 0;
$handler->display->display_options['style_options']['replace_newlines'] = 0;
$handler->display->display_options['style_options']['header'] = 1;
$handler->display->display_options['style_options']['keep_html'] = 0;
$handler->display->display_options['path'] = 'testcsv';
$handler->display->display_options['displays'] = array(
  'page' => 'page',
  'default' => 0,
);

Steven Jones’s picture

Status: Reviewed & tested by the community » Needs work

Setting back to needs work, as there seems to be some disagreement about whether it works or not.

JvE’s picture

#12 works in all cases so far reported.

#15 adds support for multidimensional select options but removes the check to see if $options is an array or not before treating it as one.

@michfuer: when is $option a multidimensional array? Could you provide an example view?