Skip to content

Commit 148ca98

Browse files
fix putData : detect collision on specific provided version id
Issue: CLDSRV-953
1 parent 0d08680 commit 148ca98

2 files changed

Lines changed: 54 additions & 3 deletions

File tree

lib/routes/routeBackbeat.js

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -450,8 +450,9 @@ function putData(request, response, bucketInfo, objMd, log, callback) {
450450
errorInstances.BadRequest.customizeDescription('bad request: invalid x-scal-version-id header'),
451451
);
452452
}
453-
if (objMd && objMd.versionId === incomingVersionIdDecoded) {
454-
// Data already at destination for this version; return 409 with the existing
453+
if (objMd) {
454+
// objMd is the specific version (fetched by versionId from x-scal-version-id header)
455+
// its existence means data is already at destination. Return 409 with the existing
455456
// microVersionId so backbeat can decide if putMetadata is still needed.
456457
log.debug('crr putData: version already at destination', {
457458
method: 'putData',
@@ -2069,7 +2070,20 @@ function routeBackbeat(clientIP, request, response, log) {
20692070
});
20702071
return next(errors.InvalidArgument);
20712072
}
2072-
const versionId = decodedVidResult;
2073+
let versionId = decodedVidResult;
2074+
// For putData api: the version to check is passed
2075+
// via x-scal-version-id header, not the URL query. Fetch that specific
2076+
// version so objMd matches the replicated version, not always the master.
2077+
const isPutDataApi = request.method === 'PUT' && request.resourceType === 'data';
2078+
if (isPutDataApi) {
2079+
const versionIdHeader = request.headers['x-scal-version-id'];
2080+
if (versionIdHeader !== undefined && versionIdHeader !== 'null') {
2081+
const decoded = decode(versionIdHeader);
2082+
if (!(decoded instanceof Error)) {
2083+
versionId = decoded;
2084+
}
2085+
}
2086+
}
20732087
if (useMultipleBackend) {
20742088
if (request.resourceType === 'multiplebackendmetadata') {
20752089
return backbeatRoutes[request.method][request.resourceType](request, response, log, next);

tests/functional/backbeat/putData.js

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,43 @@ describe('putData : VersionId collision detection', () => {
106106
const output = await putData(key, { versionId: differentVersionId });
107107
assert.ok(output.Location, 'should return a Location when data is written normally');
108108
});
109+
110+
it('should throw VersionIdCollisionException when a non-current version already exists', async () => {
111+
const key = 'putdata-non-current-collision';
112+
113+
const v1Result = await s3.send(
114+
new PutObjectCommand({
115+
Bucket: TEST_BUCKET,
116+
Key: key,
117+
Body: Buffer.from(OBJECT_BODY),
118+
ContentType: 'text/plain',
119+
}),
120+
);
121+
const v1VersionId = v1Result.VersionId;
122+
assert.ok(v1VersionId, 'first PutObject should return a VersionId');
123+
124+
const v2Result = await s3.send(
125+
new PutObjectCommand({
126+
Bucket: TEST_BUCKET,
127+
Key: key,
128+
Body: Buffer.from(OBJECT_BODY),
129+
ContentType: 'text/plain',
130+
}),
131+
);
132+
assert.ok(v2Result.VersionId, 'second PutObject should return a VersionId');
133+
134+
// putData on v1 (non-current) must detect the collision, not just compare against master
135+
try {
136+
await putData(key, { versionId: v1VersionId });
137+
assert.fail('expected VersionIdCollisionException');
138+
} catch (err) {
139+
assert.ok(
140+
err instanceof VersionIdCollisionException,
141+
`expected VersionIdCollisionException, got ${err.constructor.name}`,
142+
);
143+
assert.strictEqual(err.microVersionId, '', 'microVersionId should be empty for original write state');
144+
}
145+
});
109146
});
110147

111148
describe('putData : null-version objects (ExternalNullVersionId)', () => {

0 commit comments

Comments
 (0)