Overview

f
#3461499: Support complex SDC prop shapes: introduce (Storable)PropShape to compute field type storage settings added full test coverage, but exempted one prop shape:

      if ($storable_prop_shape->fieldTypeProp->fieldType === 'uri') {
        // @todo Figure out why the `uri` field type's sample value violates the JSON schema validation for `format=uri` 😬
        return;
      }

Proposed resolution

  1. Point to appropriate upstream issue.
  2. Fix erroneous early return 😱, which meant that any test case after the one that used the uri field type, would not be tested 😱

User interface changes

None.

Issue fork experience_builder-3466042

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

Wim Leers created an issue. See original summary.

Wim Leers credited f.mazeikis.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: Follow-up for #3461499: figure out why`format: uri` » Follow-up for #3461499: figure out why`format: uri` fails
Issue tags: +Needs upstream bugfix

Turns out that Felix spotted this same problem back on May 24, but only told me in a DM 😅

Felix Mazeikis
10:23 Good morning :wave:
10:24 Remember me complaining about URI validation failing on relative paths for images? I think I’ve figured out the cause
10:25 In our schema.json  https://git.drupalcode.org/project/experience_builder/-/blob/0.x/schema.json?ref_type=heads#L23 and in the SDC 'sdc_test:my-cta' that is owned by sdc_test in core https://git.drupalcode.org/project/drupal/-/blob/10.2.x/core/modules/sdc/tests/modules/sdc_test/components/my-cta/my-cta.component.yml#L19 we use format: uri constraint
10:26 That constraint get picked up by jsonrainbow/json-schema validator and hits this https://github.com/jsonrainbow/json-schema/blob/master
10:27 And while the format is for uri, the constraint uses FILTER_VALIDATE_URL php filter constant -  https://www.php.net/manual/en/filter.filters.validate.php
constraint uses FILTER_VALIDATE_URL php filter constant -  https://www.php.net/manual/en/filter.filters.validate.php
10:28 php.net has this to say about the FILTER_VALIDATE_URL
Validates value as URL (according to » http://www.faqs.org/rfcs/rfc2396), optionally with required components. Beware a valid URL may not specify the HTTP protocol http:// so further validation may be required to determine the URL uses an expected protocol, e.g. ssh:// or mailto:. Note that the function will only find ASCII URLs to be valid; internationalized domain names (containing non-ASCII characters) will fail.
faqs.orgfaqs.org
RFC 2396 - Uniform Resource Identifiers (URI): Generic Syntax (RFC2396)
RFC 2396 - Uniform Resource Identifiers (URI): Generic Syntax
10:28 I think this function does not acknowledge relative paths as correct URLs
10:32 There’s a issue thread discussing adjacent issue, where someone was using this to validate URNs and it also fails https://github.com/jsonrainbow/json-schema/issues/685
10:33 We might want to pitch in, as I imagine there’s we’re going to use something akin to format: uri a lot :thinking_face:

Wim Leers opened merge request !142

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: Follow-up for #3461499: figure out why`format: uri` fails » Follow-up for #3461499: figure out why`format: uri` fails + remove erroneous accompanying early return
Assigned: wim leers » f.mazeikis
Issue summary: View changes
Status: Active » Needs review
f.mazeikis’s picture

f.mazeikis commented
Status: Needs review » Reviewed & tested by the community

Approved - that’s a really neat solution until upstream changes happens

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Assigned: f.mazeikis » Unassigned
Status: Reviewed & tested by the community » Fixed

🚢

  • Wim Leers committed fb31e7be on 0.x 
    Issue #3466042 by Wim Leers, f.mazeikis: Follow-up for #3461499: figure...
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Assigned: f.mazeikis » Unassigned
Status: Reviewed & tested by the community » Fixed

oh d.o …

Status: Fixed » Closed (fixed)

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

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

This was only a partial solution. Complete solution: #3469421: `{type: string, format: uri}` disallows image URLs containing spaces, `uri` data type stores + returns invalid URIs!.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Version: » 0.x-dev

Do we have a follow up to update justinrainbow/json-schema as this has been fixed upstream now