Skip to content

fix: RetainUntilDate in the correct format - #1890

Open
gribouille wants to merge 1 commit into
minio:masterfrom
gribouille:fix-time-parsing
Open

fix: RetainUntilDate in the correct format#1890
gribouille wants to merge 1 commit into
minio:masterfrom
gribouille:fix-time-parsing

Conversation

@gribouille

Copy link
Copy Markdown

RetainUntilDate uses the time.RFC3339 format instead of the ISO 8601 format.

This problem generates the error The retain until date must be provided in ISO 8601 format.

@harshavardhana

Copy link
Copy Markdown
Member

who is returning this error? which server @gribouille

@gribouille

Copy link
Copy Markdown
Author

OVHcloud s3 object storage: https://www.ovhcloud.com/en/public-cloud/object-storage/

The problem is similar to: minio/minio#15741

@harshavardhana

Copy link
Copy Markdown
Member

OVHcloud s3 object storage: https://www.ovhcloud.com/en/public-cloud/object-storage/

The problem is similar to: minio/minio#15741

AWS S3 runs fine, this looks like OVH Cloud bug.

@harshavardhana

Copy link
Copy Markdown
Member

And has been for years. I don't think we can take this PR. It looks like server implementation bug.

@harshavardhana

harshavardhana commented Oct 19, 2023

Copy link
Copy Markdown
Member

The problem is similar to: minio/minio#15741

This is not the same problem as here. Server preserves meta is iso9601 format. But it is okay to send it either in iso8601 or rfc3339

Comment thread api-put-object.go

if !opts.RetainUntilDate.IsZero() {
header.Set("X-Amz-Object-Lock-Retain-Until-Date", opts.RetainUntilDate.Format(time.RFC3339))
header.Set(amzLockRetainUntil, opts.RetainUntilDate.Format(iso8601TimeFormat))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The trailing Z in iso8601TimeFormat is a literal, so this writes the time's local wall clock stamped as UTC — a RetainUntilDate carrying a zone offset is sent as a different instant, and for west-of-UTC zones the object becomes deletable earlier than requested. Verified against a live MinIO server: a +02:00 value came back with retention two hours later than asked; the time.RFC3339 form this replaces encoded offsets correctly, and master applied the same .UTC() normalization to this exact layout in #2135.

Suggested change
header.Set(amzLockRetainUntil, opts.RetainUntilDate.Format(iso8601TimeFormat))
header.Set(amzLockRetainUntil, opts.RetainUntilDate.UTC().Format(iso8601TimeFormat))

Comment thread api-compose-object.go
if opts.Mode != RetentionMode("") && !opts.RetainUntilDate.IsZero() {
header.Set(amzLockMode, opts.Mode.String())
header.Set(amzLockRetainUntil, opts.RetainUntilDate.Format(time.RFC3339))
header.Set(amzLockRetainUntil, opts.RetainUntilDate.Format(iso8601TimeFormat))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as api-put-object.go:161 — convert to UTC before formatting with the literal-Z layout.

Suggested change
header.Set(amzLockRetainUntil, opts.RetainUntilDate.Format(iso8601TimeFormat))
header.Set(amzLockRetainUntil, opts.RetainUntilDate.UTC().Format(iso8601TimeFormat))

Comment thread constants.go
Comment on lines 57 to 61
const (
signV4Algorithm = "AWS4-HMAC-SHA256"
iso8601DateFormat = "20060102T150405Z"
iso8601TimeFormat = "2006-01-02T15:04:05.000Z"
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This block is documented as signature-related, but the new layout is used only by the object-lock header code — a reader tracing signing finds an unrelated constant, and a reader of the object-lock path never looks here. Splitting it out also gives it a place to record that the trailing Z is a literal, so values must be converted to UTC before formatting. The same layout string already exists in this package as expirationDateFormat (post-policy.go:31), so pointing both call sites at one shared constant is an option too — kept separate here to match the MinIO server's own iso8601TimeFormat name; your call.

Suggested change
const (
signV4Algorithm = "AWS4-HMAC-SHA256"
iso8601DateFormat = "20060102T150405Z"
iso8601TimeFormat = "2006-01-02T15:04:05.000Z"
)
const (
signV4Algorithm = "AWS4-HMAC-SHA256"
iso8601DateFormat = "20060102T150405Z"
)
// iso8601TimeFormat is the millisecond-precision ISO 8601 layout used for
// S3 object-lock headers. The trailing 'Z' is a literal, so values must be
// converted to UTC before formatting.
const iso8601TimeFormat = "2006-01-02T15:04:05.000Z"

Comment thread api-put-object.go
@@ -158,7 +158,7 @@ func (opts PutObjectOptions) Header() (header http.Header) {
}

if !opts.RetainUntilDate.IsZero() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing in the tree asserts the header value these options produce, which is how a silent instant shift passes every existing test. This table test — appended to api-put-object_test.go, with "time" added to its imports — fails on the current branch for the two offset cases and passes once the .UTC() fixes above are applied.

func TestRetainUntilDateHeader(t *testing.T) {
	testCases := []struct {
		name string
		nsec int
		loc  *time.Location
		want string
	}{
		{
			name: "utc",
			nsec: 123456789,
			loc:  time.UTC,
			want: "2030-01-02T12:00:00.123Z",
		},
		{
			name: "utc-zero-fraction",
			nsec: 0,
			loc:  time.UTC,
			want: "2030-01-02T12:00:00.000Z",
		},
		{
			// 12:00 at +05:30 is 06:30 UTC. The header layout ends in a
			// literal 'Z', so the value must be normalized to UTC first.
			name: "positive-offset",
			nsec: 123456789,
			loc:  time.FixedZone("UTC+05:30", 5*3600+1800),
			want: "2030-01-02T06:30:00.123Z",
		},
		{
			// 12:00 at -08:00 is 20:00 UTC. Getting this wrong shortens
			// the retention period, making the object deletable early.
			name: "negative-offset",
			nsec: 123456789,
			loc:  time.FixedZone("UTC-08:00", -8*3600),
			want: "2030-01-02T20:00:00.123Z",
		},
	}

	for _, tc := range testCases {
		t.Run(tc.name, func(t *testing.T) {
			retainUntil := time.Date(2030, time.January, 2, 12, 0, 0, tc.nsec, tc.loc)

			putHdr := PutObjectOptions{
				Mode:            Governance,
				RetainUntilDate: retainUntil,
			}.Header()
			if got := putHdr.Get(amzLockRetainUntil); got != tc.want {
				t.Errorf("PutObjectOptions.Header()[%s] = %q, want %q", amzLockRetainUntil, got, tc.want)
			}

			copyHdr := http.Header{}
			CopyDestOptions{
				Bucket:          "bucket",
				Object:          "object",
				Mode:            Governance,
				RetainUntilDate: retainUntil,
			}.Marshal(copyHdr)
			if got := copyHdr.Get(amzLockRetainUntil); got != tc.want {
				t.Errorf("CopyDestOptions.Marshal()[%s] = %q, want %q", amzLockRetainUntil, got, tc.want)
			}
		})
	}
}

@allanrogerr

allanrogerr commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Proposed title: fix: send Object Lock retain-until-date as UTC with millisecond precision

Proposed description:

Some S3-compatible services reject the `X-Amz-Object-Lock-Retain-Until-Date` header produced by `time.RFC3339`, which renders whole seconds without a fraction and non-UTC times with a numeric zone offset. OVHcloud Object Storage answers `The retain until date must be provided in ISO 8601 format`; AWS S3 and MinIO accept both forms.

This changes `PutObjectOptions` and `CopyDestOptions` to send the millisecond-precision UTC form `2006-01-02T15:04:05.000Z` — the same layout the MinIO server itself uses. The value is converted to UTC before formatting because the trailing `Z` in that layout is a literal. The new form is sent to every endpoint. MinIO was verified live to accept it, and AWS S3 is expected to, since AWS's own SDKs emit fractional forms of the same layout — so this widens compatibility without changing what compliant servers store.

The current description says RFC3339 is used "instead of the ISO 8601 format", but time.RFC3339 output is valid ISO 8601 — for a whole-second UTC value it is byte-identical to what aws-sdk-go-v2 sends. What actually fails is one stricter service, and naming it (plus the exact shapes it rejects) is what makes the change reviewable — the first question asked here was "which server?".

One dependency: the sentence about converting to UTC describes the branch with the two inline .UTC() suggestions applied — the layout's trailing Z is a literal, so without them a zone-offset value is sent as the wrong instant. Apply those first, then this description is accurate. The rewrite also states plainly that the new format goes to every endpoint unconditionally, which is a behavior change worth having on the record.

One more gap worth recording while describing scope: PutObjectRetention serializes the same retain-until-date into its request XML through encoding/xml's default time.Time rendering (api-object-retention.go:38), which preserves zone offsets and nanoseconds — measured output for a -08:00 value is 2030-01-02T12:00:00.123456789-08:00. PutObjectFanOut JSON-encodes it the same way (api-put-object-fan-out.go:45), though that API is MinIO-specific so the stricter-server case doesn't arise there. A server that rejects the offset form for PutObject will reject the retention XML too, and this PR touches neither path, so the description shouldn't read as fixing the whole SDK.

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.

3 participants