Problem/Motivation

The JSON API spec lists the invalid characters for member names at http://jsonapi.org/format/#document-member-names

This list includes the "Reverse Solidus" (a.k.a. backslash) as an invalid character, but the symbol printed in the docs on that line is some type of double-quote, not the backslash. As a result, the validation function and tests include the wrong character in their list of invalid characters.

This validation also doesn't check against ASCII control characters (Unicode 0x00-0x1F).

Proposed resolution

Fix this by replacing the incorrect character with a backslash character. Also add checks against ASCII control characters.

CommentFileSizeAuthor
#2 validation_of_parameter-2844717-2.patch1.4 KBhampercm

Comments

hampercm created an issue. See original summary.

hampercm’s picture

hampercm commented
Assigned: hampercm » Unassigned
Status: Active » Needs review
StatusFileSize
new validation_of_parameter-2844717-2.patch1.4 KB
dawehner’s picture

dawehner
Primary language German
commented 
+++ b/src/Access/CustomParameterNames.php
@@ -42,7 +42,7 @@ class CustomParameterNames implements AccessInterface {
     foreach (array_keys($json_api_params) as $name) {
-      if (strpbrk($name, '+,.[]!”#$%&’()*/:;<=>?@^`{}~|')) {
+      if (strpbrk($name, "+,.[]!”#$%&’()*/:;<=>?@\\^`{}~|\x0\x1\x2\x3\x4\x5\x6\x7\x8\x9\xA\xB\xC\xD\xE\xF\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1A\x1B\x1C\x1D\x1E\x1F")) {
         $valid = FALSE;
         break;

I looked out for some other implementation of this. Observations: a) there are a couple of people who have installed the jsonapi module in the wild. b) Some implementation know about the standard but then fail quite a bit: https://github.com/erickt/jsonapi/blob/1ebd4e4bc8ff6302eb8a499aaceb6ef72... https://github.com/automatonic/japi/blob/7249d06007a327f935982f21ad02aee... https://github.com/trailblazer/roar-jsonapi/blob/adfc2ac4f6c4361c519efa7... https://github.com/UpstateRuby/openworks-status-api/blob/88a4fe948d38a61...

We should ideally document this particular line by maybe moving the reference to the reference from the test over here to the implementation.

hampercm’s picture

hampercm commented

The reference to the official standard is added to this class by #2844373: Address core feedback 2843147#comment-11872253

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Reviewed & tested by the community

I tried to write a regex or so to produce the same kind of value, but I couldn't. I'm glad we have test coverage now!

  • e0ipso committed 7cddb33 on 8.x-1.x authored by hampercm 
    fix(Spec) Validation of parameter names doesn't correctly check for...
e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
commented

This looks great! Merging.

e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
commented
Status: Reviewed & tested by the community » Fixed
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Related issues: +#2829328: Clean up Drupal\jsonapi\Access\CustomParameterNames and make it follow the spec more closely

Also see #2829328-12: Clean up Drupal\jsonapi\Access\CustomParameterNames and make it follow the spec more closely, which further expands this.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.