Skip to content

Commit d886a41

Browse files
Ensure thrift field IDs are within range (#302)
* Ensure thrift field IDs are within range Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com> * fmt Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com> --------- Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
1 parent efd6a64 commit d886a41

File tree

1 file changed

+86
-0
lines changed

1 file changed

+86
-0
lines changed
Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
import * as fs from 'fs';
2+
import * as path from 'path';
3+
import { expect } from 'chai';
4+
5+
/**
6+
* Validates that all Thrift-generated classes comply with field ID constraints.
7+
*
8+
* Field IDs in Thrift must stay below 3329 to avoid conflicts with reserved ranges and ensure
9+
* compatibility with various Thrift implementations and protocols.
10+
*/
11+
describe('Thrift Field ID Validation', () => {
12+
const MAX_ALLOWED_FIELD_ID = 3329;
13+
const THRIFT_DIR = path.join(__dirname, '../../thrift');
14+
15+
it('should ensure all Thrift field IDs are within allowed range', () => {
16+
const violations: string[] = [];
17+
18+
// Get all JavaScript files in the thrift directory
19+
const thriftFiles = fs
20+
.readdirSync(THRIFT_DIR)
21+
.filter((file) => file.endsWith('.js'))
22+
.map((file) => path.join(THRIFT_DIR, file));
23+
24+
expect(thriftFiles.length).to.be.greaterThan(0, 'No Thrift JavaScript files found');
25+
26+
for (const filePath of thriftFiles) {
27+
const fileName = path.basename(filePath);
28+
const fileContent = fs.readFileSync(filePath, 'utf8');
29+
30+
// Extract field IDs from both read and write functions
31+
const fieldIds = extractFieldIds(fileContent);
32+
33+
for (const fieldId of fieldIds) {
34+
if (fieldId >= MAX_ALLOWED_FIELD_ID) {
35+
violations.push(
36+
`${fileName}: Field ID ${fieldId} exceeds maximum allowed value of ${MAX_ALLOWED_FIELD_ID - 1}`,
37+
);
38+
}
39+
}
40+
}
41+
42+
if (violations.length > 0) {
43+
const errorMessage = [
44+
`Found Thrift field IDs that exceed the maximum allowed value of ${MAX_ALLOWED_FIELD_ID - 1}.`,
45+
'This can cause compatibility issues and conflicts with reserved ID ranges.',
46+
'Violations found:',
47+
...violations.map((v) => ` - ${v}`),
48+
].join('\n');
49+
50+
throw new Error(errorMessage);
51+
}
52+
});
53+
});
54+
55+
/**
56+
* Extracts all field IDs from the given Thrift JavaScript file content.
57+
* Looks for field IDs in both read functions (case statements) and write functions (writeFieldBegin calls).
58+
*/
59+
function extractFieldIds(fileContent: string): number[] {
60+
const fieldIds = new Set<number>();
61+
62+
// Pattern 1: Extract field IDs from case statements in read functions
63+
// Example: case 1281:
64+
const casePattern = /case\s+(\d+):/g;
65+
let match;
66+
67+
while ((match = casePattern.exec(fileContent)) !== null) {
68+
const fieldId = parseInt(match[1], 10);
69+
if (!isNaN(fieldId)) {
70+
fieldIds.add(fieldId);
71+
}
72+
}
73+
74+
// Pattern 2: Extract field IDs from writeFieldBegin calls in write functions
75+
// Example: output.writeFieldBegin('errorDetailsJson', Thrift.Type.STRING, 1281);
76+
const writeFieldPattern = /writeFieldBegin\([^,]+,\s*[^,]+,\s*(\d+)\)/g;
77+
78+
while ((match = writeFieldPattern.exec(fileContent)) !== null) {
79+
const fieldId = parseInt(match[1], 10);
80+
if (!isNaN(fieldId)) {
81+
fieldIds.add(fieldId);
82+
}
83+
}
84+
85+
return Array.from(fieldIds).sort((a, b) => a - b);
86+
}

0 commit comments

Comments
 (0)