lando / drupal

The Official Drupal Lando plugin
https://docs.lando.dev/drupal
GNU General Public License v3.0
16 stars 16 forks source link

Lando 3.17.0 causes issue with json api module #56

Closed vermario closed 1 year ago

vermario commented 1 year ago

Hello!

The new 3.17.0 version of lando seems to have issues with the JSONAPI Drupal core module (Drupal 10, latest version).

Symptom:

Jsonapi responds with an error:

"The following query parameters violate the JSON:API spec: 'q'."

image

How to reproduce

You could reproduce this by:

  1. cloning this repository: https://github.com/wunderio/drupal-project (we use it at our company for all projects, it's been well tested)
  2. make sure you have lando 3.17 installed
  3. lando start && lando drush si -y && lando drush en jsonapi -y
  4. visit https://drupal-project.lndo.site/jsonapi and witness the error.

Now, remove 3.17.0, reinstall 3.11, lando destroy and repeat the operations above, and jsonapi will work normally.

In another project of ours, where Drupal is installed in a subdirectory, we instead get a redirect loop at /jsonapi with 3.17.0, but all normal with 3.11.

This has been confirmed by a coworker with a different development machine.

I have found this issue in Drupal.org: https://www.drupal.org/project/jsonapi/issues/2984044 which refers to some configuration in nginx that jsonapi responds to by throwing this error. Was some default nginx configuration changed in this release?

Hope this helps!

vermario commented 1 year ago

I have tested this with the newly released 3.17.1, and the same error happens.

reynoldsalec commented 1 year ago

Could be a change in the NGINX config between 3.11 and 3.17...could be good to add a test to our Drupal suite once we fix this.

reynoldsalec commented 1 year ago

I'm going to replicate this on a stock drupal10 recipe, and if I have the same issue, first try reverting this NGINX config change, which appears to be the only one made between the relevant Drupal plugin versions.

millnut commented 1 year ago

Confirming it is related to that nginx change, just creating a PR now for it

millnut commented 1 year ago

Fixed in this PR https://github.com/lando/drupal/pull/57

reynoldsalec commented 1 year ago

Ok, added this change in and released in v0.10.0 of the Drupal plugin. That'll go out in the next Lando release, until then you can always do a "custom" install of the Drupal plugin: https://docs.lando.dev/drupal/getting-started.html#custom-installation

One weird item; I wasn't able to replicate this bug in a vanilla D10 setup. It may have just been an artifact of my testing/late night, but I think @millnut replicated this at least on a different setup, so I'm pretty confident it's an issue. Would also love to improve my test to more specifically target the error case if anyone has an idea on that: https://github.com/lando/drupal/blob/v0.10.0/examples/drupal10/README.md?plain=1#L74

Thanks @vermario for the detailed report and @millnut for addressing it.