From: Lumir Sliva <61183145+lumir-sliva@users.noreply.github.com> Date: Mon, 22 Jun 2026 15:20:24 +0000 (+0200) Subject: rgw/lc: report x-amz-expiration only for the current version X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=refs%2Fpull%2F69642%2Fhead;p=ceph.git rgw/lc: report x-amz-expiration only for the current version The x-amz-expiration response header describes the current-version Expiration lifecycle action for an object. s3_expiration_header() instead selected between the Expiration and the NoncurrentVersionExpiration rule based on whether the object key carried an instance id: const LCExpiration& rule_expiration = (obj_key.instance.empty()) ? expiration : noncur_expiration; This was wrong in two ways. OLH resolution writes the current version's instance id into the key before the header is computed, so even a plain GET/HEAD of the current version (no versionId) was treated as a non-current request and reported a NoncurrentVersionExpiration rule. And x-amz-expiration is not meant to surface NoncurrentVersionExpiration at all. A versioned bucket whose only rule was NoncurrentVersionExpiration therefore returned a bogus expiry-date and rule-id on the current object, for both a plain request and a versionId request. Compute the header from the current-version Expiration rule only, and return no header when the request targets a specific version. The header helper now passes the client request key (s->object_key) rather than the OLH-resolved key, so a plain GET/HEAD of the current version is no longer misclassified as a versioned request. Add a unit test covering a noncurrent-only policy (no header for either a plain or a versioned request) and a combined policy (the current Expiration rule for the current version, no header for a versionId request). Tracker: https://tracker.ceph.com/issues/77564 Reported-by: Saravanan Palanisamy Signed-off-by: Lumir Sliva <61183145+lumir-sliva@users.noreply.github.com> --- diff --git a/src/rgw/rgw_lc.cc b/src/rgw/rgw_lc.cc index fdfbbc5abf25..b9383cd7a493 100644 --- a/src/rgw/rgw_lc.cc +++ b/src/rgw/rgw_lc.cc @@ -3239,6 +3239,14 @@ std::string s3_expiration_header( RGWLifecycleConfiguration config(cct); std::string hdr{""}; + /* The x-amz-expiration header reports the current-version Expiration + * lifecycle action for the object. It must never be derived from a + * NoncurrentVersionExpiration rule, and it does not apply to a request that + * targets a specific version: a non-empty instance here means the caller + * asked for a particular version by versionId, so no header is returned. */ + if (!obj_key.instance.empty()) + return hdr; + const auto& aiter = bucket_attrs.find(RGW_ATTR_LC); if (aiter == bucket_attrs.end()) return hdr; @@ -3273,16 +3281,12 @@ std::string s3_expiration_header( auto& filter = rule.get_filter(); auto& prefix = filter.has_prefix() ? filter.get_prefix(): rule.get_prefix(); auto& expiration = rule.get_expiration(); - auto& noncur_expiration = rule.get_noncur_expiration(); ldpp_dout(dpp, 10) << "rule: " << ri.first << " prefix: " << prefix << " expiration: " << " date: " << expiration.get_date() << " days: " << expiration.get_days() - << " noncur_expiration: " - << " date: " << noncur_expiration.get_date() - << " days: " << noncur_expiration.get_days() << dendl; /* skip if rule !enabled @@ -3324,10 +3328,9 @@ std::string s3_expiration_header( continue; } - // compute a uniform expiration date + // compute a uniform expiration date (current-version Expiration only) boost::optional rule_expiration_date; - const LCExpiration& rule_expiration = - (obj_key.instance.empty()) ? expiration : noncur_expiration; + const LCExpiration& rule_expiration = expiration; if (rule_expiration.has_date()) { rule_expiration_date = diff --git a/src/rgw/rgw_rest_s3.cc b/src/rgw/rgw_rest_s3.cc index e0d1fc47e792..3e0b0ec3c45a 100644 --- a/src/rgw/rgw_rest_s3.cc +++ b/src/rgw/rgw_rest_s3.cc @@ -122,8 +122,14 @@ static inline std::string get_s3_expiration_header( req_state* s, const ceph::real_time& mtime) { + /* Use the client-supplied request key (s->object_key), not the + * OLH-resolved key. OLH resolution fills in the current version's instance + * id even when the client did not request a specific version, which would + * otherwise make every versioned-bucket response look like a versionId + * request. s3_expiration_header() keys its current-version decision off an + * empty instance. */ return rgw::lc::s3_expiration_header( - s, s->object->get_key(), s->tagset, mtime, s->bucket_attrs); + s, s->object_key, s->tagset, mtime, s->bucket_attrs); } static inline bool get_s3_multipart_abort_header( diff --git a/src/test/rgw/test_rgw_lc.cc b/src/test/rgw/test_rgw_lc.cc index ab5297709afc..ced9bebcee37 100644 --- a/src/test/rgw/test_rgw_lc.cc +++ b/src/test/rgw/test_rgw_lc.cc @@ -4,6 +4,9 @@ #include "rgw_xml.h" #include "rgw_lc.h" #include "rgw_lc_s3.h" +#include "rgw_common.h" +#include "common/ceph_context.h" +#include "common/dout.h" #include #include #include @@ -419,3 +422,78 @@ TEST_F(LCWorkTimeTests, ScheduleNextStartTime) run_schedule_next_start_time_test(test_values_to_expectations); } + +// Build bucket attrs holding an encoded lifecycle configuration. +static std::map +make_lc_attrs(const RGWLifecycleConfiguration& cfg) +{ + std::map attrs; + bufferlist bl; + cfg.encode(bl); + attrs[RGW_ATTR_LC] = std::move(bl); + return attrs; +} + +static LCRule make_rule(const std::string& id, const std::string* exp_days, + const std::string* noncur_days) +{ + LCRule rule; + rule.set_id(id); + rule.set_status("Enabled"); + if (exp_days) { + LCExpiration exp; + exp.set_days(*exp_days); + rule.set_expiration(exp); + } + if (noncur_days) { + LCExpiration ncexp; + ncexp.set_days(*noncur_days); + rule.set_noncur_expiration(ncexp); + } + return rule; +} + +// x-amz-expiration must reflect only the current-version Expiration action, and +// only for the current version (an empty instance). It must never be derived +// from NoncurrentVersionExpiration, nor returned for an explicit versionId +// request. Reproducer for https://tracker.ceph.com/issues/77564. +TEST(ExpHdr, S3ExpirationHeaderCurrentVersionOnly) +{ + boost::intrusive_ptr cct{ + new CephContext(CEPH_ENTITY_TYPE_ANY), false}; + NoDoutPrefix dpp(cct.get(), ceph_subsys_rgw); + RGWObjTags notags; + auto mtime = ceph::real_clock::now(); + + const std::string days30{"30"}, days60{"60"}; + const rgw_obj_key current_key{"obj"}; + const rgw_obj_key versioned_key{"obj", "versionid-0123456789"}; + + // (1) only NoncurrentVersionExpiration: never emit the header (the bug). + { + RGWLifecycleConfiguration cfg(cct.get()); + cfg.add_rule(make_rule("noncur-only", nullptr, &days60)); + auto attrs = make_lc_attrs(cfg); + + EXPECT_TRUE(rgw::lc::s3_expiration_header( + &dpp, current_key, notags, mtime, attrs).empty()); + EXPECT_TRUE(rgw::lc::s3_expiration_header( + &dpp, versioned_key, notags, mtime, attrs).empty()); + } + + // (2) Expiration + NoncurrentVersionExpiration: emit for the current version + // from the Expiration rule only; nothing for an explicit versionId request. + { + RGWLifecycleConfiguration cfg(cct.get()); + cfg.add_rule(make_rule("cur-and-noncur", &days30, &days60)); + auto attrs = make_lc_attrs(cfg); + + auto cur = rgw::lc::s3_expiration_header( + &dpp, current_key, notags, mtime, attrs); + EXPECT_FALSE(cur.empty()); + EXPECT_NE(cur.find("rule-id=\"cur-and-noncur\""), std::string::npos); + + EXPECT_TRUE(rgw::lc::s3_expiration_header( + &dpp, versioned_key, notags, mtime, attrs).empty()); + } +}