Skip to content

Commit d0137c3

Browse files
vishrclaude
andcommitted
Revert Issue #2813 fix based on maintainer feedback
Revert the DefaultBinder empty body handling changes following @aldas's concerns about: - Body replacement potentially interfering with custom readers - Lack of proper reproduction case for the original issue - Potential over-engineering for an edge case The "read one byte and reconstruct body" approach could interfere with users who add custom readers with specific behavior. Waiting for better reproduction case and less invasive solution. Refs: #2813 (comment) 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
1 parent 2fb8419 commit d0137c3

File tree

2 files changed

+139
-28
lines changed

2 files changed

+139
-28
lines changed

bind.go

Lines changed: 26 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,12 @@ import (
88
"encoding/xml"
99
"errors"
1010
"fmt"
11-
"io"
1211
"mime/multipart"
1312
"net/http"
1413
"reflect"
1514
"strconv"
1615
"strings"
16+
"time"
1717
)
1818

1919
// Binder is the interface that wraps the Bind method.
@@ -40,6 +40,13 @@ type bindMultipleUnmarshaler interface {
4040
}
4141

4242
// BindPathParams binds path params to bindable object
43+
//
44+
// Time format support: time.Time fields can use `format` tags to specify custom parsing layouts.
45+
// Example: `param:"created" format:"2006-01-02T15:04"` for datetime-local format
46+
// Example: `param:"date" format:"2006-01-02"` for date format
47+
// Uses Go's standard time format reference time: Mon Jan 2 15:04:05 MST 2006
48+
// Works with form data, query parameters, and path parameters (not JSON body)
49+
// Falls back to default time.Time parsing if no format tag is specified
4350
func (b *DefaultBinder) BindPathParams(c Context, i interface{}) error {
4451
names := c.ParamNames()
4552
values := c.ParamValues()
@@ -72,22 +79,6 @@ func (b *DefaultBinder) BindBody(c Context, i interface{}) (err error) {
7279
return
7380
}
7481

75-
// For unknown ContentLength (-1), check if body is actually empty
76-
if req.ContentLength == -1 {
77-
// Peek at the first byte to see if there's any content
78-
var buf [1]byte
79-
n, readErr := req.Body.Read(buf[:])
80-
if readErr != nil && readErr != io.EOF {
81-
return NewHTTPError(http.StatusBadRequest, readErr.Error()).SetInternal(readErr)
82-
}
83-
if n == 0 {
84-
// Body is empty, return without error
85-
return
86-
}
87-
// There's content, put the byte back by creating a new reader
88-
req.Body = io.NopCloser(io.MultiReader(strings.NewReader(string(buf[:n])), req.Body))
89-
}
90-
9182
// mediatype is found like `mime.ParseMediaType()` does it
9283
base, _, _ := strings.Cut(req.Header.Get(HeaderContentType), ";")
9384
mediatype := strings.TrimSpace(base)
@@ -279,7 +270,8 @@ func (b *DefaultBinder) bindData(destination interface{}, data map[string][]stri
279270
continue
280271
}
281272

282-
if ok, err := unmarshalInputToField(typeField.Type.Kind(), inputValue[0], structField); ok {
273+
formatTag := typeField.Tag.Get("format")
274+
if ok, err := unmarshalInputToField(typeField.Type.Kind(), inputValue[0], structField, formatTag); ok {
283275
if err != nil {
284276
return err
285277
}
@@ -315,7 +307,8 @@ func (b *DefaultBinder) bindData(destination interface{}, data map[string][]stri
315307

316308
func setWithProperType(valueKind reflect.Kind, val string, structField reflect.Value) error {
317309
// But also call it here, in case we're dealing with an array of BindUnmarshalers
318-
if ok, err := unmarshalInputToField(valueKind, val, structField); ok {
310+
// Note: format tag not available in this context, so empty string is passed
311+
if ok, err := unmarshalInputToField(valueKind, val, structField, ""); ok {
319312
return err
320313
}
321314

@@ -372,7 +365,7 @@ func unmarshalInputsToField(valueKind reflect.Kind, values []string, field refle
372365
return true, unmarshaler.UnmarshalParams(values)
373366
}
374367

375-
func unmarshalInputToField(valueKind reflect.Kind, val string, field reflect.Value) (bool, error) {
368+
func unmarshalInputToField(valueKind reflect.Kind, val string, field reflect.Value, formatTag string) (bool, error) {
376369
if valueKind == reflect.Ptr {
377370
if field.IsNil() {
378371
field.Set(reflect.New(field.Type().Elem()))
@@ -381,6 +374,19 @@ func unmarshalInputToField(valueKind reflect.Kind, val string, field reflect.Val
381374
}
382375

383376
fieldIValue := field.Addr().Interface()
377+
378+
// Handle time.Time with custom format tag
379+
if formatTag != "" {
380+
if _, isTime := fieldIValue.(*time.Time); isTime {
381+
t, err := time.Parse(formatTag, val)
382+
if err != nil {
383+
return true, err
384+
}
385+
field.Set(reflect.ValueOf(t))
386+
return true, nil
387+
}
388+
}
389+
384390
switch unmarshaler := fieldIValue.(type) {
385391
case BindUnmarshaler:
386392
return true, unmarshaler.UnmarshalParam(val)

bind_test.go

Lines changed: 113 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1082,14 +1082,6 @@ func TestDefaultBinder_BindBody(t *testing.T) {
10821082
givenContent: strings.NewReader(""),
10831083
expect: &Node{ID: 0, Node: ""},
10841084
},
1085-
{
1086-
name: "ok, POST with empty body and ContentLength -1 (Issue #2813)",
1087-
givenURL: "/api/real_node/endpoint?node=xxx",
1088-
givenMethod: http.MethodPost,
1089-
givenContent: strings.NewReader(""),
1090-
whenChunkedBody: true, // This sets ContentLength to -1
1091-
expect: &Node{ID: 0, Node: ""},
1092-
},
10931085
{
10941086
name: "ok, JSON POST bind to struct with: path + query + chunked body",
10951087
givenURL: "/api/real_node/endpoint?node=xxx",
@@ -1579,3 +1571,116 @@ func assertMultipartFileHeader(t *testing.T, fh *multipart.FileHeader, file test
15791571
err = fl.Close()
15801572
assert.NoError(t, err)
15811573
}
1574+
1575+
func TestTimeFormatBinding(t *testing.T) {
1576+
type TestStruct struct {
1577+
DateTimeLocal time.Time `form:"datetime_local" format:"2006-01-02T15:04"`
1578+
Date time.Time `query:"date" format:"2006-01-02"`
1579+
CustomFormat time.Time `form:"custom" format:"01/02/2006 15:04:05"`
1580+
DefaultTime time.Time `form:"default_time"` // No format tag - should use default parsing
1581+
PtrTime *time.Time `query:"ptr_time" format:"2006-01-02"`
1582+
}
1583+
1584+
testCases := []struct {
1585+
name string
1586+
contentType string
1587+
data string
1588+
queryParams string
1589+
expect TestStruct
1590+
expectError bool
1591+
}{
1592+
{
1593+
name: "ok, datetime-local format binding",
1594+
contentType: MIMEApplicationForm,
1595+
data: "datetime_local=2023-12-25T14:30&default_time=2023-12-25T14:30:45Z",
1596+
expect: TestStruct{
1597+
DateTimeLocal: time.Date(2023, 12, 25, 14, 30, 0, 0, time.UTC),
1598+
DefaultTime: time.Date(2023, 12, 25, 14, 30, 45, 0, time.UTC),
1599+
},
1600+
},
1601+
{
1602+
name: "ok, date format binding via query params",
1603+
queryParams: "?date=2023-01-15&ptr_time=2023-02-20",
1604+
expect: TestStruct{
1605+
Date: time.Date(2023, 1, 15, 0, 0, 0, 0, time.UTC),
1606+
PtrTime: &time.Time{},
1607+
},
1608+
},
1609+
{
1610+
name: "ok, custom format via form data",
1611+
contentType: MIMEApplicationForm,
1612+
data: "custom=12/25/2023 14:30:45",
1613+
expect: TestStruct{
1614+
CustomFormat: time.Date(2023, 12, 25, 14, 30, 45, 0, time.UTC),
1615+
},
1616+
},
1617+
{
1618+
name: "nok, invalid format should fail",
1619+
contentType: MIMEApplicationForm,
1620+
data: "datetime_local=invalid-date",
1621+
expectError: true,
1622+
},
1623+
{
1624+
name: "nok, wrong format should fail",
1625+
contentType: MIMEApplicationForm,
1626+
data: "datetime_local=2023-12-25", // Missing time part
1627+
expectError: true,
1628+
},
1629+
}
1630+
1631+
for _, tc := range testCases {
1632+
t.Run(tc.name, func(t *testing.T) {
1633+
e := New()
1634+
var req *http.Request
1635+
1636+
if tc.contentType == MIMEApplicationJSON {
1637+
req = httptest.NewRequest(http.MethodPost, "/"+tc.queryParams, strings.NewReader(tc.data))
1638+
req.Header.Set(HeaderContentType, tc.contentType)
1639+
} else if tc.contentType == MIMEApplicationForm {
1640+
req = httptest.NewRequest(http.MethodPost, "/"+tc.queryParams, strings.NewReader(tc.data))
1641+
req.Header.Set(HeaderContentType, tc.contentType)
1642+
} else {
1643+
req = httptest.NewRequest(http.MethodGet, "/"+tc.queryParams, nil)
1644+
}
1645+
1646+
rec := httptest.NewRecorder()
1647+
c := e.NewContext(req, rec)
1648+
1649+
var result TestStruct
1650+
err := c.Bind(&result)
1651+
1652+
if tc.expectError {
1653+
assert.Error(t, err)
1654+
return
1655+
}
1656+
1657+
assert.NoError(t, err)
1658+
1659+
// Check individual fields since time comparison can be tricky
1660+
if !tc.expect.DateTimeLocal.IsZero() {
1661+
assert.True(t, tc.expect.DateTimeLocal.Equal(result.DateTimeLocal),
1662+
"DateTimeLocal: expected %v, got %v", tc.expect.DateTimeLocal, result.DateTimeLocal)
1663+
}
1664+
if !tc.expect.Date.IsZero() {
1665+
assert.True(t, tc.expect.Date.Equal(result.Date),
1666+
"Date: expected %v, got %v", tc.expect.Date, result.Date)
1667+
}
1668+
if !tc.expect.CustomFormat.IsZero() {
1669+
assert.True(t, tc.expect.CustomFormat.Equal(result.CustomFormat),
1670+
"CustomFormat: expected %v, got %v", tc.expect.CustomFormat, result.CustomFormat)
1671+
}
1672+
if !tc.expect.DefaultTime.IsZero() {
1673+
assert.True(t, tc.expect.DefaultTime.Equal(result.DefaultTime),
1674+
"DefaultTime: expected %v, got %v", tc.expect.DefaultTime, result.DefaultTime)
1675+
}
1676+
if tc.expect.PtrTime != nil {
1677+
assert.NotNil(t, result.PtrTime)
1678+
if result.PtrTime != nil {
1679+
expectedPtr := time.Date(2023, 2, 20, 0, 0, 0, 0, time.UTC)
1680+
assert.True(t, expectedPtr.Equal(*result.PtrTime),
1681+
"PtrTime: expected %v, got %v", expectedPtr, *result.PtrTime)
1682+
}
1683+
}
1684+
})
1685+
}
1686+
}

0 commit comments

Comments
 (0)