Skip to content

$data reference is not supported #1

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

Closed
thatside-zaraffa opened this issue Dec 27, 2017 · 12 comments
Closed

$data reference is not supported #1

thatside-zaraffa opened this issue Dec 27, 2017 · 12 comments

Comments

@thatside-zaraffa
Copy link

thatside-zaraffa commented Dec 27, 2017

It is not possible to use $data reference in schema like:

"password": {
      "type": "string"
  },
  "passwordRepeat": {
      "type": "string",
      "const": { "$data": "#/register/password" 
}

AJV already supports this: ajv-validator/ajv#289
Discussion on this feature in JSON Schema repo: json-schema-org/json-schema-spec#51

@thatside-zaraffa
Copy link
Author

Though this feature is not present in any of drafts yet it would be very useful in combination with AJV to implement shared back-and-front data validation.

@sorinsarca
Copy link
Member

This implementation already supports a non-standard but more complex type of validation with $filters keyword. Using $filters you can add your own validation logic, for example to check if an id is present in a database. You can use $filters to validate your schema, here is an example on how to add and use a filter:

<?php

use Opis\JsonSchema\{
    Validator,
    ValidationResult,
    ValidationError,
    FilterContainer,
    IFilter
};

$filters = new FilterContainer();

$filters->add("string", "sameAs", new class implements IFilter {
    /**
     * @inheritDoc
     */
    public function validate($data, array $args): bool {
        return isset($args['value']) && $data === $args['value'];
    }
});

$validator = new Validator();
$validator->setFilters($filters);

$schema = <<<'JSON'
{
    "type": "object",
    "properties": {
        "password": {
            "type": "string"
        },
        "passwordRepeat": {
            "type": "string",
            "$filters": {
                "$func": "sameAs",
                "$vars": {
                    "value": {
                        "$ref": "1/password"
                    }
                }
            }
        }
    },
    "required": ["password", "passwordRepeat"]
}
JSON;

$schema = json_decode($schema, false);

$data = (object) [
    "password" => "secret",
    "passwordRepeat" => "secret"
];

/** @var ValidationResult $result */
$result = $validator->dataValidation($data, $schema);
if ($result->isValid()) {
    echo "Valid", PHP_EOL;
}
else {
    echo "Invalid", PHP_EOL;
}

I don't think I'll add $data since it is not present in any draft and there is already $filters where you can add any validation logic.

@thatside-zaraffa
Copy link
Author

thatside-zaraffa commented Dec 27, 2017

My idea was to have a little compatibility with other JSON Schema libraries (especially frontend ones) to be able to use same features across backend and frontend (considering "$data" reference is not drafted but planned feature).
Of course I understand your decision :)
Thank you for quick response!

@msarca
Copy link
Member

msarca commented Dec 27, 2017

IMHO, the $data reference proposal is poorly designed and it violates the first rule of server side programming, which is "Do not trust user input". Therefore, I consider this proposal, in its current form, a major security issue. @sorinsarca's solution is much better, in the sense that it allows you to write your own code to validate input. So if you manage to shoot yourself in the leg, at least you will use your own gun, not ours, and you can mitigate the consequences.

@handrews
Copy link

Note that $data remains a controversial proposal and is not guaranteed to be added to the spec in the future. We will probably resolve it one way or the other during draft-09 (so, probably six months to a year from now, depending on how long it takes us to finish the current draft-08 work).

@msarca raises some of the controversial points above.

draft-08 is focusing on modularity and extensibility, and will hopefully make it easier to add and support extension keywords / vocabularies (yeah, that's a really vague statement, we're working on it :-)

@thatside
Copy link

The problem is that most of validation libraries add their own features to standard so schema become less compatible across all implementations.

Only reason I prefer AJV implementation is because it is already working for frontend and it would be great to have that feature for my project - but currently I won't use it.
Nevertheless thank you for your response @handrews :)

@handrews
Copy link

@thatside a lot of the work going into draft-08 is intended to bring some order to extending JSON Schema with new keywords / vocabularies, and declaring what expectations a schema has in terms of vocabulary support.

While it should always be possible to extend JSON Schema (and therefore create schemas that most implementations will not understand), what we want is for it to be easy to publish and reference extensions in a general way, and make it easy for implementations to allow registering support for new keywords. Currently, many implementations make it easy to register extensions supporting non-standard format values, but there's no obvious way to provide that kind of extensibility for keywords in general. Hopefully draft-08 will improve that situation.

@handrews
Copy link

@thatside also, AJV is a perfectly good choice to use as a validator. Just be careful with the ajv-keywords extension package. But AJV itself, working with the standard keywords, is great.

@thatside
Copy link

@handrews IMO extending JSON Schema spec with most used custom features is must-have task for future drafts. Without this everything will end like in a xkcd strip: https://xkcd.com/927/

@handrews
Copy link

@thatside The maintainer of Ajv and ajv-keywords is an active participant in the draft process- rest assured that he advocates for popular extensions :-) Several have made it into the spec in the last year or so.

I do plan to make the $data proposal the primary focus of draft-09, though.

@msarca
Copy link
Member

msarca commented Dec 28, 2017

This thread reminds me about the debates regarding ActiveX and whether it should be supported by other browser vendors or not. At the time, Internet Explorer had a huge market share and many believed that other browsers should provide support for ActiveX just for the sake of compatibility. Let me share you a quote from one of the discussion forums of those times.

If Mozilla (or any other browser) wants to have any chance surviving next to the IE it should at least support all the features that IE has. And ActiveX is definetely one of them.

We all know what happened with IE's market share and I believe that now we're all glad that ActiveX was not widely adopted by browsers. So, in my opinion, popularity or personal preferences are not a viable argument in a debate concerning technology.

@handrews
Copy link

@msarca this is definitely a concern while writing JSON Schema drafts.

People tend to use what you put in front of them. If popular implementation X offers feature Y to solve problem Z, then its users will demand feature Y. Even if it turns out that, paying attention to what we learned from feature Y and discussing things more thoroughly, surveying a broader range of use cases, programming environments, etc, it turns out that Y is not the best solution.

So an extension being popular with an implementation's users tells us that there is a problem that people want solved, and that the extension is at least adequate. It does not tell us whether the extension is ideal, or whether it causes problems outside of how that implementation's user base uses it.

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

5 participants