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

Stacktrace when sending invalid json payload #601

Closed
clemens-tolboom opened this issue May 7, 2018 · 1 comment
Closed

Stacktrace when sending invalid json payload #601

clemens-tolboom opened this issue May 7, 2018 · 1 comment

Comments

@clemens-tolboom
Copy link
Contributor

clemens-tolboom commented May 7, 2018

Sending an invalid JSON which happened to me several times when typing by hand or forget to json encode

curl --request POST --data '{ "query": "invalid"' --HEADER 'content: application/json' --HEADER 'accept: application/json' 'http://drupal.web/graphql'

Drupal responds with a stack trace which should not happen

The website encountered an unexpected error. Please try again later.
TypeError: Argument 1 passed to Drupal\graphql\Utility\JsonHelper::decodeParams() must be of the type array, null given, called in /Users/clemens/Sites/drupal.web/web/modules/contrib/graphql/src/Routing/QueryRouteEnhancer.php on line 77 in Drupal\graphql\Utility\JsonHelper::decodeParams() (line 16 of modules/contrib/graphql/src/Utility/JsonHelper.php).

Drupal\graphql\Utility\JsonHelper::decodeParams(NULL) (Line: 77)
Drupal\graphql\Routing\QueryRouteEnhancer->extractBody(Object) (Line: 28)
Drupal\graphql\Routing\QueryRouteEnhancer->enhance(Array, Object) (Line: 259)
Drupal\Core\Routing\Router->applyRouteEnhancers(Array, Object) (Line: 130)
Drupal\Core\Routing\Router->matchRequest(Object) (Line: 90)
Drupal\Core\Routing\AccessAwareRouter->matchRequest(Object) (Line: 114)
Symfony\Component\HttpKernel\EventListener\RouterListener->onKernelRequest(Object, 'kernel.request', Object) (Line: 76)
Drupal\webprofiler\EventDispatcher\TraceableEventDispatcher->dispatch('kernel.request', Object) (Line: 127)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 68)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 57)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 99)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 78)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 40)
Drupal\jsonapi\StackMiddleware\FormatSetter->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 38)
Drupal\webprofiler\StackMiddleware\WebprofilerMiddleware->handle(Object, 1, 1) (Line: 50)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 664)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

My naive solution trying to add errors failed (again). Replacing the inline json_decode() producing a NULL value on invalid JSON

$values = array_merge($values, JsonHelper::decodeParams(json_decode($content, TRUE)));

by a more explicit encoding and adding errors for the client to easier debug their request

\Drupal\graphql\Routing\QueryRouteEnhancer::extractBody

  protected function extractBody(Request $request) {
    $values = [];

    // Extract the request content.
    if ($content = $request->getContent()) {
      $payload = json_decode($content, TRUE);
      if (empty($payload)) {
        $values = [
          'errors' => [
            'message' => json_last_error() . ' : ' . json_last_error_msg(),
          ],
        ];
      }
      else {
        $values = array_merge($values, JsonHelper::decodeParams($payload));
      }
    }

    if (stripos($request->headers->get('content-type'), 'multipart/form-data') !== FALSE) {
      return $this->extractMultipart($request, $values);
    }

    return $values;
  }

the error is not used in the response.

@pmelab pmelab closed this as completed in 4bc0530 May 17, 2018
@clemens-tolboom
Copy link
Contributor Author

Commit 4bc0530 does not report anything about the given payload errors. It only ignores it.

Please reconsider to bubble up this error somehow like I proposed through code snippet.

      $values = [
          'errors' => [
            'message' => json_last_error() . ' : ' . json_last_error_msg(),
          ],
        ];

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

1 participant