Skip to content

Commit b99e844

Browse files
author
Adedoyin Ogunjobi
committed
add feature to prevent invalid tunables
changes to scripts.js dont let users save invalud key or value move tunables.json and add feature Document allowed tunables and error handling Added section on allowed tunables and their validation process. Exclude tunables.json from RAT and prettier
1 parent 7c99986 commit b99e844

7 files changed

Lines changed: 193 additions & 12 deletions

File tree

constants/tunables.json

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
{
2+
"allowExpressionResultCoercion": "boolean",
3+
"allowExternalPathExpressions": "boolean",
4+
"allowSignedIntegerLength1Bit": "boolean",
5+
6+
"blobChunkSizeInBytes": "number",
7+
8+
"defaultEmptyElementParsePolicy": "string",
9+
10+
"escalateWarningsToErrors": "boolean",
11+
12+
"generatedNamespacePrefixStem": "string",
13+
14+
"initialElementOccurrencesHint": "number",
15+
"initialRegexMatchLimitInCharacters": "number",
16+
17+
"infosetWalkerSkipMin": "number",
18+
"infosetWalkerSkipMax": "number",
19+
20+
"invalidRestrictionPolicy": "string",
21+
22+
"maxBinaryDecimalVirtualPoint": "number",
23+
"maxByteArrayOutputStreamBufferSizeInBytes": "number",
24+
"maxDataDumpSizeInBytes": "number",
25+
"maxHexBinaryLengthInBytes": "number",
26+
27+
"maxOccursBounds": "number",
28+
29+
"parseUnparsePolicy": "string",
30+
31+
"releaseUnneededInfoset": "boolean",
32+
33+
"requireBitOrderProperty": "boolean",
34+
"requireEmptyElementParsePolicyProperty": "boolean"
35+
}

doc/Wiki.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -372,12 +372,21 @@ Tunables are configured using **Key** and **Value** fields in the launch configu
372372

373373
Use the **“+ Add Tunable”** button to add additional entries. Each row can be removed using the **“X”** button.
374374

375+
Tunables are case sensitive, and only allowed tunables will be allowed to be saved in the wizard.
376+
375377
Tunables control runtime behavior and performance characteristics. Most tunables have default values, so you only need to specify a value if you want to override them.
376378

377379
<img width="790" height="176" alt="tunables_gui" src="https://github.com/user-attachments/assets/f78d5471-bfff-4cfb-b6cf-62c787a8e31b" />
378380

379381
<img width="360" height="177" alt="tunables_launchjson" src="https://github.com/user-attachments/assets/0d70044d-d04c-4f47-958c-175c6e67f3f9" />
380382

383+
### List of allowed Tunables
384+
385+
In order to prevent unexpected tunables from being ran, there is a list of allowed tunables at `constants/tunables.json` which contains all valid tunables. Tunables are case sensitive and the values type must match what is expected. For example boolean values will only allow true or false. If a invalid tunable is saved in the `launch.json` , when you attempt to run daffodil you will encounter a error saying `Cancel` or `Ignore invalid tunable`. Cancel will send you back where you fix the error, and Ignore invalid tunable runs daffodil but does not register the invalid tunable. The tunable will stay in your launch.json however. The tunables.json file will be updated as new tunables are added and removed.
386+
387+
<img width="914" height="718" alt="image" src="https://github.com/user-attachments/assets/22b24b29-de7e-4ec9-b012-85fe8f1bb54c" />
388+
389+
381390
### Reference
382391
- https://daffodil.apache.org/tunables/
383392

project/Rat.scala

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ object Rat {
5252
file("build/package/NOLICENSE"),
5353
file("build/package/NONOTICE"),
5454
file("src/tests/data/test.txt"),
55+
file("constants/tunables.json"),
5556
file("debugger/src/test/data/emptyData.xml"),
5657
file("debugger/src/test/data/emptyInfoset.xml"),
5758
file("debugger/src/test/data/notInfoset.xml"),

src/adapter/activateDaffodilDebug.ts

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import * as rootCompletion from '../rootCompletion'
1818
import { tmpdir } from 'os'
1919
import JSZip from 'jszip'
2020
import { rm } from 'node:fs/promises'
21+
import { getTunables } from './extension'
2122

2223
import {
2324
CancellationToken,
@@ -766,16 +767,15 @@ class DaffodilConfigurationProvider
766767
constructor(context: vscode.ExtensionContext) {
767768
this.context = context
768769
}
769-
770770
/**
771771
* Massage a debug configuration just before a debug session is being launched,
772772
* e.g. add all missing attributes to the debug configuration.
773773
*/
774-
resolveDebugConfiguration(
774+
async resolveDebugConfiguration(
775775
folder: WorkspaceFolder | undefined,
776776
config: DebugConfiguration,
777777
token?: CancellationToken
778-
): ProviderResult<DebugConfiguration> {
778+
): Promise<DebugConfiguration | undefined> {
779779
// if launch.json is missing or empty
780780
if (!config.type && !config.request && !config.name) {
781781
config = getConfig({ name: 'Launch', request: 'launch', type: 'dfdl' })
@@ -791,6 +791,31 @@ class DaffodilConfigurationProvider
791791
data: config.data || '${command:AskForDataName}',
792792
}
793793

794+
const validTunables = getTunables(this.context)
795+
const currentTunables = config.tunables ?? {}
796+
const invalidTunables = Object.keys(currentTunables).filter(
797+
(name) => !validTunables.includes(name)
798+
)
799+
800+
if (invalidTunables.length > 0) {
801+
const choice = await vscode.window.showWarningMessage(
802+
`Invalid tunables found:\n\n${invalidTunables.join('\n')}`,
803+
{ modal: true },
804+
'Ignore Invalid Tunables'
805+
)
806+
807+
// If the user dismisses the checkbox in anyway it will exit. so Cancel , Esc or unexpected values will exit
808+
// Only pressing ignore invalid tunables will proceed with the launch and remove the invalid tunables from the config sent to the debug adapter
809+
if (choice !== 'Ignore Invalid Tunables') {
810+
return undefined
811+
}
812+
//Copies the tunables , then deletes the invalid tunables from the copied config so that the user can still see the invalid tunables in their launch.json but they won't be sent to the debug adapter and cause errors
813+
config.tunables = { ...config.tunables }
814+
815+
for (const invalid of invalidTunables) {
816+
delete config.tunables[invalid]
817+
}
818+
}
794819
let dataFolder = config.data
795820

796821
if (

src/adapter/extension.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919

2020
import * as vscode from 'vscode'
2121
import * as position from '../position'
22+
import * as fs from 'fs'
23+
import * as path from 'path'
2224
import { ProviderResult } from 'vscode'
2325
import { DaffodilDebugSession } from './daffodilDebug'
2426
import {
@@ -45,6 +47,15 @@ export function deactivate() {
4547
position.deactivate()
4648
}
4749

50+
export function getTunables(context: vscode.ExtensionContext): string[] {
51+
const file = path.join(context.extensionPath, 'constants', 'tunables.json')
52+
53+
const data = fs.readFileSync(file, 'utf8')
54+
const json = JSON.parse(data)
55+
56+
return Object.keys(json)
57+
}
58+
4859
export class InlineDebugAdapterFactory
4960
implements vscode.DebugAdapterDescriptorFactory
5061
{

src/launchWizard/launchWizard.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import { DFDLDebugger } from '../classes/dfdlDebugger'
2222
import { VSCodeLaunchConfigArgs } from '../classes/vscode-launch'
2323
import { DataEditorConfig } from '../classes/dataEditor'
2424
import { parse as jsoncParse } from 'jsonc-parser'
25+
import * as path from 'path'
2526

2627
let launchWizard: LaunchWizard | undefined
2728

@@ -279,6 +280,20 @@ async function createWizard(ctx: vscode.ExtensionContext) {
279280
let panel = launchWiz.getPanel()
280281
panel.webview.html = launchWiz.getWebViewContent()
281282

283+
//Read in list of valid
284+
285+
const tunablesPath = path.join(
286+
ctx.extensionPath,
287+
'constants',
288+
'tunables.json'
289+
)
290+
291+
const tunables = JSON.parse(fs.readFileSync(tunablesPath, 'utf8'))
292+
panel.webview.postMessage({
293+
command: 'loadTunables',
294+
tunables: tunables,
295+
})
296+
282297
panel.webview.onDidReceiveMessage(
283298
async (message) => {
284299
switch (message.command) {

src/launchWizard/script.js

Lines changed: 94 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -348,6 +348,19 @@ function save() {
348348
)
349349
}
350350

351+
// Blocks save if tunable isnt valid. This means we have to always make sure the tunable list is valid.
352+
// should we just warn the user that it will case errors and let them submit
353+
validateTunablesTable()
354+
355+
const hasErrors = [
356+
...document.querySelectorAll('#tunablesTableBody .tunable-error'),
357+
].some((el) => el.textContent.trim())
358+
359+
if (hasErrors) {
360+
console.warn('Cannot save: invalid tunables present')
361+
return
362+
}
363+
351364
vscode.postMessage({
352365
command: 'saveConfig',
353366
data: JSON.stringify(obj, null, 4),
@@ -360,11 +373,14 @@ function addTunableRow() {
360373

361374
const row = document.createElement('tr')
362375

363-
row.innerHTML = `
364-
<td><input class="file-input" /></td>
365-
<td><input class="file-input" /></td>
366-
<td><button onclick="this.closest('tr').remove()">X</button></td>
367-
`
376+
row.innerHTML = row.innerHTML = `
377+
<td>
378+
<input class="file-input" />
379+
<div class="tunable-error" style="color:red;font-size:12px"></div>
380+
</td>
381+
<td><input class="file-input" /></td>
382+
<td><button onclick="this.closest('tr').remove()">X</button></td>
383+
`
368384

369385
tableBody.appendChild(row)
370386
}
@@ -403,10 +419,19 @@ function renderTunables(tunables = {}) {
403419
const row = document.createElement('tr')
404420

405421
row.innerHTML = `
406-
<td><input class="file-input" value="${escapeHtml(key)}" /></td>
407-
<td><input class="file-input" value="${escapeHtml(value)}" /></td>
408-
<td><button onclick="this.closest('tr').remove()">X</button></td>
409-
`
422+
<td>
423+
<input class="file-input" value="${escapeHtml(key)}" />
424+
<div class="tunable-error" style="color:red;font-size:12px;"></div>
425+
</td>
426+
427+
<td>
428+
<input class="file-input" value="${escapeHtml(value)}" />
429+
</td>
430+
431+
<td>
432+
<button onclick="this.closest('tr').remove()">X</button>
433+
</td>
434+
`
410435

411436
tableBody.appendChild(row)
412437
})
@@ -463,6 +488,56 @@ function renderVariables(variables = {}) {
463488
tableBody.appendChild(row)
464489
})
465490
}
491+
let VALID_TUNABLES = {}
492+
493+
// Validates tunables. validates against list of tunables and the value expected
494+
function validateTunablesTable() {
495+
const rows = document.querySelectorAll('#tunablesTableBody tr')
496+
497+
rows.forEach((row) => {
498+
const keyInput = row.children[0].querySelector('input')
499+
const valueInput = row.children[1].querySelector('input')
500+
const errorDiv = row.children[0].querySelector('.tunable-error')
501+
502+
if (!keyInput || !valueInput || !errorDiv) return
503+
504+
const key = keyInput.value.trim()
505+
const value = valueInput.value.trim()
506+
507+
let error = ''
508+
509+
//Case sensitive check for valid tunables. If not valid, check for case insensitive match and suggest that to the user.
510+
if (key && !VALID_TUNABLES[key]) {
511+
const match = Object.keys(VALID_TUNABLES).find(
512+
(t) => t.toLowerCase() === key.toLowerCase()
513+
)
514+
515+
error = match
516+
? `Invalid tunable. Did you mean "${match}"? (case sensitive)`
517+
: 'Invalid tunable'
518+
} else if (key && value) {
519+
const expectedType = VALID_TUNABLES[key]
520+
521+
if (expectedType === 'boolean') {
522+
if (value !== 'true' && value !== 'false') {
523+
error = 'Value must be boolean (true/false)'
524+
}
525+
}
526+
527+
if (expectedType === 'number') {
528+
if (isNaN(Number(value))) {
529+
error = 'Value must be a number'
530+
}
531+
}
532+
533+
if (expectedType === 'string') {
534+
// strings are always valid
535+
}
536+
}
537+
538+
errorDiv.textContent = error
539+
})
540+
}
466541

467542
// Function to copy selected config
468543
function copyConfig() {
@@ -586,6 +661,12 @@ async function updateConfigValues(config) {
586661

587662
renderTunables(config.tunables || {})
588663
renderVariables(config.variables || {})
664+
document
665+
.getElementById('tunablesTableBody')
666+
?.addEventListener('blur', validateTunablesTable, true) // remove true and change blur to input if we want this to validate on each key
667+
668+
// catches any invalid tunables on load.
669+
validateTunablesTable()
589670
updateInfosetOutputType()
590671
updateTDMLAction()
591672

@@ -615,6 +696,10 @@ async function updateDaffodilDebugClasspath(message) {
615696
const message = event.data
616697

617698
switch (message.command) {
699+
case 'loadTunables':
700+
VALID_TUNABLES = message.tunables
701+
// validateTunablesTable()
702+
break
618703
case 'updateConfValues':
619704
await updateConfigValues(message.configValues)
620705
break

0 commit comments

Comments
 (0)