Various improvements

I like the improvements list. Haven't had a chance to test the patch yet.

Like midkemia, I think the argument format change has the danger of being very disruptive for existing sites. I'd support the change if we had legacy handling for the old argument type, but then, we're perhaps adding complexity and change without a strong need for change. Any other users want to weigh in here?

Perhaps the best course of action would be to move #2 to its own issue, where it will have an opportunity to get more attention and comments in the issue queue, and then we can proceed here with reviewing 1, 3, 4, and 5.

@Pasqualle

The complexity I was thinking of had more to do with confusion and support requests in the issue queue. :D

As long as the README.txt and the project page here on d.o are updated to reflect these changes, I don't see there being a problem with proceeding once we have a review or two of the current patch.

I tried using the patched module, but my insert views simply stopped inserting (which is a pity, as I thought it looked like a good improvement). Not sure why they stopped working yet.

Could improvement #2 be made a configurable option? I support changing the argument separator to match views.

Not sure about this as I am pretty new to Drupal but could the module support 2 tag types.

At the moment all tags begin [view:......

ie: [view:name of view=x=arg1,arg2,arg3]

What about another tag that begins [view2:....

[view2:name of view=x=arg1/arg2/arg3] (Using new argument format)

Andy

Can I please suggest adding support for escaping the argument separator? There's a simple example at http://drupal.org/node/85771#comment-158116 .

Doesn't this code mean that ',' are replaced by '/'?

      if (strpos($view_args, '/') === FALSE) {
        $view_args = str_replace(',', '/', $view_args);
      }

So now if your argument contains either character then it will be treated as multiple arguments? This means that adding an extra separator character makes the problem worse rather than better because you now have 2 characters that can't be used in an argument. :-)

The views preview interface seems to have a similar problem.

For example, say that my view displays nodes of a particular content type whose node title matches a single argument. If I have a node with title "A, B" then I can't currently match it using insert_view 6.x-1.0. In your proposed version I still can't use insert_view to match that node and I also can't match a node with title "X/Y".

You need a way of telling insert_view that a ',' or '/' should be treated as a character that is part of an argument instead of a character that is used to split multiple arguments. One way of doing this would be to add an escape character like the example I linked to above.

Another way would be to make the argument separator configurable per use of insert_view. The cleanest way I can think of doing this would be something like this example:

[view:@my_view=default=arg1@arg2]

That is, if you put a character that isn't allowed in a view name at the beginning of the view name, then that character is used as the argument separator. I haven't though about how this might interact with the PHP filter if you allowed it in the same input filter as the insert_view filter... but if you're allowing PHP then you probably don't need insert_view. ;-)

The views preview interface has a similar problem but the problems are independent.

When insert_view does this

$args = $args ? explode('/', $args) : array();

it splits on every '/' in the argument(s). There is no way to include a '/' in an argument, so you can't possibly pass an argument that matches a field that has a value containing '/'. To do this you would need to be able to escape a slash. For example, arg1part1\/arg1part2/arg2.

I partly agree. So, here's my last throw of the dice... :-)

insert_view parses the arguments into an array so it has the opportunity to decide how that parsing should be done.

If I resort to PHP and call views_embed_view() directly then I can pass arguments containing any characters, because those arguments are not parsed. So the problem is not inherent in views.

It makes sense to allow people to pass any string as an argument, even if that string contains a metacharacter. Most software allows metacharacters to be escaped.

Perhaps the real solution is to use URL encoding. In that case, perhaps views needs to decode such encoded characters in arguments before building the query? Perhaps the decoding should be optional per argument?

I've applied the patch and the module has stopped working altogether, instead outputting clear text on page view:

[view:emerg_contacts_lerp_display=page=Facilities & Services Division]

The argument type is set in my view to "Term name/synonym converted to Term ID"

Hi Pasqualle,

This is on a page that shows the view inline correctly, until I apply the patched version of the module, if I revert back the view is displaying inline correctly again.

So there are definitely records that get returned with the argument.

Subscribing.

I find it to be a pretty useful module however the documentation is pretty confusing and so is the example given in the read me text. All I need to do is pass on an argument using the url . However I am not able to figure out how ..

The syntax i m using : [view:tracker=page=1] works when "1" here is the value of the argument being passed. However I want to pass the argument "user ID" dynamically which ideally should be taken from the url and not supplied as value. Please help.. Any suggestions would be appreciated.

Hi, and thank you for the module.

Is there any updates since October 13, 2009, especially for access check for views?

I think this is an important security issue when users can insert views that have been created for admins (content list, users list, VBO).

I'm also wondering about this module's status, especially now that it is listed as an unsupported module. Just started using this module and it's doing something I haven't been able to figure out otherwise.

if you're currently using this module in a selected number of nodes that you edit with high privileges (e.g. site administrator) then you can replace it with the use the "PHP Code" input format and insert the view directly with PHP:

print views_embed_view("my_view", $display_id = 'default', $node->title)

Note that you do not want to allow the "PHP Code" input format on nodes that untrusted users can edit. That would allow those users to use PHP code to expose any information on your site.

I use this technique to add a view near the top of my front page via a sticky node that allows "PHP Code". One day I'll use Panels to do this instead... but that will require having the time to learn about Panels. ;-)

is the security warning on the front page pertaining to the 2.x-dev version as well? or this one is secured?

Hi,
I tested one inserted view for its access by anonymous with 6.x-2.x-dev 2010-Mar-05 and it checks the access (anonymous don't see the view). Thank you for the help.

Here are the steps that I followed:
Edit a view.
Under Basic Settings, there is a field called Access. I added the Admin Role only.

Results:
Users under the Admin role can see the view, but other users (ex. anonymous) cannot.

The access check appears to be working!
Thanks for the fix!

Subscribing. Thanks for the fix!

I just upgraded my dev site... I thought I only need to change the syntax if I use multiple arguments? Now none of my inserted views show up at all, even ones that use no arguments.

For example, this one doesn't work and I don't know what needs to change:

[view:staff=Page]

Detailed new syntax documentation to go with this new version would help.

Just updated, and of course had to find all the , arguments on my site and change them to / arguments.

Whipped up a quick views filter that finds nodes that have [view: plus a second, settable argument (which you set to ",0" through ",9") and outputs a table of node titles and edit links.

Generates some false positives, of course, but it's not too much bother to use browser search to look through the edit form and see if anything needs to be tweaked.

Posting it in case anyone else might find it useful:

$view = new view;
$view->name = 'Insert_View_Finder';
$view->description = 'Insert View Finder';
$view->tag = 'Insert View Finder';
$view->view_php = '';
$view->base_table = 'node';
$view->is_cacheable = FALSE;
$view->api_version = 2;
$view->disabled = FALSE; /* Edit this to true to make a default view disabled initially */
$handler = $view->new_display('default', 'Defaults', 'default');
$handler->override_option('fields', array(
  'title' => array(
    'label' => 'Title',
    'alter' => array(
      'alter_text' => 0,
      'text' => '',
      'make_link' => 0,
      'path' => '',
      'link_class' => '',
      'alt' => '',
      'prefix' => '',
      'suffix' => '',
      'target' => '',
      'help' => '',
      'trim' => 0,
      'max_length' => '',
      'word_boundary' => 1,
      'ellipsis' => 1,
      'strip_tags' => 0,
      'html' => 0,
    ),
    'empty' => '',
    'hide_empty' => 0,
    'empty_zero' => 0,
    'link_to_node' => 0,
    'exclude' => 0,
    'id' => 'title',
    'table' => 'node',
    'field' => 'title',
    'relationship' => 'none',
  ),
  'edit_node' => array(
    'label' => 'Edit link',
    'alter' => array(
      'alter_text' => 0,
      'text' => '',
      'make_link' => 0,
      'path' => '',
      'link_class' => '',
      'alt' => '',
      'prefix' => '',
      'suffix' => '',
      'target' => '',
      'help' => '',
      'trim' => 0,
      'max_length' => '',
      'word_boundary' => 1,
      'ellipsis' => 1,
      'strip_tags' => 0,
      'html' => 0,
    ),
    'empty' => '',
    'hide_empty' => 0,
    'empty_zero' => 0,
    'text' => '',
    'exclude' => 0,
    'id' => 'edit_node',
    'table' => 'node',
    'field' => 'edit_node',
    'relationship' => 'none',
  ),
));
$handler->override_option('filters', array(
  'body' => array(
    'operator' => 'contains',
    'value' => ',0',
    'group' => '0',
    'exposed' => TRUE,
    'expose' => array(
      'use_operator' => 0,
      'operator' => 'body_op',
      'identifier' => 'body',
      'label' => 'Node: Body contains this and [view:',
      'optional' => 0,
      'remember' => 0,
    ),
    'case' => 1,
    'id' => 'body',
    'table' => 'node_revisions',
    'field' => 'body',
    'relationship' => 'none',
  ),
  'body_1' => array(
    'operator' => 'contains',
    'value' => '[view:',
    'group' => '0',
    'exposed' => FALSE,
    'expose' => array(
      'operator' => FALSE,
      'label' => '',
    ),
    'case' => 1,
    'id' => 'body_1',
    'table' => 'node_revisions',
    'field' => 'body',
    'relationship' => 'none',
  ),
));
$handler->override_option('access', array(
  'type' => 'none',
));
$handler->override_option('cache', array(
  'type' => 'none',
));
$handler->override_option('items_per_page', 20);
$handler->override_option('style_plugin', 'table');

thanks, updated and everything's working now. I just needed to use the machine-readable display names.

Why don't you use views_embed_view? It has access checking, too.

Based on http://drupal.org/node/419880#comment-2681200, is this issue considered fixed?

Since the project page says that there is a vulnerability, where do things stand with this open issue? Has this been fixed?

The project homepage has to be updated, there's still the syntax [view:name of view=name of display=arg1,arg2,arg3]

Echoing naero's comment in #41, is this committed to 2.x-dev now, and is the module secure?

Will there soon be a recommended 2.x release?

Subscribing.

subscribing