feat: add encoding parameter to binary widget#1737
Merged
Conversation
Support hex (default), base64, and ascii encodings for binary widget display, record view, edit input, and filter. Adds backend validation for the encoding param on create/update and covers each encoding with frontend and AVA tests. Filter continues to emit hex on the wire to preserve the backend comparator contract. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR adds configurable binary encoding support across the application, enabling Binary widgets to handle hex, base64, and ASCII-encoded data. Backend changes add validation for encoding parameters, while frontend components are refactored to support dynamic encoding selection and conversion throughout display, editing, and filtering workflows. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds an encoding widget parameter to the Binary widget so binary values can be displayed/edited as hex (default), base64, or ascii, while keeping filter values emitted to the backend in hex.
Changes:
- Introduces binary encoding helpers (encode/decode + type guard) and expands unit tests for hex/base64/ascii behavior.
- Updates Binary display, record-view, record-edit, and filter components to respect
widget_params.encodingand update copy/tooltips accordingly. - Adds backend validation and AVA e2e coverage for accepting/rejecting Binary widget
encodingvalues.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/app/validators/base64.validator.ts | Updates base64 validator logic to regex/length-based validation. |
| frontend/src/app/lib/binary.ts | Adds encoding types/constants and base64/ascii encode/decode helpers. |
| frontend/src/app/lib/binary.spec.ts | Adds tests for base64/ascii + encoded round-trips and type guard. |
| frontend/src/app/components/ui-components/table-display-fields/binary/binary.component.ts | Displays/copies binary values using configured encoding. |
| frontend/src/app/components/ui-components/table-display-fields/binary/binary.component.html | Copies encoded value and uses encoding-aware tooltip. |
| frontend/src/app/components/ui-components/table-display-fields/binary/binary.component.spec.ts | Tests encoding behavior for table display field. |
| frontend/src/app/components/ui-components/record-view-fields/binary/binary.component.ts | Record-view display/copy uses configured encoding and truncation logic. |
| frontend/src/app/components/ui-components/record-view-fields/binary/binary.component.html | Copies encoded value and uses encoding-aware tooltip; tooltip shows full encoded when truncated. |
| frontend/src/app/components/ui-components/record-view-fields/binary/binary.component.spec.ts | Tests encoding behavior for record view field. |
| frontend/src/app/components/ui-components/record-edit-fields/binary/binary.component.ts | Edit input seeded/parsed according to encoding; emits Buffer-JSON when valid. |
| frontend/src/app/components/ui-components/record-edit-fields/binary/binary.component.html | Switches validators/placeholder/label based on encoding. |
| frontend/src/app/components/ui-components/record-edit-fields/binary/binary.component.spec.ts | Tests edit seeding and encode/decode paths per encoding. |
| frontend/src/app/components/ui-components/filter-fields/binary/binary.component.ts | Accepts typed input in selected encoding but emits hex to backend. |
| frontend/src/app/components/ui-components/filter-fields/binary/binary.component.html | Switches validators/placeholder/label based on encoding. |
| frontend/src/app/components/ui-components/filter-fields/binary/binary.component.spec.ts | Tests filter input seeding and hex-on-wire behavior per encoding. |
| frontend/src/app/components/dashboard/db-table-view/db-table-widgets/db-table-widgets.component.ts | Updates default Binary widget params docs to include encoding. |
| backend/test/ava-tests/saas-tests/table-widgets-e2e.test.ts | Adds e2e coverage for Binary widget encoding acceptance/rejection. |
| backend/src/enums/widget-type.enum.ts | Adds Binary to WidgetTypeEnum. |
| backend/src/entities/widget/utils/validate-create-widgets-ds.ts | Validates Binary widget encoding param values. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+112
to
+120
| if (widget_type && widget_type === WidgetTypeEnum.Binary) { | ||
| const rawParams = widgetDS.widget_params; | ||
| if (rawParams) { | ||
| const widget_params: Record<string, any> = | ||
| typeof rawParams === 'string' ? JSON5.parse(rawParams) : (rawParams as Record<string, any>); | ||
| if (widget_params.encoding !== undefined && !['hex', 'base64', 'ascii'].includes(widget_params.encoding)) { | ||
| errors.push(Messages.WIDGET_PARAMETER_UNSUPPORTED('encoding', WidgetTypeEnum.Binary)); | ||
| } | ||
| } |
There was a problem hiding this comment.
widget_params is JSON5-parsed here without any error handling. If a client sends an invalid JSON5 string for widget_params, JSON5.parse will throw and the endpoint will return a 500 instead of a validation error. Consider wrapping the parse in a try/catch and pushing a validation message when parsing fails (consistent with other validation errors returned from this function).
Comment on lines
+63
to
+80
| onInputChange(value: string): void { | ||
| this.rawInput = value; | ||
| this.onFieldChange.emit(this.currentHex()); | ||
| this.onComparatorChange.emit(this.filterMode); | ||
| } | ||
|
|
||
| private currentHex(): string { | ||
| if (!this.rawInput) { | ||
| this.isInvalidInput = false; | ||
| return ''; | ||
| } | ||
| const bytes = encodedToBytes(this.rawInput, this.encoding()); | ||
| if (bytes === null) { | ||
| this.isInvalidInput = true; | ||
| return ''; | ||
| } | ||
| this.isInvalidInput = false; | ||
| return bytesToHex(bytes); |
There was a problem hiding this comment.
When rawInput is invalid for the selected encoding, currentHex() returns an empty string and onInputChange() still emits that to the parent. This can unintentionally apply a filter for empty binary data (with comparator still set to eq/contains/startswith) rather than preserving the previous value or preventing filter updates until the input is valid. Consider emitting the last valid hex (or the raw input) and/or gating onFieldChange.emit when bytes === null so invalid input doesn’t translate into a real backend filter value.
Comment on lines
+3
to
+8
| export type BinaryEncoding = 'hex' | 'base64' | 'ascii'; | ||
|
|
||
| export const BINARY_ENCODINGS = ['hex', 'base64', 'ascii'] as const; | ||
|
|
||
| export function isBinaryEncoding(value: unknown): value is BinaryEncoding { | ||
| return value === 'hex' || value === 'base64' || value === 'ascii'; |
There was a problem hiding this comment.
BINARY_ENCODINGS and isBinaryEncoding() duplicate the same list of allowed values. This can drift over time if a new encoding is added. Consider deriving the type guard from BINARY_ENCODINGS (e.g., via includes) so there’s a single source of truth.
Suggested change
| export type BinaryEncoding = 'hex' | 'base64' | 'ascii'; | |
| export const BINARY_ENCODINGS = ['hex', 'base64', 'ascii'] as const; | |
| export function isBinaryEncoding(value: unknown): value is BinaryEncoding { | |
| return value === 'hex' || value === 'base64' || value === 'ascii'; | |
| export const BINARY_ENCODINGS = ['hex', 'base64', 'ascii'] as const; | |
| export type BinaryEncoding = (typeof BINARY_ENCODINGS)[number]; | |
| export function isBinaryEncoding(value: unknown): value is BinaryEncoding { | |
| return typeof value === 'string' && BINARY_ENCODINGS.includes(value as BinaryEncoding); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
backend/test/ava-tests/saas-tests/table-widgets-e2e.test.ts (1)
1705-1810: Consider parameterizing over all three accepted encodings.The accept-path currently only tests
base64. Since the validator explicitly whitelistshex,base64, andascii, parameterizing this test over the three valid values would guard against an accidental regression of the allowed list (e.g.['hex','base64']) that would still pass today.♻️ Sketch
for (const encoding of ['hex', 'base64', 'ascii'] as const) { test.serial(`${currentTest} should accept encoding=${encoding}`, async (t) => { // ... same body, parameterize widget_params and the params.encoding assertion }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/test/ava-tests/saas-tests/table-widgets-e2e.test.ts` around lines 1705 - 1810, The tests only assert that WidgetTypeEnum.Binary accepts 'base64' but the validator allows 'hex', 'base64', and 'ascii'; update the tests around the Binary widget cases (the test block names using currentTest and the CreateOrUpdateTableWidgetsDto payloads) to parameterize over the three encodings and assert each is accepted: iterate encodings = ['hex','base64','ascii'] and for each create the payload with widget_params JSON.stringify({encoding}) when posting to POST `/widget/${connectionId}?tableName=${tableNameForWidgets}`, expect 201 and verify params.encoding via JSON5.parse(ro[0].widget_params) equals the current encoding; keep the existing invalid-encoding and absent-encoding tests as-is.backend/src/entities/widget/utils/validate-create-widgets-ds.ts (1)
112-121: Extract the allowed encodings and consider flagging unknown keys.Two minor improvements for this new Binary branch:
- The allowed encoding list
['hex', 'base64', 'ascii']is hardcoded here and duplicated on the frontend (seeBinaryEncodingusage infrontend/src/app/lib/binary.ts). Consider lifting this to a single exported constant (e.g. inbackend/src/helpers/constants/constants.tsalongsideFOREIGN_KEY_FIELDS, and mirrored on the frontend) so the backend validator and frontend type cannot drift.- Unlike the
Foreign_keybranch (which flags any key not inFOREIGN_KEY_FIELDS), this branch only validatesencoding. A typo like{ encodng: 'hex' }would pass silently and be stored. Sinceencodingis the only currently-supported param, it would be cheap to reject unknown keys forBinarythe same way.♻️ Suggested refactor
if (widget_type && widget_type === WidgetTypeEnum.Binary) { const rawParams = widgetDS.widget_params; if (rawParams) { const widget_params: Record<string, any> = typeof rawParams === 'string' ? JSON5.parse(rawParams) : (rawParams as Record<string, any>); - if (widget_params.encoding !== undefined && !['hex', 'base64', 'ascii'].includes(widget_params.encoding)) { - errors.push(Messages.WIDGET_PARAMETER_UNSUPPORTED('encoding', WidgetTypeEnum.Binary)); + const SUPPORTED_BINARY_ENCODINGS = ['hex', 'base64', 'ascii']; + for (const key of Object.keys(widget_params)) { + if (key !== 'encoding') { + errors.push(Messages.WIDGET_PARAMETER_UNSUPPORTED(key, WidgetTypeEnum.Binary)); + } + } + if (widget_params.encoding !== undefined && !SUPPORTED_BINARY_ENCODINGS.includes(widget_params.encoding)) { + errors.push(Messages.WIDGET_PARAMETER_UNSUPPORTED('encoding', WidgetTypeEnum.Binary)); } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/entities/widget/utils/validate-create-widgets-ds.ts` around lines 112 - 121, The Binary branch currently hardcodes allowed encodings and only checks the "encoding" key so typos slip through; extract the encoding list into a shared backend constant (e.g. add BINARY_ENCODINGS = ['hex','base64','ascii'] in the existing constants file) and use that in the validator where WidgetTypeEnum.Binary is handled (look for widgetDS.widget_params and the current ['hex','base64','ascii'] check), then additionally reject unknown keys by comparing Object.keys(widget_params) against the single allowed set (['encoding']) and push Messages.WIDGET_PARAMETER_UNSUPPORTED for any unexpected param names (mirror the approach used for FOREIGN_KEY_FIELDS validation); keep the frontend in sync by importing the same enum/constant there.frontend/src/app/components/ui-components/record-view-fields/binary/binary.component.ts (1)
19-24: DRY suggestion: extract shared encoding derivation helper.The same
encoding,encodedValue, andcopyTooltipcomputed pattern is duplicated verbatim intable-display-fields/binary/binary.component.ts(lines 19-24) and a near-identical derivation lives in the record-edit and filter binary components. Consider extracting a small helper infrontend/src/app/lib/binary.ts(e.g.resolveEncoding(widgetParams: { encoding?: unknown } | undefined): BinaryEncoding) to avoid four copies drifting apart over time.♻️ Proposed helper
// frontend/src/app/lib/binary.ts +export function resolveEncoding(widgetParams: { encoding?: unknown } | null | undefined): BinaryEncoding { + const raw = widgetParams?.encoding; + return isBinaryEncoding(raw) ? raw : 'hex'; +}-import { bytesToEncoded, isBinaryEncoding, parseBinaryValue } from 'src/app/lib/binary'; +import { bytesToEncoded, parseBinaryValue, resolveEncoding } from 'src/app/lib/binary'; ... - public readonly encoding = computed(() => { - const raw = this.widgetStructure()?.widget_params?.encoding; - return isBinaryEncoding(raw) ? raw : 'hex'; - }); + public readonly encoding = computed(() => resolveEncoding(this.widgetStructure()?.widget_params));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/components/ui-components/record-view-fields/binary/binary.component.ts` around lines 19 - 24, The encoding derivation logic is duplicated across multiple binary components; extract a helper resolveEncoding(widgetParams) in frontend/src/app/lib/binary.ts and use it in this component to compute encoding, encodedValue and copyTooltip. Replace the computed encoding block (which reads widgetStructure()?.widget_params?.encoding and calls isBinaryEncoding) with a call to resolveEncoding(widgetStructure()?.widget_params), keep encodedValue using bytesToEncoded(this.bytes(), this.encoding()) and copyTooltip using `Copy ${this.encoding()}`; ensure the helper returns the default 'hex' and exports a BinaryEncoding type so table-display-fields/binary/binary.component.ts, record-edit and filter binary components can import and use the same resolveEncoding to remove duplication.frontend/src/app/components/ui-components/record-edit-fields/binary/binary.component.ts (1)
31-34: Access modifier for signal doesn't match guideline.
encodingis implicitlypublic. Templates can still readprotectedmembers in Angular, soprotected readonlysatisfies both template binding and the style rule.♻️ Proposed change
- public readonly encoding = computed(() => { + protected readonly encoding = computed(() => { const raw = this.widgetStructure()?.widget_params?.encoding; return isBinaryEncoding(raw) ? raw : 'hex'; });As per coding guidelines: "Signals for component state must be declared as
protectedorprivate readonlyand initialized withsignal()orcomputed()".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/components/ui-components/record-edit-fields/binary/binary.component.ts` around lines 31 - 34, Change the access modifier on the computed signal named encoding to match project guidelines: make the property protected readonly instead of public (i.e., use "protected readonly encoding = computed(...)" in the component where encoding is defined), leaving the implementation using widgetStructure() and isBinaryEncoding unchanged so template bindings still work.frontend/src/app/components/ui-components/filter-fields/binary/binary.component.ts (2)
69-81: PrivatecurrentHex()mutates validity state as a side effect.
currentHex()reads like a pure getter but flipsisInvalidInput. This couples validation to the serialization path and makes the flow harder to reason about (e.g.onFilterModeChangere-validates rawInput every time the comparator changes).Consider splitting parsing and validation, e.g. computing
bytesonce per input change and deriving bothisInvalidInputand the emitted hex from that single result.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/components/ui-components/filter-fields/binary/binary.component.ts` around lines 69 - 81, currentHex() currently mutates isInvalidInput as a side effect; make it a pure getter by removing any writes to isInvalidInput and returning hex only (use encodedToBytes(this.rawInput, this.encoding()) and bytesToHex when valid). Add a single place that parses the input (e.g. a new private parsedBytes property or parseRawInput() called from input/encoding/filter-mode change handlers such as onFilterModeChange) that computes bytes once, sets isInvalidInput based on whether encodedToBytes returned null, and stores the bytes for reuse; then have currentHex() read that stored bytes and return '' if null. Ensure you update callers to call the new parser on input/setting changes rather than relying on currentHex() to perform validation.
34-37: Encoding signal should beprotected readonly.Same guideline issue as in
record-edit-fields/binary/binary.component.ts: state signals must not bepublic. Preferprotected readonlyso the template still binds.♻️ Proposed change
- public readonly encoding = computed(() => { + protected readonly encoding = computed(() => { const raw = this.widgetStructure()?.widget_params?.encoding; return isBinaryEncoding(raw) ? raw : 'hex'; });As per coding guidelines: "Signals for component state must be declared as
protectedorprivate readonlyand initialized withsignal()orcomputed()".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/components/ui-components/filter-fields/binary/binary.component.ts` around lines 34 - 37, The encoding computed signal in BinaryComponent is declared public but must be protected readonly per component state signal guidelines; update the declaration of the encoding computed (the computed(() => { const raw = this.widgetStructure()?.widget_params?.encoding; return isBinaryEncoding(raw) ? raw : 'hex'; })) to use protected readonly so templates still bind but the signal is not public.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@frontend/src/app/components/ui-components/record-edit-fields/binary/binary.component.html`:
- Around line 26-32: The ascii branch currently lacks validation and mat-error
UI, so add an asciiValidator to the ASCII textarea and show a mat-error like the
hex/base64 branches: hook the validator into the template for the textarea bound
to [(ngModel)]="rawInput" and (ngModelChange)="onInputChange()", and add a
corresponding <mat-error> message that displays when asciiValidator fails;
implement asciiValidator (used by the component class handling
onInputChange/stringToBytes) to reject any code point > 0x7F and to detect
surrogate pairs/invalid UTF-16 sequences so users are warned when non-ASCII
characters are entered.
In `@frontend/src/app/lib/binary.ts`:
- Around line 75-89: bytesToAscii currently replaces non-printable bytes with
'.' which causes silent data loss on edits; change bytesToAscii to emit a
reversible representation for non-printable bytes (e.g. '\xNN' hex escapes)
instead of '.' so the original byte values can be round-tripped, and update
encodedToBytes(..., 'ascii') to parse those escapes back into the original
bytes; also simplify isPrintableAscii by removing the redundant "byte !== 0x7f"
check (keep the same printable-range logic). Ensure the unique symbols touched
are bytesToAscii, isPrintableAscii and encodedToBytes so you can locate and
coordinate the encode/decode changes across the binary encoding helpers and the
record-edit flow.
---
Nitpick comments:
In `@backend/src/entities/widget/utils/validate-create-widgets-ds.ts`:
- Around line 112-121: The Binary branch currently hardcodes allowed encodings
and only checks the "encoding" key so typos slip through; extract the encoding
list into a shared backend constant (e.g. add BINARY_ENCODINGS =
['hex','base64','ascii'] in the existing constants file) and use that in the
validator where WidgetTypeEnum.Binary is handled (look for
widgetDS.widget_params and the current ['hex','base64','ascii'] check), then
additionally reject unknown keys by comparing Object.keys(widget_params) against
the single allowed set (['encoding']) and push
Messages.WIDGET_PARAMETER_UNSUPPORTED for any unexpected param names (mirror the
approach used for FOREIGN_KEY_FIELDS validation); keep the frontend in sync by
importing the same enum/constant there.
In `@backend/test/ava-tests/saas-tests/table-widgets-e2e.test.ts`:
- Around line 1705-1810: The tests only assert that WidgetTypeEnum.Binary
accepts 'base64' but the validator allows 'hex', 'base64', and 'ascii'; update
the tests around the Binary widget cases (the test block names using currentTest
and the CreateOrUpdateTableWidgetsDto payloads) to parameterize over the three
encodings and assert each is accepted: iterate encodings =
['hex','base64','ascii'] and for each create the payload with widget_params
JSON.stringify({encoding}) when posting to POST
`/widget/${connectionId}?tableName=${tableNameForWidgets}`, expect 201 and
verify params.encoding via JSON5.parse(ro[0].widget_params) equals the current
encoding; keep the existing invalid-encoding and absent-encoding tests as-is.
In
`@frontend/src/app/components/ui-components/filter-fields/binary/binary.component.ts`:
- Around line 69-81: currentHex() currently mutates isInvalidInput as a side
effect; make it a pure getter by removing any writes to isInvalidInput and
returning hex only (use encodedToBytes(this.rawInput, this.encoding()) and
bytesToHex when valid). Add a single place that parses the input (e.g. a new
private parsedBytes property or parseRawInput() called from
input/encoding/filter-mode change handlers such as onFilterModeChange) that
computes bytes once, sets isInvalidInput based on whether encodedToBytes
returned null, and stores the bytes for reuse; then have currentHex() read that
stored bytes and return '' if null. Ensure you update callers to call the new
parser on input/setting changes rather than relying on currentHex() to perform
validation.
- Around line 34-37: The encoding computed signal in BinaryComponent is declared
public but must be protected readonly per component state signal guidelines;
update the declaration of the encoding computed (the computed(() => { const raw
= this.widgetStructure()?.widget_params?.encoding; return isBinaryEncoding(raw)
? raw : 'hex'; })) to use protected readonly so templates still bind but the
signal is not public.
In
`@frontend/src/app/components/ui-components/record-edit-fields/binary/binary.component.ts`:
- Around line 31-34: Change the access modifier on the computed signal named
encoding to match project guidelines: make the property protected readonly
instead of public (i.e., use "protected readonly encoding = computed(...)" in
the component where encoding is defined), leaving the implementation using
widgetStructure() and isBinaryEncoding unchanged so template bindings still
work.
In
`@frontend/src/app/components/ui-components/record-view-fields/binary/binary.component.ts`:
- Around line 19-24: The encoding derivation logic is duplicated across multiple
binary components; extract a helper resolveEncoding(widgetParams) in
frontend/src/app/lib/binary.ts and use it in this component to compute encoding,
encodedValue and copyTooltip. Replace the computed encoding block (which reads
widgetStructure()?.widget_params?.encoding and calls isBinaryEncoding) with a
call to resolveEncoding(widgetStructure()?.widget_params), keep encodedValue
using bytesToEncoded(this.bytes(), this.encoding()) and copyTooltip using `Copy
${this.encoding()}`; ensure the helper returns the default 'hex' and exports a
BinaryEncoding type so table-display-fields/binary/binary.component.ts,
record-edit and filter binary components can import and use the same
resolveEncoding to remove duplication.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b5f30d31-e854-4fe5-999f-f71ac477d0c3
📒 Files selected for processing (19)
backend/src/entities/widget/utils/validate-create-widgets-ds.tsbackend/src/enums/widget-type.enum.tsbackend/test/ava-tests/saas-tests/table-widgets-e2e.test.tsfrontend/src/app/components/dashboard/db-table-view/db-table-widgets/db-table-widgets.component.tsfrontend/src/app/components/ui-components/filter-fields/binary/binary.component.htmlfrontend/src/app/components/ui-components/filter-fields/binary/binary.component.spec.tsfrontend/src/app/components/ui-components/filter-fields/binary/binary.component.tsfrontend/src/app/components/ui-components/record-edit-fields/binary/binary.component.htmlfrontend/src/app/components/ui-components/record-edit-fields/binary/binary.component.spec.tsfrontend/src/app/components/ui-components/record-edit-fields/binary/binary.component.tsfrontend/src/app/components/ui-components/record-view-fields/binary/binary.component.htmlfrontend/src/app/components/ui-components/record-view-fields/binary/binary.component.spec.tsfrontend/src/app/components/ui-components/record-view-fields/binary/binary.component.tsfrontend/src/app/components/ui-components/table-display-fields/binary/binary.component.htmlfrontend/src/app/components/ui-components/table-display-fields/binary/binary.component.spec.tsfrontend/src/app/components/ui-components/table-display-fields/binary/binary.component.tsfrontend/src/app/lib/binary.spec.tsfrontend/src/app/lib/binary.tsfrontend/src/app/validators/base64.validator.ts
Comment on lines
+26
to
+32
| @case ('ascii') { | ||
| <textarea matInput resizeToFitContent rows="8" | ||
| name="{{label()}}-{{key()}}-ascii-content" | ||
| [required]="required()" [disabled]="disabled()" [readonly]="readonly()" | ||
| [(ngModel)]="rawInput" (ngModelChange)="onInputChange()" | ||
| ></textarea> | ||
| } |
There was a problem hiding this comment.
ASCII case silently accepts non-ASCII characters.
Unlike the hex and base64 branches, the ascii textarea has no validator and no mat-error. If the user types or pastes a character outside 0x00–0x7F (accented letters, emoji, CJK…), stringToBytes will still produce a byte per charCodeAt(i) — with bits silently truncated for surrogate pairs and no warning for code points > 127. Consider attaching an asciiValidator and surfacing a message symmetric to the other two cases.
🛡️ Sketch
`@case` ('ascii') {
- <textarea matInput resizeToFitContent rows="8"
+ <textarea matInput resizeToFitContent rows="8" asciiValidator
name="{{label()}}-{{key()}}-ascii-content"
+ `#binaryContent`="ngModel"
[required]="required()" [disabled]="disabled()" [readonly]="readonly()"
[(ngModel)]="rawInput" (ngModelChange)="onInputChange()"
></textarea>
+ `@if` (binaryContent.errors?.isInvalidAscii) {
+ <mat-error>Only 7-bit ASCII characters are allowed.</mat-error>
+ }
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@frontend/src/app/components/ui-components/record-edit-fields/binary/binary.component.html`
around lines 26 - 32, The ascii branch currently lacks validation and mat-error
UI, so add an asciiValidator to the ASCII textarea and show a mat-error like the
hex/base64 branches: hook the validator into the template for the textarea bound
to [(ngModel)]="rawInput" and (ngModelChange)="onInputChange()", and add a
corresponding <mat-error> message that displays when asciiValidator fails;
implement asciiValidator (used by the component class handling
onInputChange/stringToBytes) to reject any code point > 0x7F and to detect
surrogate pairs/invalid UTF-16 sequences so users are warned when non-ASCII
characters are entered.
Comment on lines
+75
to
+89
| // Replacement character for non-printable ASCII bytes in display/encode. | ||
| const ASCII_REPLACEMENT = '.'; | ||
|
|
||
| function isPrintableAscii(byte: number): boolean { | ||
| if (byte === 0x09 || byte === 0x0a || byte === 0x0d) return true; | ||
| return byte >= 0x20 && byte !== 0x7f && byte <= 0x7e; | ||
| } | ||
|
|
||
| export function bytesToAscii(bytes: number[]): string { | ||
| let out = ''; | ||
| for (const b of bytes) { | ||
| out += isPrintableAscii(b) ? String.fromCharCode(b) : ASCII_REPLACEMENT; | ||
| } | ||
| return out; | ||
| } |
There was a problem hiding this comment.
ASCII encoding is lossy and round-trips as data corruption in the record-edit path.
bytesToAscii replaces any non-printable byte with . (0x2e). In record-edit-fields/binary/binary.component.ts, ngOnInit seeds rawInput via bytesToEncoded(parseBinaryValue(this.value()), 'ascii'), and onInputChange round-trips the current rawInput through encodedToBytes(..., 'ascii') → stringToBytes. So any pre-existing non-printable byte is shown as . and, on the next edit + save, is written back as a literal 0x2e. The original bytes are silently lost.
Concretely, bytes like 0x00, 0x7f, 0xff that existed in the DB become 0x2e as soon as the user modifies any part of the field.
Options:
- Disable editing when
encoding === 'ascii'and the parsed bytes contain any non-printable byte, or render ascii as read-only for the record-edit component. - Make
encodedToBytes(str, 'ascii')reject/round-trip-check so a rawInput containing placeholder.s that don't match the originally-decoded bytes isn't silently accepted. - Document ASCII as display-only (and remove it from
encodedToBytes/record-edit use).
Also minor: in isPrintableAscii, byte !== 0x7f is redundant because byte <= 0x7e already excludes 0x7f.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/app/lib/binary.ts` around lines 75 - 89, bytesToAscii currently
replaces non-printable bytes with '.' which causes silent data loss on edits;
change bytesToAscii to emit a reversible representation for non-printable bytes
(e.g. '\xNN' hex escapes) instead of '.' so the original byte values can be
round-tripped, and update encodedToBytes(..., 'ascii') to parse those escapes
back into the original bytes; also simplify isPrintableAscii by removing the
redundant "byte !== 0x7f" check (keep the same printable-range logic). Ensure
the unique symbols touched are bytesToAscii, isPrintableAscii and encodedToBytes
so you can locate and coordinate the encode/decode changes across the binary
encoding helpers and the record-edit flow.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Support hex (default), base64, and ascii encodings for binary widget display, record view, edit input, and filter. Adds backend validation for the encoding param on create/update and covers each encoding with frontend and AVA tests. Filter continues to emit hex on the wire to preserve the backend comparator contract.
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes