Skip to content

Commit 939d18d

Browse files
danieljbrucegoogle-labs-jules[bot]gemini-code-assist[bot]
authored
fix: Upgrade protobufjs and fix legacy key decoding in Datastore (#8088)
* fix: upgrade protobufjs and fix legacy key decoding Upgrades protobufjs to ^7.5.5 to address security vulnerabilities. Introduces a fix for legacy App Engine key decoding by explicitly calling .setup() on Reference, Path, and Element types. This ensures that group support (wire types 3 and 4) is correctly initialized in newer versions of protobufjs, which is required for these legacy keys. Fixes the "invalid wire type 4" error in unit tests. Co-authored-by: danieljbruce <8935272+danieljbruce@users.noreply.github.com> * fix: upgrade protobufjs and fix legacy key decoding Upgrades protobufjs to ^7.5.5 to address security vulnerabilities (GHSA-xq3m-2v4x-88gg). Resolves a functional regression in decoding legacy App Engine keys by explicitly calling .setup() on Reference, Path, and Element types. This forces the generation of optimized decoders that correctly support proto2 groups, preventing "invalid wire type 4" errors. Adds 10+ regression tests for legacyDecode covering various scenarios. Co-authored-by: danieljbruce <8935272+danieljbruce@users.noreply.github.com> * fix: upgrade protobufjs and fix legacy key decoding with inverse tests Upgrades protobufjs to ^7.5.5 to address security vulnerabilities (GHSA-xq3m-2v4x-88gg). Resolves a functional regression in decoding legacy App Engine keys by explicitly calling .setup() on Reference, Path, and Element types. This forces the generation of optimized decoders that correctly support proto2 groups, preventing "invalid wire type 4" errors. Adds 10+ regression tests for legacyDecode and updates all legacy key tests to verify that legacyDecode and legacyEncode are inverses of each other. Co-authored-by: danieljbruce <8935272+danieljbruce@users.noreply.github.com> * Apply suggestion from @gemini-code-assist[bot] Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * fix: Upgrade protobufjs and fix legacy key decoding in Datastore * Remove the as any cast * Add a describe block for the encode/decode tests --------- Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com> Co-authored-by: danieljbruce <8935272+danieljbruce@users.noreply.github.com> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
1 parent 682c5dd commit 939d18d

3 files changed

Lines changed: 84 additions & 1 deletion

File tree

‎handwritten/datastore/package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@
5252
"extend": "^3.0.2",
5353
"google-gax": "^5.0.2-rc.1",
5454
"is": "^3.3.0",
55-
"protobufjs": "7.4.0",
55+
"protobufjs": "^7.5.5",
5656
"split-array-stream": "^2.0.0",
5757
"stream-events": "^1.0.5"
5858
},

‎handwritten/datastore/src/entity.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1302,6 +1302,14 @@ export namespace entity {
13021302
path.join(__dirname, '..', 'protos', 'app_engine_key.proto'),
13031303
);
13041304
loadedRoot.resolveAll();
1305+
1306+
// Ensure all types are initialized and setup correctly.
1307+
// In newer versions of protobufjs, this ensures group support
1308+
// is preserved for legacy keys.
1309+
for (const typeName of ['Reference', 'Path', 'Element']) {
1310+
loadedRoot.lookupType(typeName).setup();
1311+
}
1312+
13051313
return loadedRoot.nested;
13061314
}
13071315

‎handwritten/datastore/test/entity.ts‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1800,6 +1800,10 @@ describe('entity', () => {
18001800
const key = urlSafeKey.legacyDecode(encodedKey);
18011801
assert.strictEqual(key.namespace, 'NS');
18021802
assert.deepStrictEqual(key.path, ['Task', 'sampletask1']);
1803+
assert.strictEqual(
1804+
urlSafeKey.legacyEncode(PROJECT_ID, key, LOCATION_PREFIX),
1805+
encodedKey,
1806+
);
18031807
});
18041808

18051809
it('should decode key with single path element string type', () => {
@@ -1808,6 +1812,10 @@ describe('entity', () => {
18081812
const key = urlSafeKey.legacyDecode(encodedKey);
18091813
assert.strictEqual(key.namespace, undefined);
18101814
assert.deepStrictEqual(key.path, ['Task', 'sampletask1']);
1815+
assert.strictEqual(
1816+
urlSafeKey.legacyEncode(PROJECT_ID, key),
1817+
encodedKey,
1818+
);
18111819
});
18121820

18131821
it('should decode key with single path element long int type', () => {
@@ -1816,6 +1824,10 @@ describe('entity', () => {
18161824
const key = urlSafeKey.legacyDecode(encodedKey);
18171825
assert.strictEqual(key.namespace, undefined);
18181826
assert.deepStrictEqual(key.path, ['Task', '5754248394440704']);
1827+
assert.strictEqual(
1828+
urlSafeKey.legacyEncode(PROJECT_ID, key, LOCATION_PREFIX),
1829+
encodedKey,
1830+
);
18191831
});
18201832

18211833
it('should decode key with parent path', () => {
@@ -1831,6 +1843,69 @@ describe('entity', () => {
18311843
]);
18321844
assert.strictEqual(key.parent!.name, 'sampletask1');
18331845
assert.deepStrictEqual(key.parent!.path, ['Task', 'sampletask1']);
1846+
assert.strictEqual(
1847+
urlSafeKey.legacyEncode(PROJECT_ID, key, LOCATION_PREFIX),
1848+
encodedKey,
1849+
);
1850+
});
1851+
1852+
describe('should ensure that decode inverses encode and decoding is correct', () => {
1853+
const TEST_PROJECT = 'test-project';
1854+
const testCases = [
1855+
{name: 'numeric ID', path: ['Kind', '123']},
1856+
{name: 'string name with spaces', path: ['Kind', 'name with spaces']},
1857+
{name: 'special characters', path: ['Kind', 'special!@#$%^&*()']},
1858+
{
1859+
name: '3-level parent',
1860+
path: ['Grandparent', '1', 'Parent', 'p1', 'Child', '2'],
1861+
},
1862+
{
1863+
name: 'namespace and numeric ID',
1864+
path: ['Kind', '456'],
1865+
namespace: 'MyNS',
1866+
},
1867+
{
1868+
name: 'namespace and parent',
1869+
path: ['Parent', '1', 'Child', 'c1'],
1870+
namespace: 'MyNS',
1871+
},
1872+
{
1873+
name: 'long integer ID as string',
1874+
path: ['Kind', '9223372036854775807'],
1875+
},
1876+
{name: 'kind with hyphens', path: ['My-Kind', '1']},
1877+
{
1878+
name: 'different kinds in path',
1879+
path: ['User', 'user1', 'Post', '100', 'Comment', 'c1'],
1880+
},
1881+
{
1882+
name: 'same kinds in path',
1883+
path: ['Node', '1', 'Node', '2', 'Node', '3'],
1884+
},
1885+
];
1886+
1887+
testCases.forEach(tc => {
1888+
it(`should decode and re-encode ${tc.name} correctly`, () => {
1889+
const key = new testEntity.Key({
1890+
path: tc.path,
1891+
namespace: tc.namespace,
1892+
});
1893+
const encoded = urlSafeKey.legacyEncode(
1894+
TEST_PROJECT,
1895+
key,
1896+
LOCATION_PREFIX,
1897+
);
1898+
const decoded = urlSafeKey.legacyDecode(encoded);
1899+
assert.strictEqual(decoded.namespace, tc.namespace);
1900+
assert.deepStrictEqual(decoded.path, tc.path);
1901+
const reEncoded = urlSafeKey.legacyEncode(
1902+
TEST_PROJECT,
1903+
decoded,
1904+
LOCATION_PREFIX,
1905+
);
1906+
assert.strictEqual(reEncoded, encoded);
1907+
});
1908+
});
18341909
});
18351910
});
18361911
});

0 commit comments

Comments
 (0)