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.

Sign up for GitHub

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

Jump to bottom

IDs are always validated as URIs. #513

Open
gdovalMule opened this issue Aug 19, 2024 · 6 comments
Open

IDs are always validated as URIs. #513

gdovalMule opened this issue Aug 19, 2024 · 6 comments

Comments

@gdovalMule
Copy link

gdovalMule commented Aug 19, 2024 •
edited
Loading

Description
From what I understand, the 'id' keyword at the root level is used as a URI reference, but only at this point (I might be wrong). However, I see that Everit performs a recursive search in the child elements, looking for all the IDs, and these are also validated as URIs.

In the following example, I understand that the ID should not be considered as a URI:

{
  "$schema": "http://json-schema.org/draft-04/schema#",
  "$ref": "#/definitions/SubstitutionRules",
  "definitions": {
    "SubstitutionRules": {
      "x-amf-examples": {
        "output": {
          "substitutionRules": [
            {
              "id": "ABCDEGH@20201005000000 @20201031000000 @S",
              "user": "ABCDEGH",
              "beginDate": "2020-10-17T00:00:00Z",
              "endDate": "2020-10-27T00:00:00Z",
              "fullName": "Abcde Abcde",
              "isEnabled": 1,
              "supportsDeleteSubstitutionRule": true,
              "supportsEnableSubstitutionRule": true,
              "mode": 1,
              "profile": "ZPROCUREMENT",
              "profileText": "Procurement",
              "sourceSystem": "ENUM1"
            }
          ]
        }
      },
      "type": "object",
      "additionalProperties": true,
      "required": [
        "substitutionRules"
      ],
      "properties": {
        "substitutionRules": {
          "items": {
            "$ref": "#/definitions/type"
          },
          "type": "array"
        }
      }
    },
    "type": {
      "type": "object",
      "additionalProperties": true,
      "required": [
        "beginDate",
        "endDate"
      ],
      "properties": {
        "id": {
          "type": "string"
        },
        "user": {
          "type": "string"
        },
        "beginDate": {
          "type": "string",
          "format": "date-time"
        },
        "endDate": {
          "type": "string",
          "format": "date-time"
        },
        "fullName": {
          "type": "string"
        },
        "isEnabled": {
          "description": "Mapped to Permission Scope on the Exp layer takes value 1 (Receive My Tasks) or 2 (Fill In For Me)",
          "type": "integer"
        },
        "supportsDeleteSubstitutionRule": {
          "type": "boolean"
        },
        "supportsEnableSubstitutionRule": {
          "type": "boolean"
        },
        "mode": {
          "description": "Mapped to Delegation Type on the Exp layer takes value 0 (Unplanned) or 1 (Planned)",
          "type": "integer"
        },
        "role": {
          "description": "Mapped to Role on the Exp Layer takes values 1 (Approve), 2(Approve&Update), 3(Reporting), 4(Update)",
          "type": "integer"
        },
        "profile": {
          "description": "Used for Invoice system as an static value \"ZPROCUREMENT\"",
          "type": "string"
        },
        "profileText": {
          "description": "Used for Invoice system as an static value \"Procurement\"",
          "type": "string"
        },
        "sourceSystem": {
          "$ref": "#/definitions/type_1"
        }
      }
    },
    "type_1": {
      "type": "string",
      "description": "The source system of the data in SAP",
      "enum": [
        "ENUM1",
        "ENUM2",
        "ENUM3"
      ]
    }
  }
}

I tested with some online validators and everything seems to work fine, but Everit shows the following error:

_
java.lang.RuntimeException: java.net.URISyntaxException: Illegal character in path at index 22: ABCDEGH@20201005000000 @20201031000000 @s

at org.everit.json.schema.loader.internal.ReferenceResolver.resolve(ReferenceResolver.java:29)
at org.everit.json.schema.loader.LoadingState.extractChildId(LoadingState.java:32)
at org.everit.json.schema.loader.LoadingState.<init>(LoadingState.java:72)
at org.everit.json.schema.loader.LoadingState.childFor(LoadingState.java:120)
at org.everit.json.schema.loader.LoadingState.childFor(LoadingState.java:127)
at org.everit.json.schema.loader.JsonArray.at(JsonArray.java:29)
at org.everit.json.schema.loader.SubschemaRegistry.collectObjectsWithId(SubschemaRegistry.java:29)
at org.everit.json.schema.loader.SubschemaRegistry.collectObjectsWithId(SubschemaRegistry.java:24)
at org.everit.json.schema.loader.SubschemaRegistry.collectObjectsWithId(SubschemaRegistry.java:24)
at org.everit.json.schema.loader.SubschemaRegistry.collectObjectsWithId(SubschemaRegistry.java:24)
at org.everit.json.schema.loader.SubschemaRegistry.collectObjectsWithId(SubschemaRegistry.java:24)
at org.everit.json.schema.loader.SubschemaRegistry.collectObjectsWithId(SubschemaRegistry.java:24)
at org.everit.json.schema.loader.SubschemaRegistry.<init>(SubschemaRegistry.java:12)
at java.base/java.util.HashMap.computeIfAbsent(HashMap.java:1220)
at org.everit.json.schema.loader.LoadingState.getSubschemaRegistry(LoadingState.java:169)
at org.everit.json.schema.loader.ReferenceLookup.lookupObjById(ReferenceLookup.java:71)
at org.everit.json.schema.loader.ReferenceLookup.lookup(ReferenceLookup.java:138)
at org.everit.json.schema.loader.ReferenceSchemaExtractor.extract(SchemaExtractor.java:199)
at org.everit.json.schema.loader.AbstractSchemaExtractor.extract(SchemaExtractor.java:114)
at org.everit.json.schema.loader.SchemaLoader.runSchemaExtractors(SchemaLoader.java:411)
at org.everit.json.schema.loader.SchemaLoader.loadSchemaObject(SchemaLoader.java:388)
at org.everit.json.schema.loader.JsonValue$Multiplexer.requireAny(JsonValue.java:47)
at org.everit.json.schema.loader.SchemaLoader.load(SchemaLoader.java:462)
at org.example.JsonSchemaTest.dereferencing(Main.java:30)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:568)
at org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:727)
at org.junit.jupiter.engine.execution.MethodInvocation.proceed(MethodInvocation.java:60)
at org.junit.jupiter.engine.execution.InvocationInterceptorChain$ValidatingInvocation.proceed(InvocationInterceptorChain.java:131)
at org.junit.jupiter.engine.extension.TimeoutExtension.intercept(TimeoutExtension.java:156)
at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestableMethod(TimeoutExtension.java:147)
at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestMethod(TimeoutExtension.java:86)
at org.junit.jupiter.engine.execution.InterceptingExecutableInvoker$ReflectiveInterceptorCall.lambda$ofVoidMethod$0(InterceptingExecutableInvoker.java:103)
at org.junit.jupiter.engine.execution.InterceptingExecutableInvoker.lambda$invoke$0(InterceptingExecutableInvoker.java:93)
at org.junit.jupiter.engine.execution.InvocationInterceptorChain$InterceptedInvocation.proceed(InvocationInterceptorChain.java:106)
at org.junit.jupiter.engine.execution.InvocationInterceptorChain.proceed(InvocationInterceptorChain.java:64)
at org.junit.jupiter.engine.execution.InvocationInterceptorChain.chainAndInvoke(InvocationInterceptorChain.java:45)
at org.junit.jupiter.engine.execution.InvocationInterceptorChain.invoke(InvocationInterceptorChain.java:37)
at org.junit.jupiter.engine.execution.InterceptingExecutableInvoker.invoke(InterceptingExecutableInvoker.java:92)
at org.junit.jupiter.engine.execution.InterceptingExecutableInvoker.invoke(InterceptingExecutableInvoker.java:86)
at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.lambda$invokeTestMethod$7(TestMethodTestDescriptor.java:217)
at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.invokeTestMethod(TestMethodTestDescriptor.java:213)
at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:138)
at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:68)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:151)
at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:41)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:155)
at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:41)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:155)
at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.submit(SameThreadHierarchicalTestExecutorService.java:35)
at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor.execute(HierarchicalTestExecutor.java:57)
at org.junit.platform.engine.support.hierarchical.HierarchicalTestEngine.execute(HierarchicalTestEngine.java:54)
at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:147)
at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:127)
at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:90)
at org.junit.platform.launcher.core.EngineExecutionOrchestrator.lambda$execute$0(EngineExecutionOrchestrator.java:55)
at org.junit.platform.launcher.core.EngineExecutionOrchestrator.withInterceptedStreams(EngineExecutionOrchestrator.java:102)
at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:54)
at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:114)
at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:86)
at org.junit.platform.launcher.core.DefaultLauncherSession$DelegatingLauncher.execute(DefaultLauncherSession.java:86)
at org.junit.platform.launcher.core.SessionPerRequestLauncher.execute(SessionPerRequestLauncher.java:53)
at com.intellij.junit5.JUnit5IdeaTestRunner.startRunnerWithArgs(JUnit5IdeaTestRunner.java:57)
at com.intellij.rt.junit.IdeaTestRunner$Repeater$1.execute(IdeaTestRunner.java:38)
at com.intellij.rt.execution.junit.TestsRepeater.repeat(TestsRepeater.java:11)
at com.intellij.rt.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:35)
at com.intellij.rt.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:232)
at com.intellij.rt.junit.JUnitStarter.main(JUnitStarter.java:55)

Caused by: java.net.URISyntaxException: Illegal character in path at index 22: ABCDEGH@20201005000000 @20201031000000 @s
at java.base/java.net.URI$Parser.fail(URI.java:2974)
at java.base/java.net.URI$Parser.checkChars(URI.java:3145)
at java.base/java.net.URI$Parser.parseHierarchical(URI.java:3227)
at java.base/java.net.URI$Parser.parse(URI.java:3186)
at java.base/java.net.URI.(URI.java:623)
at org.everit.json.schema.loader.internal.ReferenceResolver.resolve(ReferenceResolver.java:26)
... 93 more_

I have attached a simplified application to reproduce the issue mentioned.

t-json-schema-m.zip
@erosb
Copy link
Contributor

erosb commented Aug 19, 2024 via email

@gdovalMule
Copy link
Author

gdovalMule commented Aug 23, 2024 •
edited
Loading

Hi @erosb thanks for your response.

I'm taking the definition from https://json-schema.org/understanding-json-schema/structuring#id, and in my case, I need to use draft 4.

image

From what I see in SubschemaRegistry, it recursively searches throughout the entire JSON schema for any references to id and is validated as an URI.

@asanguinetti
Copy link

Hi @erosb we have some customers that are getting anxious about this fix. Could you confirm if this is considered in the backlog for your next release?
Thanks

@erosb
Copy link
Contributor

erosb commented Oct 17, 2024

Hello @asanguinetti , as per the respective section of the json schema draft-4 spec , The value for this keyword MUST be a string, and MUST be a valid URI.

Moreover, please refer to the deprecation notice at the beginning is this project's README. Thanks.

@asanguinetti
Copy link

asanguinetti commented Oct 17, 2024 •
edited
Loading

Thanks for your response @erosb.

I think the specification reference you shared corresponds to the id keyword only. That is, when the id is used to "alter the base URI against which a reference must resolve".

In the provided example in the issue description, it is not the id keyword, it is an example (using the x-amf-examples extension). The type SubstitutionRules has a property called id which accepts any string. The extension is just attempting to provide an example for that value.

It seems in later draft versions, the id keyword was replaced to $id, which makes more sense to avoid this kind of ambiguous situations. But I think, even without the dollar sign, the interpretation of the id as a special keyword should only happen at the root of a schema (or subschema) definition, but not just globally.

The issue started happening after we upgraded from 1.5.1 to 1.14.3, because we needed the fixes from 1.12.0.
I hadn't seen the deprecation notice, does json-sKema support draft 4? in any case I'm assuming it is not a swap-in replacement.

Even though this project (json-schema) is in deprecation mode, are you still providing bug fixes? or are you open for bug fix contributions?

@erosb
Copy link
Contributor

erosb commented Oct 17, 2024 •
edited
Loading

In the provided example in the issue description, it is not the id keyword, it is an example (using the x-amf-examples extension).

Ok, now I got it. I think the current implementation is still correct according to the draft-4 spec, I understand how it causes a problem (AFAIR I also couldn't wrap my head around why "id" is defined this way). The official test suite has some explicit tests verifying that an "id" inside an "enum" or "const" is not recogized as a schema id, but nothing is tested about objects nested under non-schema keywords.

Moreover, it wouldn't be safe to change this behavior: a "$ref" can point to anywhere, there is no constraint about the expected destination location. Bringing your case, if the schema contains a "$ref": "#/definitions/SubstitutionRules/x-amf-examples/output/substitutionRules/0" , that's actually a valid $ref, and the "id" should be taken into account accordingly.

As far as I could figure out, behavior in such cases is undefined:

It seems in later draft versions, the id keyword was replaced to $id,

correct, this implementation is aware of such differences, if the schema's meta-schema is draft-4, then it looks for "id", otherwise "$id"

the interpretation of the id as a special keyword should only happen at the root of a schema (or subschema) definition, but not just globally

Not sure what was the reason for implementing it this way, but better not to remove that global lookup completely. Probably, it would do the least harm to stop that recursion when any of the "example", "enum" or "const" keywords are found, and make this list configurable (you will have to add x-amf-examples to the list for your usecase). Maybe that would be the best way to handle this, but I would hide this behind a feature flag and turn it off by default. If you decide to raise a PR I can find time to do the release.

I hadn't seen the deprecation notice, does json-sKema support draft 4? in any case I'm assuming it is not a swap-in replacement.

It doesn't support draft-4, only draft2020-12, and it certainly isn't a drop-in replacement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@erosb @asanguinetti @gdovalMule