s3api: address review feedback round 3

- evaluator.go: use expectedExpiryTime for MPU abort cutoff to match
  AWS midnight-UTC semantics instead of raw time addition
- s3api_policy.go: fix Filter MarshalXML double-emission of size
  filters when And is set (And already contains its own size fields)
- s3api_policy_test.go: add bounds checks before accessing lc.Rules[0]
This commit is contained in:
Copilot
2026-03-28 11:28:53 -07:00
parent 2f84fdfc8d
commit 91b834bd9d
3 changed files with 29 additions and 9 deletions
+19 -8
View File
@@ -87,6 +87,7 @@ func (f Filter) MarshalXML(e *xml.Encoder, start xml.StartElement) error {
return err
}
if f.andSet {
// And already contains its own size filters; don't emit at Filter level.
if err := e.EncodeElement(f.And, xml.StartElement{Name: xml.Name{Local: "And"}}); err != nil {
return err
}
@@ -94,19 +95,29 @@ func (f Filter) MarshalXML(e *xml.Encoder, start xml.StartElement) error {
if err := e.EncodeElement(f.Tag, xml.StartElement{Name: xml.Name{Local: "Tag"}}); err != nil {
return err
}
if f.ObjectSizeGreaterThan > 0 {
if err := e.EncodeElement(f.ObjectSizeGreaterThan, xml.StartElement{Name: xml.Name{Local: "ObjectSizeGreaterThan"}}); err != nil {
return err
}
}
if f.ObjectSizeLessThan > 0 {
if err := e.EncodeElement(f.ObjectSizeLessThan, xml.StartElement{Name: xml.Name{Local: "ObjectSizeLessThan"}}); err != nil {
return err
}
}
} else {
if err := e.EncodeElement(f.Prefix, xml.StartElement{Name: xml.Name{Local: "Prefix"}}); err != nil {
return err
}
}
if f.ObjectSizeGreaterThan > 0 {
if err := e.EncodeElement(f.ObjectSizeGreaterThan, xml.StartElement{Name: xml.Name{Local: "ObjectSizeGreaterThan"}}); err != nil {
return err
if f.ObjectSizeGreaterThan > 0 {
if err := e.EncodeElement(f.ObjectSizeGreaterThan, xml.StartElement{Name: xml.Name{Local: "ObjectSizeGreaterThan"}}); err != nil {
return err
}
}
}
if f.ObjectSizeLessThan > 0 {
if err := e.EncodeElement(f.ObjectSizeLessThan, xml.StartElement{Name: xml.Name{Local: "ObjectSizeLessThan"}}); err != nil {
return err
if f.ObjectSizeLessThan > 0 {
if err := e.EncodeElement(f.ObjectSizeLessThan, xml.StartElement{Name: xml.Name{Local: "ObjectSizeLessThan"}}); err != nil {
return err
}
}
}
return e.EncodeToken(xml.EndElement{Name: start.Name})
+9
View File
@@ -69,6 +69,9 @@ func TestLifecycleXMLRoundTrip_AbortIncompleteMultipartUpload(t *testing.T) {
t.Fatalf("unmarshal: %v", err)
}
if len(lc.Rules) != 1 {
t.Fatalf("expected 1 rule, got %d", len(lc.Rules))
}
rule := lc.Rules[0]
if rule.AbortIncompleteMultipartUpload.DaysAfterInitiation != 7 {
t.Errorf("expected DaysAfterInitiation=7, got %d", rule.AbortIncompleteMultipartUpload.DaysAfterInitiation)
@@ -100,6 +103,9 @@ func TestLifecycleXMLRoundTrip_FilterWithTag(t *testing.T) {
t.Fatalf("unmarshal: %v", err)
}
if len(lc.Rules) != 1 {
t.Fatalf("expected 1 rule, got %d", len(lc.Rules))
}
rule := lc.Rules[0]
if !rule.Filter.tagSet {
t.Error("expected Filter.tagSet to be true")
@@ -132,6 +138,9 @@ func TestLifecycleXMLRoundTrip_FilterWithAnd(t *testing.T) {
t.Fatalf("unmarshal: %v", err)
}
if len(lc.Rules) != 1 {
t.Fatalf("expected 1 rule, got %d", len(lc.Rules))
}
rule := lc.Rules[0]
if !rule.Filter.andSet {
t.Error("expected Filter.andSet to be true")
+1 -1
View File
@@ -107,7 +107,7 @@ func EvaluateMPUAbort(rules []Rule, uploadKey string, createdAt time.Time, now t
if !matchesPrefix(rule.Prefix, uploadKey) {
continue
}
cutoff := createdAt.Add(time.Duration(rule.AbortMPUDaysAfterInitiation) * 24 * time.Hour)
cutoff := expectedExpiryTime(createdAt, rule.AbortMPUDaysAfterInitiation)
if !now.Before(cutoff) {
return EvalResult{Action: ActionAbortMultipartUpload, RuleID: rule.ID}
}