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

URI schemas incorrectly validated with FILTER_VALIDATE_URL #685

Open
ycecube opened this issue Apr 4, 2022 · 8 comments
Open

URI schemas incorrectly validated with FILTER_VALIDATE_URL #685

ycecube opened this issue Apr 4, 2022 · 8 comments

Comments

@ycecube
Copy link

ycecube commented Apr 4, 2022

Hi,

The uri, uriref and uri-reference type of fields are incorrectly being validated with filter_var($element, FILTER_VALIDATE_URL); as this php function validates only URLs and it does not (cannot) validate URIs.

So while the https://example.com is a valid URL the urn:oasis:names:specification:docbook:dtd:xml:4.1.2 is not, however it is a valid URI. Therefore these kind of fields should be validated differently, php does not have built-in functionality to do this.
In the related php ticket it is being mentioned that URIs could be validated simply as <scheme>:<extra> or implement RFC 3986.

Example JSON document that can cause a validation error.

{
  "openapi": "3.0.2",
  "info": {
    "title": "Example",
    "version": "1.0.0"
  },
  "paths": {
    "/example": {
      "post": {
        "requestBody": {
          "content": {
            "application/xml": {
              "schema": {
                "$ref": "#/components/schemas/exampleXml"
              }
            }
          }
        },
        "responses": {
          "200": {
            "description": "Success"
          }
        }
      }
    }
  },
  "components": {
    "schemas": {
      "exampleXml": {
        "type": "string",
        "xml": {
          "name": "Document",
          "namespace": "urn:isbn:0451450523"
        }
      }
    }
  }
}

Example script to reproduce the issue.

<?php declare(strict_types = 1);

require_once('vendor/autoload.php');

use JsonSchema\Validator;

$data = json_decode(file_get_contents('test.json'));
$schema = json_decode(file_get_contents('https://raw.githubusercontent.com/OAI/OpenAPI-Specification/main/schemas/v3.0/schema.json'));

// Validate.
$validator = new Validator();
$validator->validate($data, $schema);

if ($validator->isValid()) {
  echo "The supplied JSON validates against the schema.\n";
}
else {
  echo "JSON does not validate. Violations:\n";
  foreach ($validator->getErrors() as $error) {
    printf("[%s] %s\n", $error['property'], $error['message']);
  }
}

The validation error is: [components.schemas.exampleXml.xml.namespace] Invalid URL format.

(Unrelated but there is also a [components.schemas.exampleXml.$ref] The property $ref is required validation error, however it is not being marked as an issue for example by https://editor.swagger.io/)

Related:

@DannyvdSluijs
Copy link
Collaborator

I've done some research in an attempt to move the issue forward.

It seems the issue report is valid and a fix is desirable. PR's are welcome.

@erayd
Copy link
Contributor

erayd commented Feb 8, 2024

...can be challenging due to PHP version constraints, given this lib still supports PHP >=5.3.3

@DannyvdSluijs For what it's worth, if you can find a suitable library that has >=5.6 compat, that will be OK. I think we can safely drop support for versions older than 5.6.

@boobaa
Copy link

boobaa commented Apr 29, 2024

Hi @DannyvdSluijs ,

I see that this issue was scheduled for triage. May I ask an update on your findings and decisions, please? It is a blocker to us for a long time. Is there a clear path to move forward based on your findingins in #685 (comment)? If yes, Pronovix might be able to help with raising a PR.

@mxr576
Copy link

mxr576 commented Apr 29, 2024

Putting a dependency in place such as https://github.com/thephpleague/uri can be challenging due to PHP version constraints, given this lib still supports PHP >=5.3.3

What about Guzzle's PSR7 library? >=2.4.5's current support range is php: ^7.2.5 || ^8.0. Would that be a better alternative?

https://packagist.org/packages/guzzlehttp/psr7

Based on my dummy test, it also parses the URI in the bug report correctly:

  $uri = new \GuzzleHttp\Psr7\Uri('urn:isbn:0451450523');
  echo (string) $uri;

@DannyvdSluijs
Copy link
Collaborator

@boobaa thanks for the ping. The state of this issue is Accepting PR's as mentioned in my latest comment.
In all honestly I can't guarantee that PR's can be merged at this point in time. I know Erayd has seen my Triage #716 and he has been closing quite some as per my suggestions. We still have a way to go before we can consider this project to be really active again, for me personally we are still in the "reboot"-phase.

I do think we are making progress, three weeks ago 31 issues where closed. Maybe we are not yet at the speed we would like to be but I'll trust that time will resolve that. In the meantime feel free to help with the research, search for options or even go as far and create a PR if you feel comfortable enough. I'm available for reviewing and helping out.

The input from @mrix seems helpfull and can be a step in the right direction, combined with our (current) desire to support older version PHP maybe it is worth looking into the 1.x branch of GuzzleHttp if that could help?

@mxr576
Copy link

mxr576 commented Apr 30, 2024

combined with our (current) desire to support older version PHP maybe it is worth looking into the 1.x branch of GuzzleHttp if that could help?

Well, at least Guzzle PSR7 1.x is still supported in some sense... So if this lib would support both 1.x and 2.x at the same time, that could lead to a solution where both older PHP versions are supported and up to date applications can depend on justinrainbow/json-schema without any dependency issues.

Version Status PHP Version
1.x Security fixes only >=5.4,<8.1

What is the lowest PHP version that this lib would like to support, and why? Due to Zend LTS versions?

@DannyvdSluijs
Copy link
Collaborator

This repo is a dependency of Composer.
As mentioned by Seldaek (Jordi Boggiano) in #706 (comment) we can bump to PHP 7.2+ but that would be a whole project on its own. We are still supporting PHP 5.3 which makes the v1 of GuzzleHttp/PSR7
I don't know the license of GuzzleHttp but perhaps we could copy over the code and tweak it every so slightly to becomes PHP 5.3 compatible (if even needed at all)

Supporting both 1.x and 2.x would be fine I guess as long as this specific class behaves the same and the code in this repo doesn't have to be aware of any v1 or v2 things.

@wimleers
Copy link

wimleers commented Aug 5, 2024

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

6 participants