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

Bug fix: relative URI starting with a slash #80

Merged

Conversation

ctbucha
Copy link

@ctbucha ctbucha commented Feb 22, 2024

Hi there! Thanks for the work on this library!

I am new to using JSON schemas, so I am not completely familiar with the details of the specs, but some JSON schemas that I am working with lately have had an $id field as a relative URI starting with a slash / with additional slashes (e.g. /foods/apple). In the Exonerate.Context.id_swap_with/5 function, the updated_resource variable merges the id (/foods/apple) with the resource (/foods/apple) erroneously resulting in /foods/foods/apple and then crashing when it cannot find /foods/foods/apple in the ets table.

This change updates the extended URI.merge function on a relative URL to include expected behavior when the dest_path starts with a slash /, which is to completely replace the path with the dest_path. Example from the standard URI.merge function:

"https://example.com/hello/world" |> URI.parse() |> URI.merge("/food/apples")
%URI{
  scheme: "https",
  authority: "example.com",
  userinfo: nil,
  host: "example.com",
  port: 443,
  path: "/food/apples",
  query: nil,
  fragment: nil
}

I would like to add a unit test for this change as well, but I could use some help if possible.

Thanks again, and let me know how I can help.

@ityonemo
Copy link
Collaborator

modifying to add to a new branch which will incorporate these changes -- but we need to also account for the case when there is both a leading AND a trailing slash.

@ityonemo ityonemo changed the base branch from master to bug-fix-path-types March 10, 2024 23:36
@ityonemo ityonemo merged commit d89e061 into E-xyza:bug-fix-path-types Mar 10, 2024
@ityonemo
Copy link
Collaborator

@ctbucha can you provide a simplified/redacted schema that you would like to see supported? I'm having trouble reconciling what we're trying to acheive with these relative URI ids (which we must support) as I'm writing some tests.

@ityonemo
Copy link
Collaborator

also, if you could provide the draft you're targetting that would be great, thanks!

@ctbucha
Copy link
Author

ctbucha commented Mar 11, 2024

modifying to add to a new branch which will incorporate these changes -- but we need to also account for the case when there is both a leading AND a trailing slash.

In the case where the dest_path starts with a slash, it wouldn't matter if the path ends with a slash since path would be overwritten regardless.

can you provide a simplified/redacted schema that you would like to see supported? I'm having trouble reconciling what we're trying to acheive with these relative URI ids (which we must support) as I'm writing some tests.

/my_project/src/containers/basket.json

{
    "$schema": "https://json-schema.org/draft/2020-12/schema",
    "$id": "/containers/basket",
    "title": "Basket",
    "description": "A basket that contains apples.",
    "type": "object",
    "properties": {
        "contents": {
            "description": "The apples contained in the basket",
            "type": "array",
            "items": {
                "$ref": "/foods/apple"
            }
        }
    }
}

my_project/src/foods/apple.json

{
    "$schema": "https://json-schema.org/draft/2020-12/schema",
    "$id": "/foods/apple",
    "title": "Apple",
    "description": "An apple.",
    "type": "object",
    "properties": {
        "type": {
            "description": "The type of apple",
            "enum": [
                "GALA",
                "FUJI",
                "PINK_LADY"
            ]
        }
    }
}

also, if you could provide the draft you're targetting that would be great, thanks!

https://json-schema.org/draft/2020-12/schema

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

Successfully merging this pull request may close these issues.

2 participants