danielgtaylor / huma

Huma REST/HTTP API Framework for Golang with OpenAPI 3.1
https://huma.rocks/
MIT License
1.89k stars 138 forks source link

time.Time cannot validate using some specified format hints #537

Open coury-clark opened 1 month ago

coury-clark commented 1 month ago

The docs specify that the tag format can change the validation format for a date/time. For example format:"date" should allow RFC3339 dates (2024-01-01).

However, there is another validation that is restricting the value to the value of the timeFormat tag. This is reproduced in the example below. You will see two tests, the first provides an RFC3339 date and fails validation because it's not a datetime, then providing a datetime to the same endpoint fails validation because it's not a date.

This additional validation takes place here - it isn't clear to me why this is separate from the other validators.

type MyInput struct {
    MyTime time.Time `query:"my-time" format:"date"`
}

func Test_Reproduce_Bad_DateFormat(t *testing.T) {
    _, api := humatest.New(t)
    huma.Register(api, huma.Operation{Method: "GET", Path: "/test"}, func(ctx context.Context, i *MyInput) (*struct{}, error) {
        return &struct{}{}, nil
    })

    t.Run("with RFC3339 date", func(t *testing.T) {
        // this should work, but fails because there is a _datetime_ validation
        resp := api.Get("/test?my-time=2024-01-01")
        if resp.Code != http.StatusOK {
            t.Fatal()
        }
    })

    t.Run("with RFC3339 datetime", func(t *testing.T) {
        // this should fail, and does appropriately
        resp := api.Get("/test?my-time=2024-01-01T00:00:00Z")
        if resp.Code != http.StatusOK {
            t.Fatal()
        }
    })
}

As a workaround, one can instead specify the timeFormat tag as so:

type MyInput struct {
    MyTime time.Time `query:"my-time" timeFormat:"2006-01-02"`
}
krisatverbidio commented 1 month ago

I believe that format tag is only for string data types. If you want to have a specific format provided in an input for a time.Time, you will probably need to create a custom data type with its own resolver.

Alternatively, you can change the datatype to a string and then parse it into a time.Time in your handler. The string should be validated as being in the proper format at that time.

coury-clark commented 1 month ago

@krisatverbidio I suppose that's mostly what this issue is after - what is the intention?

From the second test in the example above, time.Time is validated using format, but cannot functionally pass if the format is not date-time. This is because it is functionally validated as a string here, but only after this other timeFormat parsing (so it can become a string).

The timeFormat tag isn't mentioned in the input documentation, but does work - is that the intended route here?

It seems like it could be a fairly straightforward change to support the datetime related tag fields for time.Time in the validation above. I'd be happy to make a PR, I'm just looking for clarity here on what is expected to happen?

krisatverbidio commented 1 month ago

@coury-clark I think @danielgtaylor would have to chime in on the intention. But my answer is based on the docs. Where the formats are described in the documentation, it explicitly says "Built-in STRING formats include:" (emphasis mine) and within there date-time is one of them.

danielgtaylor commented 1 month ago

@coury-clark thanks for opening this issue. The format tag is intended to document and limit the input string value, while the timeFormat is intended to tell Go how to marshal/unmarshal the time from the string. I need to document this better, sorry about that! Go uses a fairly unique way of defining how to marshal/unmarshal a time and I wanted to be explicit rather than trying to do magic with format regular expressions to determine how to parse it.

Mainly timeFormat exists because headers like Last-Modified use a different time format from what you would commonly send in with query params, but it can be used to support any time format Go can parse. I hope that makes sense.

The downside of all this is to support custom date/time formats you likely have to set both format and timeFormat. Another complication is that the Go JSON marshaller doesn't support custom time formats, so this only works for path/query/cookie params and header values at the moment.

If this doesn't work for a given scenario you can bypass the automatic validation/unmarshal by just taking a string and converting it to a time.Time yourself when needed, possibly in a resolver so you still get exhaustive errors for the client. Hope this helps clear things up a bit!