Skip to content

feat: Add --allow-invalid-content-type-endpoint option to validate-json - #932

Open
zoliszabo wants to merge 3 commits into
jsonrainbow:mainfrom
zoliszabo:feature/env-for-invalid-content-type-endpoints
Open

feat: Add --allow-invalid-content-type-endpoint option to validate-json#932
zoliszabo wants to merge 3 commits into
jsonrainbow:mainfrom
zoliszabo:feature/env-for-invalid-content-type-endpoints

Conversation

@zoliszabo

Copy link
Copy Markdown

Description

The validate-json CLI now accepts a repeatable --allow-invalid-content-type-endpoint=<url> option. Each URL prefix is registered on the UriRetriever via the existing addInvalidContentTypeEndpoint() method, bypassing the schema media type check for schemas fetched from that host.

Related Issue

N/A

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Code refactoring
  • Other (please describe):

Checklist

  • I have read the CONTRIBUTING guidelines
  • My code follows the code style of this project
  • I have added tests that prove my fix is effective or that my feature works
  • All new and existing tests pass
  • I have updated the documentation accordingly
  • My changes generate no new warnings

Additional Notes

Example use case:

WordPress's theme.json uses https://schemas.wp.org/trunk/theme.json as its JSON schema (docs here: https://developer.wordpress.org/block-editor/reference-guides/theme-json-reference/theme-json-living/). That URL redirects to https://raw.githubusercontent.com/WordPress/gutenberg/trunk/schemas/json/theme.json, which serves Content-Type: text/plain; charset=utf-8, so running

vendor/bin/validate-json theme.json https://schemas.wp.org/trunk/theme.json

fails with InvalidSchemaMediaTypeException.

With the included changes, passing --allow-invalid-content-type-endpoint=https://schemas.wp.org/ whitelists the requested host and lets the validation run.

vendor/bin/validate-json theme.json https://schemas.wp.org/trunk/theme.json --allow-invalid-content-type-endpoint=https://schemas.wp.org/

The validate-json CLI now accepts a repeatable `--allow-invalid-content-type-endpoint=<url>` option.
Each URL prefix is registered on the UriRetriever via the existing `addInvalidContentTypeEndpoint()` API, bypassing the schema media type check for schemas fetched from that host.

WordPress's `theme.json` uses `https://schemas.wp.org/trunk/theme.json` as its JSON schema (docs here: https://developer.wordpress.org/block-editor/reference-guides/theme-json-reference/theme-json-living/).
That URL redirects to `https://raw.githubusercontent.com/WordPress/gutenberg/trunk/schemas/json/theme.json`, which serves `Content-Type: text/plain; charset=utf-8`, so running `vendor/bin/validate-json theme.json https://schemas.wp.org/trunk/theme.json` failed with `InvalidSchemaMediaTypeException`.
With the included changes, passing `--allow-invalid-content-type-endpoint=https://schemas.wp.org/` whitelists the requested host and lets the validation run.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot wasn't able to review any files in this pull request.


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@DannyvdSluijs DannyvdSluijs left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the addition. This is a good improvement, going through the changes I've made some comments to fix some small issues being introduced.
Can you take a look?

Comment thread bin/validate-json Outdated
array_shift($argv);//script itself
foreach ($argv as $arg) {
if ($arg[0] == '-') {
if (strpos($arg, '--allow-invalid-content-type-endpoint=') === 0) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The parsing should be improved. This introduces two unwanted edge cases:

  1. Empty value error (--allow-invalid-content-type-endpoint=) is silently ignored. This should result in an non success exit code.

  2. Value passing without a equals sign (--allow-invalid-content-type-endpoint https://schemas.wp.org) would set $arArgs['https://schemas.wp.org'] = true resulting in an error

./bin/validate-json --allow-invalid-content-type-endpoint https://schemas.wp.org/ theme.json 

PHP Warning:  file_get_contents(https://schemas.wp.org/): Failed to open stream: HTTP request failed! HTTP/1.1 404 Not Found
 in ./bin/validate-json on line 148
PHP Stack trace:
PHP   1. {main}() ./bin/validate-json:0
PHP   2. file_get_contents($filename = 'https://schemas.wp.org/', $use_include_path = FALSE, $context = resource(24) of type (stream-context)) ./bin/validate-json:148

Warning: file_get_contents(https://schemas.wp.org/): Failed to open stream: HTTP request failed! HTTP/1.1 404 Not Found
 in ./bin/validate-json on line 148

Call Stack:
    0.0004     506208   1. {main}() ./bin/validate-json:0
    0.0040     814392   2. file_get_contents($filename = 'https://schemas.wp.org/', $use_include_path = FALSE, $context = resource(24) of type (stream-context)) ./bin/validate-json:148

Data file is not readable or empty.

Comment thread bin/validate-json
&& is_array($arOptions['--allow-invalid-content-type-endpoint'])
) {
foreach ($arOptions['--allow-invalid-content-type-endpoint'] as $endpoint) {
$retriever->addInvalidContentTypeEndpoint($endpoint);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is only applied to the schema storage which isn't passed to the validator on line 233
Could you pass it using a factory, this way it would also be solved for nested schemas.

new JsonSchema\Validator(new JsonSchema\Constraints\Factory($refResolver, $retriever))

Accept the value in both --option=value and --option value forms. Exit with a non-zero code when the value is missing instead of silently ignoring an empty value. Add CLI coverage for the option.
@zoliszabo

Copy link
Copy Markdown
Author

@DannyvdSluijs Thanks for the review. I pushed new commits fixing the highlighted issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants