Ignore some query parameters which violate the JSON API specification

The Robustness principle recommends that we are very strict with what the JSON API module outputs, and liberal in what inputs we accept. In particular:

In other words, programs that send messages to other machines (or to other programs on the same machine) should conform completely to the specifications, but programs that receive messages should accept non-conformant input as long as the meaning is clear.

I didn't know this had a name, but this gut feeling @Wim Leers, @gdd and myself had is apparently a very real thing.

I am leaning towards option #1: We could simply not enforce it.

Thanks @gabesullice for doing what @e0ipso and I should already have done: open a new issue and discuss there.

#2933062-45: [>=8.5.4] Spec Compliance: Return 400 for unrecognized/unsupported query parameters:

What's heavy-handed about it? That's exactly what middlewares are for. I guess just that it's a lot of code to write? That's exactly what contrib is for.

I'm not a fan of having to install another module that strips out disallowed parameters, because that then means extra things to figure out and configure: developers have to know about this extra module, and configure it just the right way to strip just the right query parameters.

#2933062-46: [>=8.5.4] Spec Compliance: Return 400 for unrecognized/unsupported query parameters:

I don't agree. I would consider this legitimate if they reserved a less generic namespace. If they reserved jsonapi_* or _*, … we would not be having this conversation.

That'd mean it would not be ?filter=, but ?jsonapi_filter= or ?_filter=. So I totally get why they're being so broad; for the sake of DX of JSON API users.

#3: I'm familiar with the robustness principle — I pointed to it many times, including in the docblock of \Drupal\rest\EventSubscriber\ResourceResponseSubscriber::getResponseFormat() :)

I thought about this some more. I am very tempted to apply the robustness principle in a tentative way:

1. Ship JSON API 2.x as-is, with the strict checking.
2. If people complain, make it less strict again.

Because it seems that the most common scenario for having additional URL query parameters is proxies/CDNs that do certain things based on those query parameters. But … those proxies/CDNs should then also remove that query parameter when the request is forwarded to the origin (the Drupal site with JSON API). They should already be doing that. And if they are… then there is no problem! :D

The only non-proxy/CDN example given at #2933062-42: [>=8.5.4] Spec Compliance: Return 400 for unrecognized/unsupported query parameters is ?preview. But this is custom code in a Drupal module. So that module could quite easily be updated with a middleware to map the one (or few) specific query parameters it reacts to, to parameter names that are allowed by the JSON API spec. Then clients would not need to be updated.

In other words:

• CDN/proxy URL query parameters should not affect us at all, because they target the CDN/proxy's functionality and hence should never hit origin. If they do, it's a huge bug in the CDN/proxy.
• Drupal module-owned query parameters can easily be updated to comply, by the module owning it. Violating modules can even retain BC by mapping it early enough, in a middleware.
• The end result is that nothing needs to change for any JSON API client: CDN/proxy query parameters should already not affect us, and custom Drupal modules can easily be updated, without the need for additional Drupal modules or configuration!

Thoughts?

If people complain, make it less strict again.

That forces people to speak up and fight against the current status. That's something the vast majority of people won't do.

In any case I won't fight this either. I think it's a mistake, but admittedly it won't have a huge impact.

That forces people to speak up and fight against the current status.

I don't think people are afraid to file bug reports? (They'll perceive this as a bug.)

Only the 3 of us are following this issue. Zero bug reports even remotely related to this in nearly 5 months and 3 weeks. I think it's safe to say this is unneeded :) And we can reopen it if that need does exist in the future!

I ran into the following issue, after doing a security update to the latest version:

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

and it ended up being a silly nginx configuration error: it was using the old Drupal6 rewrite on the index.php, instead of the one for Drupal7/Drupal8 (well, an incorrect Aegir config was kicking in..).

Hi I have the same issue : #8

Can you give me the config for nginx ?

use:

rewrite ^ /index.php?$query_string last;

instead of

rewrite ^/(.*)$ /index.php?q=$1 last;

(but you may want to double-check with the various howtos/guides out there)

Note: if you're using Laravel Valet and seeing this bug. There's a PR in review here: https://github.com/laravel/valet/pull/705

Hey there, this issue actually came up recently within my internship project. Theres a quick patch you can perform on the JsonApiSpec.php to simply ignore the q parameter! Please try it out for yourself, thanks!

Patch for Drupal 9.2

Works adding the "q": #13

I'm using Drupal version 9.3.6.

Thank you @angheloko

well its kinda like this ... modules that are very opinionated and don't let people do what they need is kinda crap

so this leads to hackery like the following.

public function kernelRequest(Event $event) {
    $request = $event->getRequest();
    // json api only 
    if ($request->getRequestFormat() !== 'api_json') {
      return;
    }
    $uri = $request->getUri();
    if (str_contains($uri, 'jsonapathpart')) {
      $mycustomtoken = $event->getRequest()->query->get('mycustomtoken');
      // remove it or it jsonAPI validator will cry.
      $event->getRequest()->query->remove("mycustomtoken");
      if (empty($token)) {
        $event->setResponse(new RedirectResponse('/'));
      }
      else {
        $token_match = 'something'
        if ($mycustomtoken != $token_match) {
          // go away
          $event->setResponse(new RedirectResponse('/'));
        }
      }
    }

Rerolling #12