perusio / drupal-with-nginx

Running Drupal using nginx: an idiosyncratically crafted bleeding edge configuration.
854 stars 246 forks source link

Autocomplete broken on Drupal 6.37 #226

Open mmcclell opened 9 years ago

mmcclell commented 9 years ago

After upgrading from Drupal 6.36 to 6.37, typing in form elements that have an autocomplete started causing a 404 error popup. The AJAX request is now using a non-clean URL, but I'm on the D6 branch of your config which has this in drupal6.conf:

location = /index.php { internal;

Commenting out "internal;" allows it to work again. It looks like 6.37 requires non-clean URLs to work as part of a security fix. From includes/form.inc:

// Force autocomplete to use non-clean URLs since this protects against the // browser interpreting the path plus search string as an actual file.

andythornton commented 9 years ago

We are getting the same challenge with our upgrade too.

rfay commented 9 years ago

This is critical - breaks all sites that use autocomplete on D6 (and I assume D7). Thanks!

perusio commented 9 years ago

Ok. If the problem is only with thw Ajax calls is quite simple to solve. Add the following location to the drupal.conf and/or drupal6.conf file:

location ^~ /system/ajax {
    include apps/drupal/fastcgi_drupal.conf;
    fastcgi_pass phpcgi;
}
rfay commented 9 years ago

No, this is a problem with all autocomplete. So for example, the node.module autocomplete URL is going to be index.php?q=user/autocomplete

I don't think that system/ajax is affected or used by any autocomplete stuff, but could be wrong. And I don't think that standard ajax is affected anywhere by this change.

From modules/node/node.module:

    $form['owner_name'] = array(
      '#type' => 'textfield',
      '#title' => t('Username'),
      '#default_value' => $owner_name,
      '#autocomplete_path' => 'user/autocomplete',
      '#size' => '6',
      '#maxlength' => '60',
      '#description' => $description,
    );

So in that case, Drupal 6.37 converts the clean-url user/autocomplete/xxx into index.php?q=user/autocomplete/xxx

perusio commented 9 years ago

Well then the problem is different because we need to match the $arg_q variable with anything ending with /autocomplete. Hmm. The quick temporary fix is to comment out the internal line like @mmcclell suggested above. I have to install the latest Drupal 6 and see what's going on. In Drupal 6 the conversion from clean to non-clean happens in the configuration.

rfay commented 9 years ago

No, things don't necessarily end in /autocmplete. The form api #autocomplete_path value is free form, so can be anything.

andythornton commented 9 years ago

for what it is worth ... we tried this on one of our Pantheon sites (they host 75,000+ Drupal sites) and it was not a problem. I presume in their Nginx configs they don't restrict access to index.php with the 'internal' modifier. I don't pretend to fully understand the magic you do with these configs (I am a Drupal dev), but it seems like permitting q= access would be fine ... dunno if you can do that without giving full access to index.php, though?

perusio commented 9 years ago

@andythornton @rfay the internal keyword is a nginx special directive that only allows access to a given location (URL) from whitin the server. You won't be able to access index.php directly in an HTTP client, but internal redirections and the application can.

It's a security measure to forbid access to index.php so that people can't try to play with it directly.

rfay commented 9 years ago

Well... commenting out the internal keyword solves the problem, even though it's the browser that's doing the access via js. Hmm.... With "internal" commented out I can access index.php?q=location_autocomplete/us on my server from a browser.

In a normal plain vanilla drupal install you can test this just by changing the owning user of a node - it uses user/autocomplete for that.

I'm using nginx 1.8.0-1~precise on ubuntu 12.04.

rfay commented 9 years ago

By the way, @perusio - off topic, but you're awesome. Every time I work on nginx I thank you for your configs and your maintenance of them. It makes life so good. THANKS!

geckolinux commented 9 years ago

I can confirm that removing internal fixes it.

What are the security implications now with this disabled?

Thanks @perusio for the help with this!

heidiblobaum commented 9 years ago

Just joining in as our D6 sites are affected by this as well. With core functionality broken, we're struggling with either needing to rollback the security update -- which Drupal indicates is a "critical" security risk -- or implementing this "internal" fix which has other security implications. What is the best decision for us to make in this kind of scenario? Thanks for everyone's research and input, and especially @perusio !

perusio commented 9 years ago

@sb56637 @heidiblobaum there's no great issue in commenting out the internal directive. Don't rollback. This directive is just a nice to have thing.