Skip to content

Commit e3f03ac

Browse files
Ensure thrift field IDs are within range
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
1 parent 0d012c0 commit e3f03ac

File tree

1 file changed

+83
-0
lines changed

1 file changed

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

0 commit comments

Comments
 (0)