Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Autocomplete broken on Drupal 6.37 #226

Open
mmcclell opened this issue Aug 22, 2015 · 13 comments
Open

Autocomplete broken on Drupal 6.37 #226

mmcclell opened this issue Aug 22, 2015 · 13 comments

Comments

@mmcclell
Copy link

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
Copy link

We are getting the same challenge with our upgrade too.

@rfay
Copy link

rfay commented Aug 26, 2015

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

@perusio
Copy link
Owner

perusio commented Aug 26, 2015

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
Copy link

rfay commented Aug 26, 2015

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
Copy link
Owner

perusio commented Aug 26, 2015

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
Copy link

rfay commented Aug 26, 2015

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

@andythornton
Copy link

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
Copy link
Owner

perusio commented Aug 26, 2015

@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
Copy link

rfay commented Aug 26, 2015

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
Copy link

rfay commented Aug 26, 2015

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
Copy link

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
Copy link

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
Copy link
Owner

perusio commented Aug 31, 2015

@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.

wellebee pushed a commit to wellebee/drupal-with-nginx that referenced this issue Feb 24, 2016
wellebee pushed a commit to wellebee/drupal-with-nginx that referenced this issue Feb 24, 2016
wellebee pushed a commit to wellebee/drupal-with-nginx that referenced this issue Feb 24, 2016
wellebee pushed a commit to wellebee/drupal-with-nginx that referenced this issue Feb 24, 2016
wellebee pushed a commit to wellebee/drupal-with-nginx that referenced this issue Feb 24, 2016
wellebee pushed a commit to wellebee/drupal-with-nginx that referenced this issue Feb 24, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants