Go 1.17 - Test failure in gojsonschema time format validation

[olivierlemasle]

Expected Behavior

Test success (works for Go <= 1.16)

Actual Behavior

Test failure with Go 1.17 rc:

--- FAIL: TestFormats (0.07s)
    jsonschema_test.go:102: Test failed : /opa/internal/gojsonschema/testdata/draft7/optional/format/time.json
        validation of time strings.
        only RFC3339 not all of ISO 8601 are valid.
        expects: false, given true
        Schema: {"format":"time"}
        Data: "01:01:01,1111"

Additional Info

A test in internal/gojsonschema/testdata/draft7/optional/format/time.json verifies that 01:01:01,1111 fails to validate as a time.

Time validation is done here:

opa/internal/gojsonschema/format_checkers.go

Lines 266 to 271 in 47feea7

	if _, err := time.Parse("15:04:05Z07:00", asString); err == nil {
	return true
	}
	_, err := time.Parse("15:04:05", asString)
	return err == nil

However _, err := time.Parse("15:04:05", "01:01:01,1111") returns an error parsing time "01:01:01,1111": extra text: ",1111" with Go <= 1.16, and no error with Go 1.17.

This is due to this change in Go: golang/go@f02a26b ("," supported as separator for fractional seconds)

[srenatus]

Thanks for raising this. Let's fix it. I'm not certain, but I don't think that this has a huge relevance for our use of json schema 🤔 Also, it would mean that some previously forbidden inputs become valid, as far as I understand.

What would be the best approach in your opinion? I'm not sure this is even worth introducing a build tag for go 1.17 into our tests... 💭

[olivierlemasle]

• I've just updated my comment above (I had mixed TimeFormatChecker and DateTimeFormatChecker);
• As the objective of the failing test is to verify that the JSON schema requires RFC 3339 and not all valid ISO 8601 times, I would switch to test 14:30, which is a valid (according to Wikipedia, as I don't have access to the ISO spec), but not a RFC 3339 "full-time".
• I don't think a build tag for go 1.17 is currently needed.

Is that ok for you?