xeipuuv / gojsonschema

An implementation of JSON Schema, draft v4 v6 & v7 - Go language
2.53k stars 356 forks source link

Validate returns error on non-JSON document instead of an invalid result #294

Open aelse opened 4 years ago

aelse commented 4 years ago

When validating a JSON document I get an error from Validate if the user-provided document is not valid JSON. My expectation is that no error would be generated and result.Valid() == false.

To reproduce, load a JSON schema and attempt to validate against a document that is not well-formed JSON: {"abc": "invalid json encoding"

The current example code in the readme panics in the case of an error during validation. To me this suggests that error cases are due to internal logic or runtime problems rather than bad user input.

If I want to differentiate between invalid JSON and a gojsonschema error then I need to perform an additional step to unmarshal the payload.

I would suggest a behaviour change to return nil error and an invalid result in the case that the document is not well formed JSON, but that might be a big ask.

Alternatively, the example in the readme could be updated to be clearer about the possible source of the error. eg.

if err != nil {
    fmt.Printf("The document is not JSON, or some other error occurred: %v"., err)
}

Readme code below for easy reference.

package main

import (
    "fmt"
    "github.com/xeipuuv/gojsonschema"
)

func main() {

    schemaLoader := gojsonschema.NewReferenceLoader("file:///home/me/schema.json")
    documentLoader := gojsonschema.NewReferenceLoader("file:///home/me/document.json")

    result, err := gojsonschema.Validate(schemaLoader, documentLoader)
    if err != nil {
        panic(err.Error())
    }

    if result.Valid() {
        fmt.Printf("The document is valid\n")
    } else {
        fmt.Printf("The document is not valid. see errors :\n")
        for _, desc := range result.Errors() {
            fmt.Printf("- %s\n", desc)
        }
    }
}

Our code that encountered the problem looks more like:

package main

import (
    "fmt"

    js "github.com/xeipuuv/gojsonschema"
)

func main() {
    schemaLoader := js.NewSchemaLoader()
    if err := schemaLoader.AddSchemas(js.NewReferenceLoader("file:///home/me/definitions.json")); err != nil {
        panic(err)
    }
    schema, err := schemaLoader.Compile(js.NewReferenceLoader("file:///home/me/schema.json"))
    if err != nil {
        panic(err)
    }
    validate := func(payload []byte) (*js.Result, error) {
        return schema.Validate(js.NewBytesLoader(payload))
    }

    inputs := []string{
        `{"invalid": "this isn't json"`,
        `this isn't json`,
    }
    for _, input := range inputs {
        fmt.Printf("Testing input: '%s'\n", input)
        result, err := validate([]byte(input))
        if err != nil {
            fmt.Printf("error: %v\n", err)
        }
        if result != nil {
            fmt.Printf("valid result? %v\n", result.Valid())
        }
    }
}

The output from this program is:

Testing input: '{"invalid": "this isn't json"'
error: unexpected EOF
Testing input: 'this isn't json'
error: invalid character 'h' in literal true (expecting 'r')
johandorland commented 4 years ago

I could see how changing the README would communicate clearer what would be returned as an err. I don't agree on treating malformed JSON and an invalid JSON document in the same manner. JSON schema validation and JSON parsing are fundamentally two different things and I don't want to combine the two in the same error mechanism.

aelse commented 4 years ago

If you're happy with a README change I'll put in a PR.

johandorland commented 4 years ago

I'm always very happy with any kind of contribution 😃