Skip to content

Commit 7a60e0c

Browse files
committed
Drop 24-hex ObjectId guess; EJSON.parse handles ObjectId via $oid form
1 parent ba8ea73 commit 7a60e0c

2 files changed

Lines changed: 19 additions & 24 deletions

File tree

src/documentdb/utils/parseDocumentId.test.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,18 +42,20 @@ describe('parseDocumentId', () => {
4242
outputChannelError.mockClear();
4343
});
4444

45-
it('parses an ObjectId from canonical EJSON', () => {
45+
it('parses an ObjectId from its EJSON $oid form (no hex guessing needed)', () => {
4646
const oid = new ObjectId();
47+
// This is exactly how the UI serializes an ObjectId _id: {"$oid":"..."}.
48+
expect(canonical(oid)).toBe(`{"$oid":"${oid.toHexString()}"}`);
49+
4750
const result = parseDocumentId(canonical(oid));
4851
expect(result).toBeInstanceOf(ObjectId);
4952
expect((result as ObjectId).toHexString()).toBe(oid.toHexString());
5053
});
5154

52-
it('parses a bare 24-character hex string as an ObjectId (legacy shape)', () => {
55+
it('does NOT guess: a bare 24-character hex string (no $oid) throws', () => {
5356
const hex = 'a'.repeat(24);
54-
const result = parseDocumentId(hex);
55-
expect(result).toBeInstanceOf(ObjectId);
56-
expect((result as ObjectId).toHexString()).toBe(hex);
57+
expect(() => parseDocumentId(hex)).toThrow(/Unable to parse the document _id/);
58+
expect(outputChannelError).toHaveBeenCalledTimes(1);
5759
});
5860

5961
it('keeps a string _id as a string', () => {

src/documentdb/utils/parseDocumentId.ts

Lines changed: 12 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55

66
import * as l10n from '@vscode/l10n';
77
import { EJSON } from 'bson';
8-
import { ObjectId } from 'mongodb';
98
import { ext } from '../../extensionVariables';
109

1110
/**
@@ -22,30 +21,24 @@ const MAX_LOGGED_ID_LENGTH = 256;
2221
* The id is expected to be Extended JSON (EJSON) produced by `EJSON.stringify`,
2322
* which is the BSON library's own canonical/relaxed serialization. `EJSON.parse`
2423
* is therefore the ground truth: it round-trips every `_id` type the driver
25-
* supports — `ObjectId`, string, number, `UUID`, `Date`, `Decimal128`,
26-
* **embedded documents** (e.g. `{ author: 'John', userId: 2343345 }`) and
27-
* arrays — preserving field order, which is significant when the driver matches
28-
* an embedded-document `_id`.
24+
* supports — `ObjectId` (as `{"$oid":"…"}`), string, number, `UUID`, `Date`,
25+
* `Decimal128`, **embedded documents** (e.g. `{ author: 'John', userId: 2343345 }`)
26+
* and arrays — preserving field order, which is significant when the driver
27+
* matches an embedded-document `_id`.
2928
*
30-
* As a single deterministic concession we also accept a bare 24-character hex
31-
* string and construct an `ObjectId` from it (a legacy, non-EJSON id shape).
32-
*
33-
* If the id maps to neither, we **throw** rather than guess. Silently coercing
34-
* an unrecognized id into a raw string would change which document the driver
35-
* matches and could read or delete the wrong record, so we fail loudly instead.
36-
* The offending value (length-capped) is written to the output channel for
37-
* follow-up.
29+
* If the id cannot be parsed, we **throw** rather than guess. Silently coercing
30+
* an unrecognized id into a raw string (or assuming it is an ObjectId) would
31+
* change which document the driver matches and could read or delete the wrong
32+
* record, so we fail loudly instead. The offending value (length-capped) is
33+
* written to the output channel for follow-up.
3834
*
3935
* @throws Error if the id cannot be interpreted as a BSON value the driver understands.
4036
*/
4137
export function parseDocumentId(id: string): unknown {
4238
try {
4339
return EJSON.parse(id);
4440
} catch {
45-
// Not Extended JSON. The only other shape we accept is a bare 24-char hex ObjectId.
46-
if (ObjectId.isValid(id)) {
47-
return new ObjectId(id);
48-
}
41+
// Not Extended JSON — fall through to fail loudly below rather than guessing.
4942
}
5043

5144
const preview = id.length > MAX_LOGGED_ID_LENGTH ? id.slice(0, MAX_LOGGED_ID_LENGTH) : id;
@@ -55,8 +48,8 @@ export function parseDocumentId(id: string): unknown {
5548

5649
ext.outputChannel.error(
5750
'Unable to parse document _id. The value could not be interpreted as a BSON type understood ' +
58-
'by the driver (expected Extended JSON — e.g. {"$oid":"…"}, {"$numberInt":"…"}, an embedded ' +
59-
`document — or a 24-character hex ObjectId). Received _id: ${detail}`,
51+
'by the driver (expected Extended JSON — e.g. {"$oid":"…"}, {"$numberInt":"…"}, or an ' +
52+
`embedded document). Received _id: ${detail}`,
6053
);
6154

6255
throw new Error(l10n.t('Unable to parse the document _id. See the output channel for details.'));

0 commit comments

Comments
 (0)