From 6eccb86db249e01092be1833390880478780439d Mon Sep 17 00:00:00 2001 From: demiliani Date: Tue, 13 Oct 2026 15:20:44 +0200 Subject: [PATCH 1/2] knowledge: isolated events, FilterGroup scope, BLOB CalcFields and stream text encoding Add six Microsoft-layer articles with good/bad samples for concerns the corpus did not cover: - events: isolated events are isolated only outside a write transaction; an isolated event's rollback does not undo var parameters (IsHandled). - security: page scope filters must not be set in filter group 0; restore the filter group and avoid platform groups 1/3/4/6/7. - data-modeling: calculate a BLOB field before reading its stream. - web-services: set an explicit TextEncoding on BLOB streams whose text leaves or enters Business Central (default is MSDos). Wire each article into its leaf review skill (tokens and targeted cue) and register the paired articles in the review-fixtures overrides. Co-Authored-By: Claude Opus 5.5 --- evaluation/review-fixtures.json | 12 +++-- ...elds-blob-before-reading-its-stream.bad.al | 17 +++++++ ...lds-blob-before-reading-its-stream.good.al | 17 +++++++ ...lcfields-blob-before-reading-its-stream.md | 34 ++++++++++++++ ...llback-does-not-undo-var-parameters.bad.al | 26 +++++++++++ ...lback-does-not-undo-var-parameters.good.al | 35 ++++++++++++++ ...t-rollback-does-not-undo-var-parameters.md | 36 +++++++++++++++ ...ed-only-outside-a-write-transaction.bad.al | 20 ++++++++ ...d-only-outside-a-write-transaction.good.al | 28 +++++++++++ ...olated-only-outside-a-write-transaction.md | 38 +++++++++++++++ ...ers-must-not-be-in-filtergroup-zero.bad.al | 30 ++++++++++++ ...rs-must-not-be-in-filtergroup-zero.good.al | 32 +++++++++++++ ...filters-must-not-be-in-filtergroup-zero.md | 36 +++++++++++++++ ...rgroup-after-setting-hidden-filters.bad.al | 42 +++++++++++++++++ ...group-after-setting-hidden-filters.good.al | 46 +++++++++++++++++++ ...iltergroup-after-setting-hidden-filters.md | 34 ++++++++++++++ ...ing-on-streams-exchanged-externally.bad.al | 26 +++++++++++ ...ng-on-streams-exchanged-externally.good.al | 24 ++++++++++ ...ncoding-on-streams-exchanged-externally.md | 32 +++++++++++++ .../skills/review/al-data-modeling-review.md | 7 +-- microsoft/skills/review/al-events-review.md | 4 +- microsoft/skills/review/al-security-review.md | 4 +- .../skills/review/al-web-services-review.md | 5 +- 23 files changed, 575 insertions(+), 10 deletions(-) create mode 100644 microsoft/knowledge/data-modeling/calcfields-blob-before-reading-its-stream.bad.al create mode 100644 microsoft/knowledge/data-modeling/calcfields-blob-before-reading-its-stream.good.al create mode 100644 microsoft/knowledge/data-modeling/calcfields-blob-before-reading-its-stream.md create mode 100644 microsoft/knowledge/events/isolated-event-rollback-does-not-undo-var-parameters.bad.al create mode 100644 microsoft/knowledge/events/isolated-event-rollback-does-not-undo-var-parameters.good.al create mode 100644 microsoft/knowledge/events/isolated-event-rollback-does-not-undo-var-parameters.md create mode 100644 microsoft/knowledge/events/isolated-events-are-isolated-only-outside-a-write-transaction.bad.al create mode 100644 microsoft/knowledge/events/isolated-events-are-isolated-only-outside-a-write-transaction.good.al create mode 100644 microsoft/knowledge/events/isolated-events-are-isolated-only-outside-a-write-transaction.md create mode 100644 microsoft/knowledge/security/page-scope-filters-must-not-be-in-filtergroup-zero.bad.al create mode 100644 microsoft/knowledge/security/page-scope-filters-must-not-be-in-filtergroup-zero.good.al create mode 100644 microsoft/knowledge/security/page-scope-filters-must-not-be-in-filtergroup-zero.md create mode 100644 microsoft/knowledge/security/restore-the-filtergroup-after-setting-hidden-filters.bad.al create mode 100644 microsoft/knowledge/security/restore-the-filtergroup-after-setting-hidden-filters.good.al create mode 100644 microsoft/knowledge/security/restore-the-filtergroup-after-setting-hidden-filters.md create mode 100644 microsoft/knowledge/web-services/set-text-encoding-on-streams-exchanged-externally.bad.al create mode 100644 microsoft/knowledge/web-services/set-text-encoding-on-streams-exchanged-externally.good.al create mode 100644 microsoft/knowledge/web-services/set-text-encoding-on-streams-exchanged-externally.md diff --git a/evaluation/review-fixtures.json b/evaluation/review-fixtures.json index 68904529..f8f8ed9b 100644 --- a/evaluation/review-fixtures.json +++ b/evaluation/review-fixtures.json @@ -14,6 +14,7 @@ "data-modeling": { "articles": [ "activate-new-price-calculation-handler-via-onfindsupportedsetup", + "calcfields-blob-before-reading-its-stream", "check-blocked-in-referencing-code-not-in-master", "code-must-not-change-workdate", "custom-document-dispatch-must-not-bypass-report-selections", @@ -44,7 +45,9 @@ "articles": [ "reset-ishandled-only-when-the-value-can-carry-over", "changecompany-runs-triggers-in-the-calling-company", - "database-trigger-setup-flags-may-only-be-set-to-true" + "database-trigger-setup-flags-may-only-be-set-to-true", + "isolated-events-are-isolated-only-outside-a-write-transaction", + "isolated-event-rollback-does-not-undo-var-parameters" ] }, "finance": { @@ -140,7 +143,9 @@ "articles": [ "al-has-no-built-in-htmlencode", "do-not-concatenate-external-text-into-setfilter", - "exposed-objects-must-be-in-a-permission-set" + "exposed-objects-must-be-in-a-permission-set", + "page-scope-filters-must-not-be-in-filtergroup-zero", + "restore-the-filtergroup-after-setting-hidden-filters" ] }, "style": { @@ -197,7 +202,8 @@ "check-http-status-before-consuming-response-body", "check-json-null-before-converting-values", "format-exchanged-values-with-standard-format-9", - "api-page-least-privilege-write-access" + "api-page-least-privilege-write-access", + "set-text-encoding-on-streams-exchanged-externally" ] } } diff --git a/microsoft/knowledge/data-modeling/calcfields-blob-before-reading-its-stream.bad.al b/microsoft/knowledge/data-modeling/calcfields-blob-before-reading-its-stream.bad.al new file mode 100644 index 00000000..a02f85f1 --- /dev/null +++ b/microsoft/knowledge/data-modeling/calcfields-blob-before-reading-its-stream.bad.al @@ -0,0 +1,17 @@ +// Demonstration only. Shows the wrong pattern: a BLOB field is read without being calculated. + +codeunit 50188 "Payload Reader Bad" +{ + procedure GetPayload(EntryNo: Integer): Text + var + IntegrationLog: Record "Integration Log Demo"; + PayloadInStream: InStream; + PayloadText: Text; + begin + IntegrationLog.Get(EntryNo); + // BAD: Get does not load BLOB content. The stream is empty and PayloadText stays ''. + IntegrationLog.Payload.CreateInStream(PayloadInStream, TextEncoding::UTF8); + PayloadInStream.ReadText(PayloadText); + exit(PayloadText); + end; +} diff --git a/microsoft/knowledge/data-modeling/calcfields-blob-before-reading-its-stream.good.al b/microsoft/knowledge/data-modeling/calcfields-blob-before-reading-its-stream.good.al new file mode 100644 index 00000000..eed88a22 --- /dev/null +++ b/microsoft/knowledge/data-modeling/calcfields-blob-before-reading-its-stream.good.al @@ -0,0 +1,17 @@ +// Demonstration only. Shows the recommended pattern: the BLOB field is calculated before its stream is read. + +codeunit 50189 "Payload Reader Good" +{ + procedure GetPayload(EntryNo: Integer): Text + var + IntegrationLog: Record "Integration Log Demo"; + PayloadInStream: InStream; + PayloadText: Text; + begin + IntegrationLog.Get(EntryNo); + IntegrationLog.CalcFields(Payload); + IntegrationLog.Payload.CreateInStream(PayloadInStream, TextEncoding::UTF8); + PayloadInStream.ReadText(PayloadText); + exit(PayloadText); + end; +} diff --git a/microsoft/knowledge/data-modeling/calcfields-blob-before-reading-its-stream.md b/microsoft/knowledge/data-modeling/calcfields-blob-before-reading-its-stream.md new file mode 100644 index 00000000..6f832c3e --- /dev/null +++ b/microsoft/knowledge/data-modeling/calcfields-blob-before-reading-its-stream.md @@ -0,0 +1,34 @@ +--- +bc-version: [all] +domain: data-modeling +keywords: [blob, calcfields, setautocalcfields, createinstream, instream, empty-stream, blob-field] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Calculate a BLOB field before reading its stream + +## Description + +`Get`, `Find`, `FindSet`, and `Next` do not load the content of `Blob` fields. Until the field is calculated with `CalcFields` (or loaded through `SetAutoCalcFields` before the read), the in-memory BLOB is empty. `CreateInStream` on that field succeeds and returns an empty stream, so code that reads text, JSON, or an image from it gets an empty result and no error. The defect does not surface in tests that fill and read the BLOB in the same record variable. + +`Media` and `MediaSet` fields are different types and are not loaded with `CalcFields`; this article covers `Blob` only. + +## Best Practice + +Call `CalcFields` on the BLOB field after retrieving the record and before `CreateInStream`, or call `SetAutoCalcFields` on the field before a `FindSet` loop that reads the BLOB for every row. + +Do not flag code that reads a BLOB it has just written through `CreateOutStream` on the same record variable, a BLOB on a temporary record filled in the same session, or a read preceded by `SetAutoCalcFields` on that field. Writing a BLOB does not require a prior `CalcFields`. + +See sample: [`calcfields-blob-before-reading-its-stream.good.al`](calcfields-blob-before-reading-its-stream.good.al). + +## Anti Pattern + +A record retrieved from the database followed by `..CreateInStream(...)` with no `CalcFields` or `SetAutoCalcFields` on that field in between. The reader sees an empty stream and passes empty content on to the parser, export, or HTTP request that consumes it. + +See sample: [`calcfields-blob-before-reading-its-stream.bad.al`](calcfields-blob-before-reading-its-stream.bad.al). + +## References + +[Record.CalcFields method](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/methods-auto/record/record-calcfields-method) (CalcFields also retrieves BLOBs) and [BigText.Read method](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/methods-auto/bigtext/bigtext-read-method) (a BLOB field must be calculated before it is used in AL). diff --git a/microsoft/knowledge/events/isolated-event-rollback-does-not-undo-var-parameters.bad.al b/microsoft/knowledge/events/isolated-event-rollback-does-not-undo-var-parameters.bad.al new file mode 100644 index 00000000..5d8fde51 --- /dev/null +++ b/microsoft/knowledge/events/isolated-event-rollback-does-not-undo-var-parameters.bad.al @@ -0,0 +1,26 @@ +// Demonstration only. Shows the wrong pattern: an isolated event used as an IsHandled override point. + +codeunit 50182 "Invoice Export Bad" +{ + procedure ExportInvoice(InvoiceNo: Code[20]) + var + IsHandled: Boolean; + begin + // BAD: a subscriber can set IsHandled := true and then fail. Its table writes are rolled + // back, but IsHandled keeps the value true, so the default export is silently skipped. + OnBeforeExportInvoice(InvoiceNo, IsHandled); + if IsHandled then + exit; + + ExportToDefaultChannel(InvoiceNo); + end; + + local procedure ExportToDefaultChannel(InvoiceNo: Code[20]) + begin + end; + + [IntegrationEvent(false, false, true)] + local procedure OnBeforeExportInvoice(InvoiceNo: Code[20]; var IsHandled: Boolean) + begin + end; +} diff --git a/microsoft/knowledge/events/isolated-event-rollback-does-not-undo-var-parameters.good.al b/microsoft/knowledge/events/isolated-event-rollback-does-not-undo-var-parameters.good.al new file mode 100644 index 00000000..3001ce6a --- /dev/null +++ b/microsoft/knowledge/events/isolated-event-rollback-does-not-undo-var-parameters.good.al @@ -0,0 +1,35 @@ +// Demonstration only. Shows the recommended pattern: the override point is a normal event; the isolated event only notifies. + +codeunit 50183 "Invoice Export Good" +{ + procedure ExportInvoice(InvoiceNo: Code[20]) + var + IsHandled: Boolean; + begin + // Normal event: a subscriber error propagates, so IsHandled is never consumed half-applied. + OnBeforeExportInvoice(InvoiceNo, IsHandled); + if not IsHandled then + ExportToDefaultChannel(InvoiceNo); + end; + + procedure NotifyInvoiceExported(InvoiceNo: Code[20]) + begin + // Called after the export has been committed. By-value parameters only: a failing + // subscriber cannot change anything the publisher reads afterwards. + OnAfterInvoiceExported(InvoiceNo); + end; + + local procedure ExportToDefaultChannel(InvoiceNo: Code[20]) + begin + end; + + [IntegrationEvent(false, false)] + local procedure OnBeforeExportInvoice(InvoiceNo: Code[20]; var IsHandled: Boolean) + begin + end; + + [IntegrationEvent(false, false, true)] + local procedure OnAfterInvoiceExported(InvoiceNo: Code[20]) + begin + end; +} diff --git a/microsoft/knowledge/events/isolated-event-rollback-does-not-undo-var-parameters.md b/microsoft/knowledge/events/isolated-event-rollback-does-not-undo-var-parameters.md new file mode 100644 index 00000000..ed36f094 --- /dev/null +++ b/microsoft/knowledge/events/isolated-event-rollback-does-not-undo-var-parameters.md @@ -0,0 +1,36 @@ +--- +bc-version: [20..] +domain: events +keywords: [isolated-event, isolated, var-parameter, ishandled, onbefore, integration-event, rollback, subscriber-error, side-effects] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# An isolated event's rollback does not undo var parameters or other in-memory state + +## Description + +When a subscriber to an isolated event fails, the platform rolls back only the `Insert`, `Modify`, and `Delete` calls it made on normal tables. Everything else the subscriber changed before the error survives: values assigned to `var` parameters (including `IsHandled`), fields of a `var` record that were never written, globals of single-instance codeunits, temporary-table contents, and external effects such as HTTP calls. Execution then continues with the next subscriber and returns to the publisher as if nothing went wrong. + +Reviewers and authoring agents tend to read "isolated" as "a failed subscriber leaves no trace", so they design isolated events with `var` result parameters or an `IsHandled` switch. A subscriber that sets `IsHandled := true` and then errors makes the publisher skip its default behaviour, even though the subscriber's own work was rolled back. + +## Best Practice + +Use isolated events for notifications: pass parameters by value and let subscribers act on their own data. When the publisher needs a decision or a result from extensions, publish a normal (non-isolated) event so subscriber errors propagate and the publisher never consumes a half-applied result. + +If an isolated event must carry `var` data for compatibility, the publisher must validate the returned values before using them and must not let a returned flag bypass required logic. + +See sample: [`isolated-event-rollback-does-not-undo-var-parameters.good.al`](isolated-event-rollback-does-not-undo-var-parameters.good.al). + +## Anti Pattern + +An event declared with `Isolated = true` whose signature includes `var IsHandled`, a `var` result value, or a `var` record the publisher reads afterwards, where the publisher changes its behaviour based on those values. A failing subscriber can leave them modified while its database writes are rolled back, so the publisher acts on a decision nobody completed. + +Changing the `Isolated` flag of an event that has already shipped is a separate contract problem covered by [do-not-change-shipped-event-attribute-flags](do-not-change-shipped-event-attribute-flags.md); this article is about the design of new isolated events. + +See sample: [`isolated-event-rollback-does-not-undo-var-parameters.bad.al`](isolated-event-rollback-does-not-undo-var-parameters.bad.al). + +## References + +[Isolated events in AL, Rollback](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/devenv-events-isolated#rollback): only Modify/Delete/Insert on normal tables are rolled back; HTTP calls, variable changes, and single-instance codeunit members are not, and a `var` integer modified by a failing subscriber keeps its change. diff --git a/microsoft/knowledge/events/isolated-events-are-isolated-only-outside-a-write-transaction.bad.al b/microsoft/knowledge/events/isolated-events-are-isolated-only-outside-a-write-transaction.bad.al new file mode 100644 index 00000000..d946ac53 --- /dev/null +++ b/microsoft/knowledge/events/isolated-events-are-isolated-only-outside-a-write-transaction.bad.al @@ -0,0 +1,20 @@ +// Demonstration only. Shows the wrong pattern: an isolated event raised inside an open write transaction. + +codeunit 50180 "Shipment Confirmation Bad" +{ + procedure ConfirmShipment(var ShipmentHeader: Record "Shipment Header Demo") + begin + ShipmentHeader.TestField("Carrier Code"); + ShipmentHeader.Status := ShipmentHeader.Status::Confirmed; + ShipmentHeader.Modify(true); + + // BAD: the Modify above is not committed, so the platform runs this "isolated" event + // as a normal event. A failing subscriber rolls back the confirmation as well. + OnAfterConfirmShipment(ShipmentHeader); + end; + + [IntegrationEvent(false, false, true)] + local procedure OnAfterConfirmShipment(ShipmentHeader: Record "Shipment Header Demo") + begin + end; +} diff --git a/microsoft/knowledge/events/isolated-events-are-isolated-only-outside-a-write-transaction.good.al b/microsoft/knowledge/events/isolated-events-are-isolated-only-outside-a-write-transaction.good.al new file mode 100644 index 00000000..531002eb --- /dev/null +++ b/microsoft/knowledge/events/isolated-events-are-isolated-only-outside-a-write-transaction.good.al @@ -0,0 +1,28 @@ +// Demonstration only. Shows the recommended pattern: the isolated event is raised from a context that has written nothing. + +codeunit 50181 "Shipment Confirmation Good" +{ + procedure ConfirmShipment(var ShipmentHeader: Record "Shipment Header Demo") + begin + ShipmentHeader.TestField("Carrier Code"); + ShipmentHeader.Status := ShipmentHeader.Status::Confirmed; + ShipmentHeader.Modify(true); + end; + + // Called by the page action or job queue entry after ConfirmShipment has completed and its + // transaction has ended. This procedure only reads, so each subscriber runs in its own + // transaction and a failing subscriber cannot undo or block anything. + procedure NotifyShipmentConfirmed(ShipmentNo: Code[20]) + var + ShipmentHeader: Record "Shipment Header Demo"; + begin + ShipmentHeader.Get(ShipmentNo); + ShipmentHeader.TestField(Status, ShipmentHeader.Status::Confirmed); + OnAfterShipmentConfirmed(ShipmentHeader); + end; + + [IntegrationEvent(false, false, true)] + local procedure OnAfterShipmentConfirmed(ShipmentHeader: Record "Shipment Header Demo") + begin + end; +} diff --git a/microsoft/knowledge/events/isolated-events-are-isolated-only-outside-a-write-transaction.md b/microsoft/knowledge/events/isolated-events-are-isolated-only-outside-a-write-transaction.md new file mode 100644 index 00000000..0975e573 --- /dev/null +++ b/microsoft/knowledge/events/isolated-events-are-isolated-only-outside-a-write-transaction.md @@ -0,0 +1,38 @@ +--- +bc-version: [20..] +domain: events +keywords: [isolated-event, isolated, write-transaction, commit, modify, uncommitted-write, subscriber-error, rollback, integration-event] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Isolated events are isolated only when no write transaction is open + +## Description + +Setting `Isolated = true` on a `BusinessEvent`, `IntegrationEvent`, or `InternalEvent` (runtime 9.0 / BC20 and later) runs each subscriber in its own transaction: a failing subscriber is rolled back and the next subscriber, then the caller, continue. The platform only does this when the publisher is raised from a read-only transaction. When the caller has already written to the database and not committed, the isolated event is invoked like a normal event, so a subscriber error fails the whole operation. The same fallback applies during extension install, uninstall, and upgrade. + +An agent that sees `Isolated = true` tends to assume subscriber errors can never reach the caller, and either skips error handling around the raise or adds a `Commit()` right before it to "make isolation work". The first leaves a hidden failure path; the second splits a business transaction that was meant to be atomic. + +## Best Practice + +Raise an isolated event at a point where the transaction is naturally read-only or already committed, for example after the posting routine has finished and committed, or from a job queue entry, page action, or notification handler that has written nothing yet. Treat isolation as a way to keep optional, non-critical subscribers (telemetry, notifications, external sync) from breaking the publisher, not as a guarantee. + +If the event must be raised in the middle of a write transaction, design it as a normal event and document that subscriber errors propagate. When error isolation is genuinely needed there, move the optional work after the commit point or into a separate `Codeunit.Run` / background task, which is easier to test than an isolated event. + +See sample: [`isolated-events-are-isolated-only-outside-a-write-transaction.good.al`](isolated-events-are-isolated-only-outside-a-write-transaction.good.al). + +## Anti Pattern + +An isolated event raised after uncommitted `Insert`/`Modify`/`Delete` calls in the same transaction, with code or comments relying on subscriber errors being contained. The subscriber runs non-isolated and its error rolls back the caller's work. + +Equally wrong is inserting `Commit()` immediately before the raise inside a posting, release, or other all-or-nothing routine just to obtain isolation: a later failure now leaves the first half of the operation committed. + +Do not flag an isolated event raised from a read-only context or after the routine's own completed commit, and do not flag a normal (non-isolated) event for lacking isolation. + +See sample: [`isolated-events-are-isolated-only-outside-a-write-transaction.bad.al`](isolated-events-are-isolated-only-outside-a-write-transaction.bad.al). + +## References + +[Isolated events in AL](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/devenv-events-isolated) (write transactions must be committed before the event; install and upgrade run isolated events normally) and [Types of events for extensibility](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/types-of-events-for-extensibility#isolated-events) (isolated events need a commit before invoking and a detailed review; `Codeunit.Run` as the alternative). diff --git a/microsoft/knowledge/security/page-scope-filters-must-not-be-in-filtergroup-zero.bad.al b/microsoft/knowledge/security/page-scope-filters-must-not-be-in-filtergroup-zero.bad.al new file mode 100644 index 00000000..7fc5b384 --- /dev/null +++ b/microsoft/knowledge/security/page-scope-filters-must-not-be-in-filtergroup-zero.bad.al @@ -0,0 +1,30 @@ +// Demonstration only. Shows the wrong pattern: a page restricts its records with a filter the user can remove. + +page 50184 "My Expense Claims Bad" +{ + PageType = List; + SourceTable = "Expense Claim Demo"; + ApplicationArea = All; + UsageCategory = Lists; + Editable = false; + + layout + { + area(Content) + { + repeater(Claims) + { + field("No."; Rec."No.") { } + field("Employee User ID"; Rec."Employee User ID") { } + field(Amount; Rec.Amount) { } + } + } + } + + trigger OnOpenPage() + begin + // BAD: no FilterGroup call, so this filter is in group 0. The user can clear it in the + // filter pane and see every employee's claims. + Rec.SetRange("Employee User ID", UserId()); + end; +} diff --git a/microsoft/knowledge/security/page-scope-filters-must-not-be-in-filtergroup-zero.good.al b/microsoft/knowledge/security/page-scope-filters-must-not-be-in-filtergroup-zero.good.al new file mode 100644 index 00000000..56dcb724 --- /dev/null +++ b/microsoft/knowledge/security/page-scope-filters-must-not-be-in-filtergroup-zero.good.al @@ -0,0 +1,32 @@ +// Demonstration only. Shows the recommended pattern: the restricting filter is placed in a hidden filter group. + +page 50185 "My Expense Claims Good" +{ + PageType = List; + SourceTable = "Expense Claim Demo"; + ApplicationArea = All; + UsageCategory = Lists; + Editable = false; + + layout + { + area(Content) + { + repeater(Claims) + { + field("No."; Rec."No.") { } + field("Employee User ID"; Rec."Employee User ID") { } + field(Amount; Rec.Amount) { } + } + } + } + + trigger OnOpenPage() + begin + // Group 2 is not editable from the filter pane; the user can only narrow within it. + // Confidentiality of other employees' claims is enforced separately by security filters. + Rec.FilterGroup(2); + Rec.SetRange("Employee User ID", UserId()); + Rec.FilterGroup(0); + end; +} diff --git a/microsoft/knowledge/security/page-scope-filters-must-not-be-in-filtergroup-zero.md b/microsoft/knowledge/security/page-scope-filters-must-not-be-in-filtergroup-zero.md new file mode 100644 index 00000000..6a8fc0c5 --- /dev/null +++ b/microsoft/knowledge/security/page-scope-filters-must-not-be-in-filtergroup-zero.md @@ -0,0 +1,36 @@ +--- +bc-version: [all] +domain: security +keywords: [filtergroup, filter-group, setrange, setfilter, onopenpage, user-filter, record-scope, security-filter] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Filters that restrict what a page shows must not be set in filter group 0 + +## Description + +Filter group 0 is the standard group: it holds the filters the user sees and edits in the filter pane. A `SetRange` or `SetFilter` issued in page code without a prior `FilterGroup` call lands in group 0, so the user can clear it with one click and see every record in the table. Filters in any other group are invisible to the user and can only be narrowed, never widened. Code that restricts a page to "my" records (the current user's entries, one salesperson's customers, one company's documents) therefore silently stops restricting anything if it is set in group 0. + +A non-zero filter group is a UI scope, not a security boundary. A user with read permission on the table can still reach the records through another page, an API, a report, or Excel export. Record-level confidentiality requires security filters on permission sets, or not granting the table at all. + +## Best Practice + +In `OnOpenPage` (or wherever a page narrows its source records) switch to a non-zero group before setting the restricting filters, then switch back to group 0 so the user's own filters land where the client expects them. Group 2 is the conventional choice in page code: the Base Application uses `FilterGroup(2)` for the same purpose, and it is the group that already holds `SourceTableView` and `SetTableView` filters. Apply the [restore rule](restore-the-filtergroup-after-setting-hidden-filters.md) when doing so. + +When the restriction is a confidentiality requirement, enforce it with permission sets and security filters as well, and treat the page filter as a convenience. + +See sample: [`page-scope-filters-must-not-be-in-filtergroup-zero.good.al`](page-scope-filters-must-not-be-in-filtergroup-zero.good.al). + +## Anti Pattern + +A page trigger that limits `Rec` to a user-, role-, or context-specific subset with `SetRange`/`SetFilter` while the current filter group is 0. The user can remove the filter from the filter pane. + +The opposite review error is reporting a non-zero filter group as adequate data protection, or flagging group 0 filters that are meant to be a removable default (for example, a default date range the user is supposed to change). + +See sample: [`page-scope-filters-must-not-be-in-filtergroup-zero.bad.al`](page-scope-filters-must-not-be-in-filtergroup-zero.bad.al). + +## References + +[Record.FilterGroup method](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/methods-auto/record/record-filtergroup-method#remarks): group 0 holds filters set from the filter dialog; a filter in another group cannot be widened by the user. [Using security filters](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/security/security-filters) for record-level security. diff --git a/microsoft/knowledge/security/restore-the-filtergroup-after-setting-hidden-filters.bad.al b/microsoft/knowledge/security/restore-the-filtergroup-after-setting-hidden-filters.bad.al new file mode 100644 index 00000000..d1686b93 --- /dev/null +++ b/microsoft/knowledge/security/restore-the-filtergroup-after-setting-hidden-filters.bad.al @@ -0,0 +1,42 @@ +// Demonstration only. Shows the wrong pattern: a hidden filter group is left active and later overwritten. + +page 50186 "Region Customers Bad" +{ + PageType = List; + SourceTable = Customer; + ApplicationArea = All; + UsageCategory = Lists; + Editable = false; + + layout + { + area(Content) + { + repeater(Customers) + { + field("No."; Rec."No.") { } + field(Name; Rec.Name) { } + field("Territory Code"; Rec."Territory Code") { } + } + } + } + + trigger OnOpenPage() + begin + Rec.FilterGroup(2); + Rec.SetRange("Territory Code", GetUserTerritory()); + // BAD: no FilterGroup(0) here. The record stays in group 2. + ApplyDefaultView(Rec); + end; + + local procedure ApplyDefaultView(var Customer: Record Customer) + begin + // This runs in group 2 and replaces the territory restriction instead of adding to it. + Customer.SetFilter("Territory Code", '<>%1', ''); + end; + + local procedure GetUserTerritory(): Code[10] + begin + exit('NORTH'); + end; +} diff --git a/microsoft/knowledge/security/restore-the-filtergroup-after-setting-hidden-filters.good.al b/microsoft/knowledge/security/restore-the-filtergroup-after-setting-hidden-filters.good.al new file mode 100644 index 00000000..33d39cdb --- /dev/null +++ b/microsoft/knowledge/security/restore-the-filtergroup-after-setting-hidden-filters.good.al @@ -0,0 +1,46 @@ +// Demonstration only. Shows the recommended pattern: the previous filter group is saved and restored. + +page 50187 "Region Customers Good" +{ + PageType = List; + SourceTable = Customer; + ApplicationArea = All; + UsageCategory = Lists; + Editable = false; + + layout + { + area(Content) + { + repeater(Customers) + { + field("No."; Rec."No.") { } + field(Name; Rec.Name) { } + field("Territory Code"; Rec."Territory Code") { } + } + } + } + + trigger OnOpenPage() + var + PreviousFilterGroup: Integer; + begin + PreviousFilterGroup := Rec.FilterGroup(); + Rec.FilterGroup(2); + Rec.SetRange("Territory Code", GetUserTerritory()); + Rec.FilterGroup(PreviousFilterGroup); + + // Runs in the original group, so it narrows the view without touching the hidden filter. + ApplyDefaultView(Rec); + end; + + local procedure ApplyDefaultView(var Customer: Record Customer) + begin + Customer.SetFilter("Territory Code", '<>%1', ''); + end; + + local procedure GetUserTerritory(): Code[10] + begin + exit('NORTH'); + end; +} diff --git a/microsoft/knowledge/security/restore-the-filtergroup-after-setting-hidden-filters.md b/microsoft/knowledge/security/restore-the-filtergroup-after-setting-hidden-filters.md new file mode 100644 index 00000000..106206b8 --- /dev/null +++ b/microsoft/knowledge/security/restore-the-filtergroup-after-setting-hidden-filters.md @@ -0,0 +1,34 @@ +--- +bc-version: [all] +domain: security +keywords: [filtergroup, filter-group, setrange, setfilter, subpagelink, filter-overwrite, record-scope, page-filters] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Restore the filter group after setting hidden filters, and do not reuse platform groups + +## Description + +`FilterGroup(N)` changes the current group for every later `SetRange`/`SetFilter` on that record variable until it is changed again. Each field holds one filter per group, so a later filter on the same field in the same group replaces the earlier one instead of combining with it. Code that switches to a hidden group and does not switch back leaves the record in that group: subsequent code, or a procedure that receives the record, then overwrites the hidden restriction instead of adding to it, and the page's scope widens without any visible change. + +Several groups are used by the platform: 1 global, 2 `SourceTableView`/`SetTableView`/`DataItemTableView`, 3 `RunPageView`/`SubPageView`, 4 `SubPageLink`/`DataItemLink`, 6 security filters, and 7 FactBox state. Writing a custom filter into 4 replaces the link between a subpage and its header; using 7 can break FactBoxes. Microsoft documents that these groups should not be used. Group 2 is the established exception for page code, where it extends the page's own table view. + +## Best Practice + +Save the current group, switch, set the hidden filters, and restore the saved group in the same procedure. Use group 2 for page-level scope restrictions (matching Base Application practice) or a group number above 7 for module-private hidden filters on a record variable you own. Do not set filters in groups 1, 3, 4, 6, or 7. + +See sample: [`restore-the-filtergroup-after-setting-hidden-filters.good.al`](restore-the-filtergroup-after-setting-hidden-filters.good.al). + +## Anti Pattern + +A `FilterGroup(N)` call with N not 0 that is not followed by a restore before the procedure ends, or before the record is passed on or filtered further. Also a custom `SetRange`/`SetFilter` in filter group 4, 6, or 7, or in 1 or 3, on a page, report, or link-driven record. + +Do not flag a procedure that sets a hidden group on a local record variable it discards immediately afterwards, or a `FilterGroup()` call that only reads the current group. + +See sample: [`restore-the-filtergroup-after-setting-hidden-filters.bad.al`](restore-the-filtergroup-after-setting-hidden-filters.bad.al). + +## References + +[Record.FilterGroup method](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/methods-auto/record/record-filtergroup-method#remarks): table of platform filter groups, the warning that using group 4 replaces `SubPageLink` filtering, the warning about group 7, and the rule that filters in all groups are combined. diff --git a/microsoft/knowledge/web-services/set-text-encoding-on-streams-exchanged-externally.bad.al b/microsoft/knowledge/web-services/set-text-encoding-on-streams-exchanged-externally.bad.al new file mode 100644 index 00000000..841cf7c1 --- /dev/null +++ b/microsoft/knowledge/web-services/set-text-encoding-on-streams-exchanged-externally.bad.al @@ -0,0 +1,26 @@ +// Demonstration only. Shows the wrong pattern: exported text uses the default MSDos stream encoding. + +codeunit 50190 "Customer CSV Export Bad" +{ + procedure ExportCustomers() + var + Customer: Record Customer; + ExportBuffer: Record "Export Buffer Demo" temporary; + CsvOutStream: OutStream; + CsvInStream: InStream; + FileName: Text; + begin + // BAD: no TextEncoding, so the stream uses MSDos. Names such as "Müller" or "Łódź" + // arrive corrupted in the downloaded file. + ExportBuffer.Content.CreateOutStream(CsvOutStream); + if Customer.FindSet() then + repeat + CsvOutStream.WriteText(Customer."No." + ';' + Customer.Name); + CsvOutStream.WriteText(); + until Customer.Next() = 0; + + ExportBuffer.Content.CreateInStream(CsvInStream); + FileName := 'customers.csv'; + DownloadFromStream(CsvInStream, '', '', '', FileName); + end; +} diff --git a/microsoft/knowledge/web-services/set-text-encoding-on-streams-exchanged-externally.good.al b/microsoft/knowledge/web-services/set-text-encoding-on-streams-exchanged-externally.good.al new file mode 100644 index 00000000..cb2a2bc0 --- /dev/null +++ b/microsoft/knowledge/web-services/set-text-encoding-on-streams-exchanged-externally.good.al @@ -0,0 +1,24 @@ +// Demonstration only. Shows the recommended pattern: exported text uses an explicit UTF-8 encoding on both streams. + +codeunit 50191 "Customer CSV Export Good" +{ + procedure ExportCustomers() + var + Customer: Record Customer; + ExportBuffer: Record "Export Buffer Demo" temporary; + CsvOutStream: OutStream; + CsvInStream: InStream; + FileName: Text; + begin + ExportBuffer.Content.CreateOutStream(CsvOutStream, TextEncoding::UTF8); + if Customer.FindSet() then + repeat + CsvOutStream.WriteText(Customer."No." + ';' + Customer.Name); + CsvOutStream.WriteText(); + until Customer.Next() = 0; + + ExportBuffer.Content.CreateInStream(CsvInStream, TextEncoding::UTF8); + FileName := 'customers.csv'; + DownloadFromStream(CsvInStream, '', '', '', FileName); + end; +} diff --git a/microsoft/knowledge/web-services/set-text-encoding-on-streams-exchanged-externally.md b/microsoft/knowledge/web-services/set-text-encoding-on-streams-exchanged-externally.md new file mode 100644 index 00000000..65e0e757 --- /dev/null +++ b/microsoft/knowledge/web-services/set-text-encoding-on-streams-exchanged-externally.md @@ -0,0 +1,32 @@ +--- +bc-version: [all] +domain: web-services +keywords: [textencoding, text-encoding, utf8, msdos, createoutstream, createinstream, blob, file-export, character-corruption] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Set an explicit text encoding on BLOB streams whose bytes leave or enter Business Central + +## Description + +`Blob.CreateInStream` and `Blob.CreateOutStream` take an optional `TextEncoding`; when it is omitted, the stream uses `MSDos` (OEM) encoding, whose code page depends on the server's system locale. Text written with `WriteText` and read back with `ReadText` through streams created with the same default round-trips correctly inside Business Central, so the problem stays hidden in tests. It appears when the bytes cross a system boundary: a downloaded CSV, a file sent to a bank, an HTTP body, or an imported file produced elsewhere. Accented and non-Latin characters (for example `à`, `ß`, `ø`, `ł`) are then corrupted or replaced. In Business Central online the server locale cannot be controlled, so Microsoft recommends UTF-8 or UTF-16. + +## Best Practice + +Pass `TextEncoding::UTF8` (or the encoding the counterpart requires, such as `UTF16` or `Windows`) whenever a BLOB stream carries text that is downloaded, uploaded, sent over HTTP, or exchanged with another system. Use the same encoding on both the writing and the reading side. Treat `MSDos` as legacy, chosen only when the receiving system demands an OEM code page. + +Do not flag the default encoding when the stream carries binary content (images, PDFs, ZIP archives) or when text is written and read back only inside Business Central through streams with the same encoding. + +See sample: [`set-text-encoding-on-streams-exchanged-externally.good.al`](set-text-encoding-on-streams-exchanged-externally.good.al). + +## Anti Pattern + +`CreateOutStream(OutStr)` or `CreateInStream(InStr)` without an encoding argument on a BLOB whose text is then passed to `DownloadFromStream`, `File.DownloadFromStream`, an `HttpContent`, or a file-exchange or bank-export routine, or that was filled by `UploadIntoStream` from an external file. The defect is invisible with ASCII-only test data. + +See sample: [`set-text-encoding-on-streams-exchanged-externally.bad.al`](set-text-encoding-on-streams-exchanged-externally.bad.al). + +## References + +[Blob.CreateInStream method](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/methods-auto/blob/blob-createinstream-method) (the default encoding is MSDos) and [File handling and text encoding](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/devenv-file-handling-and-text-encoding) (MS-DOS depends on the server's system locale; online, use UTF-8/UTF-16). diff --git a/microsoft/skills/review/al-data-modeling-review.md b/microsoft/skills/review/al-data-modeling-review.md index fa76847f..ef22653c 100644 --- a/microsoft/skills/review/al-data-modeling-review.md +++ b/microsoft/skills/review/al-data-modeling-review.md @@ -16,7 +16,7 @@ application-area: [all] Reviews AL source changes against the `data-modeling` knowledge domain in BCQuality and emits a findings report. This is a leaf action skill: it invokes no sub-skills. It is one of the skills composed by `al-code-review`. -An orchestrator invokes this skill with a `pr-diff`, `file-path`, or `folder-path`. Data-modeling findings are narrow by design — they apply when the review scope contains setup or master tables, their card pages, primary keys, number-series assignment, block enforcement, audit fields, document print/email/Post-and-Send actions, `Navigate` page subscribers, Report Selection registration or dispatch, price-calculation/price-source extensibility, code that reads or writes sales/purchase/service line price and amount fields, `TransferFields`-based posting-cascade field mirroring, `TableRelation` field-length design, barcode/report-layout font-provider usage, dimension wiring, journal-based posting-routine structure, Item Ledger Entry document-number lookups after a combined sales post, or code that inserts or deletes master/reference records. The skill returns `not-applicable` when none of those apply. +An orchestrator invokes this skill with a `pr-diff`, `file-path`, or `folder-path`. Data-modeling findings are narrow by design — they apply when the review scope contains setup or master tables, their card pages, primary keys, number-series assignment, block enforcement, audit fields, document print/email/Post-and-Send actions, `Navigate` page subscribers, Report Selection registration or dispatch, price-calculation/price-source extensibility, code that reads or writes sales/purchase/service line price and amount fields, `TransferFields`-based posting-cascade field mirroring, `TableRelation` field-length design, barcode/report-layout font-provider usage, dimension wiring, journal-based posting-routine structure, Item Ledger Entry document-number lookups after a combined sales post, code that reads a `Blob` field's stream, or code that inserts or deletes master/reference records. The skill returns `not-applicable` when none of those apply. ## Source @@ -39,7 +39,7 @@ Narrow the relevant files to the subset that applies to the changes under review - The changed AL object names and types — especially `* Setup` singleton tables and Card pages, custom master tables, tableextensions that add master-data fields, document or journal lines that reference a master, document pages/codeunits exposing print/email/Post-and-Send actions, codeunits subscribing to `Navigate`, enumextensions to `"Report Selection Usage"`/`"Price Calculation Handler"`/`"Price Source Type"`, and report objects that render barcodes. - The changed fields, keys, triggers, and procedures, weighted toward `Primary Key`, `No.`, `No. Series`, `Blocked`, `Last Date Modified`, `OnInsert`, `OnModify`, `OnRename`, reference-field `OnValidate`, posting validation, and posting-cascade `TransferFields` calls. -- Tokens extracted from the diff that relate to data modeling (`setup`, `master`, `Primary Key`, `Code[10]`, `Code[20]`, `AutoIncrement`, `SystemId`, `No.`, `No. Series`, `NoSeriesManagement`, `Codeunit "No. Series"`, `GetNextNo`, `IsManual`, `TestManual`, `Blocked`, `TestField`, `Last Date Modified`, `Today`, `WorkDate`, `InsertAllowed`, `DeleteAllowed`, `PageType = Card`, `OnOpenPage`, `GetRecordOnce`, `OnInsert`, `OnModify`, `OnRename`, `InitRecord`, `Round`, `Precision`, `Direction`, `TableRelation`, `ValidateTableRelation`, `TestTableRelation`, `Text[`, `tableextension`, `enumextension`, `Media`, `MediaSet`, `Item`, `Count`, `TransferFields`, `Navigate`, `OnAfterFindRecords`, `OnBeforeShowRecords`, `Report Selections`, `Report Selection Usage`, `InsertRecord`, `Document Sending Profile`, `PrintForCust`, `PrintWithDialogForCust`, `PrintWithDialogForVend`, `SendEmailToCust`, `SendEmailToVendor`, `Report.RunModal`, `Report.Run`, `Price Calculation Handler`, `Price Calculation`, `OnFindSupportedSetup`, `Price Calculation Setup`, `Price Source Type`, `PriceSourceList`, `OnAfterAddSources`, `UpdateUnitPrice`, `PlanPriceCalcByField`, `UpdateUnitPriceByField`, `Prices Including VAT`, `Unit Price`, `Direct Unit Cost`, `Line Amount`, `Prepmt. Line Amount`, `Amount Including VAT`, `CalculateOutstandingAmountExclTax`, `Barcode Font Provider`, `Barcode Font Provider 2D`, `EncodeFont`, `ValidateInput`, `Insert`, `Delete`, `DeleteAll`). +- Tokens extracted from the diff that relate to data modeling (`setup`, `master`, `Primary Key`, `Code[10]`, `Code[20]`, `AutoIncrement`, `SystemId`, `No.`, `No. Series`, `NoSeriesManagement`, `Codeunit "No. Series"`, `GetNextNo`, `IsManual`, `TestManual`, `Blocked`, `TestField`, `Last Date Modified`, `Today`, `WorkDate`, `InsertAllowed`, `DeleteAllowed`, `PageType = Card`, `OnOpenPage`, `GetRecordOnce`, `OnInsert`, `OnModify`, `OnRename`, `InitRecord`, `Round`, `Precision`, `Direction`, `TableRelation`, `ValidateTableRelation`, `TestTableRelation`, `Text[`, `tableextension`, `enumextension`, `Media`, `MediaSet`, `Item`, `Count`, `TransferFields`, `Navigate`, `OnAfterFindRecords`, `OnBeforeShowRecords`, `Report Selections`, `Report Selection Usage`, `InsertRecord`, `Document Sending Profile`, `PrintForCust`, `PrintWithDialogForCust`, `PrintWithDialogForVend`, `SendEmailToCust`, `SendEmailToVendor`, `Report.RunModal`, `Report.Run`, `Price Calculation Handler`, `Price Calculation`, `OnFindSupportedSetup`, `Price Calculation Setup`, `Price Source Type`, `PriceSourceList`, `OnAfterAddSources`, `UpdateUnitPrice`, `PlanPriceCalcByField`, `UpdateUnitPriceByField`, `Prices Including VAT`, `Unit Price`, `Direct Unit Cost`, `Line Amount`, `Prepmt. Line Amount`, `Amount Including VAT`, `CalculateOutstandingAmountExclTax`, `Barcode Font Provider`, `Barcode Font Provider 2D`, `EncodeFont`, `ValidateInput`, `Insert`, `Delete`, `DeleteAll`, `Blob`, `CalcFields`, `SetAutoCalcFields`, `CreateInStream`). A file enters the candidate worklist when its `keywords` intersect the extracted tokens or its topic (derived from the index entry's `path`, `title`, and `description`) matches a changed object type. Read an article's full file — its `## Best Practice` / `## Anti Pattern` bodies — only after it makes the worklist; candidate selection uses the index alone. When the diff contains no data-modeling changes by any of the above signals, return `outcome: "not-applicable"` without evaluating files. @@ -75,6 +75,7 @@ The following targeted checks cover every current `data-modeling` article. Treat - A journal-based posting codeunit is added or changed and validation, Journal-table access, ledger writes, and user-interaction (`Confirm`/dialogs) all occur in one procedure or one codeunit, rather than split across `Check Line`/`Post Line`/`Post Batch`-shaped companions — `check-post-line-batch-pattern`. A document posting routine calling `Post Line` directly without a `Post Batch` companion is not this anti-pattern. - An existing, already-published table's `keys` block adds, removes, or reorders a field in its primary key or any `Clustered = true` key — `do-not-change-primary-key`. A new table defining its own key for the first time is not this anti-pattern; requires repository/publication context to know the table has already shipped. - Code reads a setup/configuration-table field inside a branch that has already decided the value is required, and blank/zero is handled with a fallback to a default rather than `TestField`/an equivalent guard — `testfield-required-setup-field`. A read that is genuinely optional in that branch, or one already guarded by `TestField`, is not this anti-pattern. +- A record is read from the database (`Get`, `Find*`, `Next`) and `CreateInStream` is then called on one of its `Blob` fields with no `CalcFields` or prior `SetAutoCalcFields` on that field — `calcfields-blob-before-reading-its-stream`. Do not match a BLOB just written through `CreateOutStream` on the same variable, a temporary record filled in the same session, or `Media`/`MediaSet` fields. Once the candidate worklist is known, resolve layer-precedence conflicts per READ. Drop lower-precedence files whose normative guidance (`## Best Practice` or `## Anti Pattern`) directly contradicts a higher-precedence candidate, and record each dropped file in `suppressed` with `reason: "layer-precedence"`. Files that would have been candidates but are hidden because their layer is disabled in consumer configuration are recorded with `reason: "configuration"`. Files that never became candidates are NOT recorded in `suppressed`. @@ -104,7 +105,7 @@ Outcome selection: - `completed` — the skill evaluated every worklist item. - `no-knowledge` — no applicable data-modeling knowledge survived filtering. -- `not-applicable` — the diff touches no setup/master table, page, key, numbering, block-check, or audit-field surface, and no document print/email/Post-and-Send action, `Navigate` subscriber, Report Selection registration/dispatch, price-calculation/price-source extensibility point, sales/purchase/service line price or amount read/write, posting-cascade `TransferFields` mirroring, `TableRelation` field length, barcode/report-font-provider usage, dimension wiring, posting-routine structure, Item-Ledger-Entry-document-number surface, or master/reference-record insert/delete call. +- `not-applicable` — the diff touches no setup/master table, page, key, numbering, block-check, or audit-field surface, and no document print/email/Post-and-Send action, `Navigate` subscriber, Report Selection registration/dispatch, price-calculation/price-source extensibility point, sales/purchase/service line price or amount read/write, posting-cascade `TransferFields` mirroring, `TableRelation` field length, barcode/report-font-provider usage, dimension wiring, posting-routine structure, Item-Ledger-Entry-document-number surface, `Blob` stream read, or master/reference-record insert/delete call. - `partial` — a budget was hit before the worklist was exhausted. - `failed` — an unrecoverable error occurred. diff --git a/microsoft/skills/review/al-events-review.md b/microsoft/skills/review/al-events-review.md index 1a5feeab..972e322f 100644 --- a/microsoft/skills/review/al-events-review.md +++ b/microsoft/skills/review/al-events-review.md @@ -39,7 +39,7 @@ Narrow the relevant files to the subset that applies to the changes under review - The changed AL object names and types — especially codeunits that publish events or host event subscribers, posting/release/validation routines that should expose extension points, and test codeunits that bind subscribers. - The changed procedures and triggers, weighted toward event publisher methods, methods carrying the `[EventSubscriber(...)]` attribute, routines that raise `OnBefore`/`OnAfter` events, and any procedure that calls `BindSubscription`/`UnbindSubscription`. -- Tokens extracted from the diff that relate to events and the publish/subscribe model (`IntegrationEvent`, `BusinessEvent`, `InternalEvent`, `EventSubscriber`, `IsHandled`, `BindSubscription`, `UnbindSubscription`, `EventSubscriberInstance`, `OnBefore`, `OnAfter`, `Manual`, `IncludeSender`, `GlobalVarAccess`, `Isolated`, `local`, `internal`, `Sender`, `this`, `RecordRef`, `xRec`, `temporary`, `Temp`, `repeat`, `ChangeCompany`, `StartSession`, `RunTrigger`, `GetDatabaseTableTriggerSetup`, `OnAfterGetDatabaseTableTriggerSetup`, `GlobalTriggerManagement`, `Global Triggers`, `OnDatabaseInsert`, `OnDatabaseModify`, `OnDatabaseDelete`, `OnDatabaseRename`). +- Tokens extracted from the diff that relate to events and the publish/subscribe model (`IntegrationEvent`, `BusinessEvent`, `InternalEvent`, `EventSubscriber`, `IsHandled`, `BindSubscription`, `UnbindSubscription`, `EventSubscriberInstance`, `OnBefore`, `OnAfter`, `Manual`, `IncludeSender`, `GlobalVarAccess`, `Isolated`, `local`, `internal`, `Sender`, `this`, `RecordRef`, `xRec`, `temporary`, `Temp`, `repeat`, `ChangeCompany`, `StartSession`, `RunTrigger`, `GetDatabaseTableTriggerSetup`, `OnAfterGetDatabaseTableTriggerSetup`, `Commit`, `GlobalTriggerManagement`, `Global Triggers`, `OnDatabaseInsert`, `OnDatabaseModify`, `OnDatabaseDelete`, `OnDatabaseRename`). A file enters the candidate worklist when its `keywords` intersect the extracted tokens or its topic (derived from the index entry's `path`, `title`, and `description`) matches a changed object type. Read an article's full file — its `## Best Practice` / `## Anti Pattern` bodies — only after it makes the worklist; candidate selection uses the index alone. @@ -67,6 +67,8 @@ The following targeted checks map diff signals to specific `events` articles. Tr - An `if IsHandled then exit;` whose skipped body performs posting, ledger-entry creation, number-series consumption, or integrity/permission validation — `do-not-bypass-critical-operations-with-ishandled`. - A record variable that had `ChangeCompany()` called on it and is later used with `Insert`, `Modify`, `Delete`, or `Validate`, where the table is not owned by the extension, has triggers that read company data, or has trigger-event subscribers that do not exit on `RunTrigger = false` — `changecompany-runs-triggers-in-the-calling-company`. Do not match a read-only use after `ChangeCompany`, a write with `RunTrigger = false` into an extension-owned table whose triggers do not read company data and whose trigger-event subscribers exit on `RunTrigger = false`, or the parameterless `ChangeCompany()` reset. - A subscriber to `GetDatabaseTableTriggerSetup` (`Global Triggers`) or `OnAfterGetDatabaseTableTriggerSetup` (`GlobalTriggerManagement`) that assigns one of its `var` flags (`OnDatabaseInsert`, `OnDatabaseModify`, `OnDatabaseDelete`, `OnDatabaseRename`) a literal `false` or a lookup/Boolean expression that does not `or` in the flag's current value, or that calls `Clear` on one — `database-trigger-setup-flags-may-only-be-set-to-true`. Do not match `Flag := true` under a condition, `Flag := Flag or `, or `if not Flag then Flag := false;`, none of which can clear a flag. Do not match code that runs only in demo-data generation or test-library sessions (for example the base application's demo data tool or a test library's backup/restore subscriber), where clearing other features' flags is accepted. +- An event declared with `Isolated = true` (BC20/runtime 9.0 or later) is raised after uncommitted `Insert`/`Modify`/`Delete` calls in the same transaction, or a `Commit()` is inserted immediately before the raise inside a posting, release, or other all-or-nothing routine to obtain isolation — `isolated-events-are-isolated-only-outside-a-write-transaction`. Do not match an isolated event raised from a procedure that has written nothing, or a normal event. +- A newly designed event declared with `Isolated = true` carries `var IsHandled`, a `var` result, or a `var` record that the publisher reads afterwards to decide its behaviour — `isolated-event-rollback-does-not-undo-var-parameters`. Changing `Isolated` on a shipped event belongs to `do-not-change-shipped-event-attribute-flags` instead. ## Action diff --git a/microsoft/skills/review/al-security-review.md b/microsoft/skills/review/al-security-review.md index 3d70173e..98d8bee9 100644 --- a/microsoft/skills/review/al-security-review.md +++ b/microsoft/skills/review/al-security-review.md @@ -39,12 +39,14 @@ Narrow the relevant files to the subset that applies to the changes under review - The changed AL object names and types — especially permission sets, codeunits handling authentication or authorization, objects touching `Isolated Storage`, `OAuth2` flows, web service endpoints, API pages, event publishers, and RecordRef helpers. - The changed procedures and triggers, weighted toward those that call `HttpClient`, validate or compose URLs, write to telemetry, read or write secrets, unwrap SecretText, manipulate record-level security, expose var Boolean guard parameters, or bypass the permission model (for example, `RecordRef.Open`, `Record.WritePermission`, direct table access from a non-owning app). -- Tokens extracted from the diff that relate to security concerns (`IsolatedStorage`, `SetEncrypted`, `OAuth2`, `SecretText`, `Unwrap`, `NonDebuggable`, `Password`, `Token`, `HttpClient`, `Uri`, `AreURIsHaveSameHost`, `IsValidURIPattern`, `RecordRef`, `RecordId`, `TransferFields`, `Codeunit.Run`, `Access = Internal`, `internalsVisibleTo`, `Open`, `IntegrationEvent`, `SkipValidation`, `HasAccess`, `Permission`, `UserSecurityId`, `Commit`, `SetFilter`, `ServiceEnabled`, `PageType = API`, `QueryType = API`, `permissionset`, `Web Services`). +- Tokens extracted from the diff that relate to security concerns (`IsolatedStorage`, `SetEncrypted`, `OAuth2`, `SecretText`, `Unwrap`, `NonDebuggable`, `Password`, `Token`, `HttpClient`, `Uri`, `AreURIsHaveSameHost`, `IsValidURIPattern`, `RecordRef`, `RecordId`, `TransferFields`, `Codeunit.Run`, `Access = Internal`, `internalsVisibleTo`, `Open`, `IntegrationEvent`, `SkipValidation`, `HasAccess`, `Permission`, `UserSecurityId`, `Commit`, `SetFilter`, `ServiceEnabled`, `PageType = API`, `QueryType = API`, `permissionset`, `Web Services`, `FilterGroup`, `OnOpenPage`, `SetRange`). A file enters the candidate worklist when its `keywords` intersect the extracted tokens or its topic (derived from the index entry's `path`, `title`, and `description`) matches a changed object type. Read an article's full file — its `## Best Practice` / `## Anti Pattern` bodies — only after it makes the worklist; candidate selection uses the index alone. Always worklist `internal-access-is-not-a-security-boundary.md` when changed comments or code rely on `Access = Internal` or `internalsVisibleTo` to protect a sensitive operation, or an internal `OnRun` codeunit performs privileged work without an independent authorization boundary. Do not flag `internal` used only to keep implementation details out of the supported API. +Worklist `page-scope-filters-must-not-be-in-filtergroup-zero.md` when a page trigger (typically `OnOpenPage`) restricts `Rec` to a user-, role-, or context-specific subset with `SetRange`/`SetFilter` while the current filter group is 0. Do not flag a group 0 filter meant as a removable default, such as a default date range. Worklist `restore-the-filtergroup-after-setting-hidden-filters.md` when a non-zero `FilterGroup(N)` call is not followed by a restore before the record is filtered further or passed on, or when custom filters are set in group 1, 3, 4, 6, or 7. + For secret values, select the most specific sink owner: - When a `Text`/`Code` credential is declared, passed, returned, or unwrapped without a visible HTTP URI/header/body sink, use `secrettext-for-credentials.md`. diff --git a/microsoft/skills/review/al-web-services-review.md b/microsoft/skills/review/al-web-services-review.md index 6d7d71a0..42cf1da5 100644 --- a/microsoft/skills/review/al-web-services-review.md +++ b/microsoft/skills/review/al-web-services-review.md @@ -41,11 +41,12 @@ Narrow the relevant files to the subset that applies to the changes under review - The changed properties and triggers, weighted toward API page metadata (`APIPublisher`, `APIGroup`, `APIVersion`, `EntityName`, `EntitySetName`, `ODataKeyFields`, `SourceTable`, `SourceTableTemporary`), navigation metadata (`SubPageLink`, `Multiplicity`, and visible singleton or collection semantics), CRUD guards (`InsertAllowed`, `ModifyAllowed`, `DeleteAllowed`, `Editable`), the `OnOpenPage` trigger, and `OnValidate` triggers on exposed fields. - Outbound HTTP integration code that constructs or sends requests, captures a client method's optional Boolean result, checks an `HttpResponseMessage`, or reads and parses response content. - Integration code that converts values to or from exchanged text: `Format` or `Evaluate` on a request URL, body, or file line, and `JsonToken`/`JsonValue` conversions of payload properties. +- Integration code that writes or reads exchanged text through `Blob` streams: `CreateOutStream`/`CreateInStream` without a `TextEncoding` argument whose content reaches `DownloadFromStream`, `File.DownloadFromStream`, an `HttpContent`, or a file/bank export, or that was filled by `UploadIntoStream` — `set-text-encoding-on-streams-exchanged-externally`. Do not match binary content or text that is only written and read back inside Business Central. - Webhook subscriber handlers and subscription lifecycle code, especially code that creates or renews subscriptions, handles `validationToken`, schedules from `expirationDateTime`, or targets resources whose eligibility is visible in the diff. - An API page (`PageType = API`) is added or changed and its `InsertAllowed`/`ModifyAllowed`/`DeleteAllowed` properties are left at their default `true` (or explicitly set `true`) for an operation the endpoint's stated purpose does not need — `api-page-least-privilege-write-access.md`. The signal is an operation left enabled beyond what the endpoint's own described purpose requires, not the mere presence of these properties; a page that genuinely needs full CRUD and grants it deliberately is not a violation. - An API page (`PageType = API`) with `InsertAllowed = true` lists a field in `ODataKeyFields` and that same field control sets `Editable = false` — `api-page-key-fields-must-be-editable-on-insert.md`. A system-generated key such as `SystemId` marked read-only is not this anti-pattern. - An API page exposes a field via `Rec` directly, and that field is a stored value computed from other fields inside an `OnValidate` trigger rather than a FlowField recalculated in `OnAfterGetRecord` — `stored-derived-fields-must-not-be-exposed-directly.md`. Exposing a genuine FlowField, or a value already recalculated in `OnAfterGetRecord`, is not this anti-pattern. -- Tokens extracted from the diff that relate to API surface and behaviour (`PageType`, `QueryType`, `API`, `api-page`, `page-part`, `APIPublisher`, `APIGroup`, `APIVersion`, `EntityName`, `EntitySetName`, `ODataKeyFields`, `SystemId`, `SubPageLink`, `subpagelink`, `Multiplicity`, `multiplicity`, `Many`, `ZeroOrOne`, `SourceTableTemporary`, `Job Queue Entry`, `webhook`, `webhookSupportedResources`, `webhook-supported-resources`, `subscriptions`, `notificationUrl`, `validationToken`, `validationtoken`, `expirationDateTime`, `expirationdatetime`, `ServiceEnabled`, `WebServiceActionContext`, `SetActionResponse`, `ReadIsolation`, `IsolationLevel`, `ReadCommitted`, `InsertAllowed`, `ModifyAllowed`, `DeleteAllowed`, `Editable`, `SourceTable`, `HttpClient`, `HttpRequestMessage`, `HttpResponseMessage`, `Get`, `Post`, `Put`, `Delete`, `Send`, `IsSuccessStatusCode`, `HttpStatusCode`, `Content`, `ReadAs`, `JsonObject`, `JsonToken`, `JsonValue`, `AsValue`, `IsNull`, `SelectToken`, `Format`, `Evaluate`, `XmlDocument`). +- Tokens extracted from the diff that relate to API surface and behaviour (`PageType`, `QueryType`, `API`, `api-page`, `page-part`, `APIPublisher`, `APIGroup`, `APIVersion`, `EntityName`, `EntitySetName`, `ODataKeyFields`, `SystemId`, `SubPageLink`, `subpagelink`, `Multiplicity`, `multiplicity`, `Many`, `ZeroOrOne`, `SourceTableTemporary`, `Job Queue Entry`, `webhook`, `webhookSupportedResources`, `webhook-supported-resources`, `subscriptions`, `notificationUrl`, `validationToken`, `validationtoken`, `expirationDateTime`, `expirationdatetime`, `ServiceEnabled`, `WebServiceActionContext`, `SetActionResponse`, `ReadIsolation`, `IsolationLevel`, `ReadCommitted`, `InsertAllowed`, `ModifyAllowed`, `DeleteAllowed`, `Editable`, `SourceTable`, `HttpClient`, `HttpRequestMessage`, `HttpResponseMessage`, `Get`, `Post`, `Put`, `Delete`, `Send`, `IsSuccessStatusCode`, `HttpStatusCode`, `Content`, `ReadAs`, `JsonObject`, `JsonToken`, `JsonValue`, `AsValue`, `IsNull`, `SelectToken`, `Format`, `Evaluate`, `XmlDocument`, `TextEncoding`, `CreateOutStream`, `CreateInStream`, `DownloadFromStream`, `UploadIntoStream`). A file enters the candidate worklist when its `keywords` intersect the extracted tokens or its topic (derived from the index entry's `path`, `title`, and `description`) matches a changed object type. Read an article's full file — its `## Best Practice` / `## Anti Pattern` bodies — only after it makes the worklist; candidate selection uses the index alone. @@ -86,7 +87,7 @@ Outcome selection: - `completed` — the skill evaluated every worklist item; default when the skill finishes normally, including when the resulting `findings` array is empty. - `no-knowledge` — no applicable web-services knowledge survived Source, Relevance, configuration filtering, and conflict resolution. `findings` is empty. -- `not-applicable` — the task context contains no AL API surface, outbound AL HTTP integration, JavaScript webhook subscription lifecycle code, or JavaScript notification handler, or the `technologies` filter rejected the task. +- `not-applicable` — the task context contains no AL API surface, outbound AL HTTP integration, AL file or stream text exchange, JavaScript webhook subscription lifecycle code, or JavaScript notification handler, or the `technologies` filter rejected the task. - `partial` — a time or token budget was hit before the worklist was exhausted. `summary.coverage` reflects the evaluated subset; `outcome-reason` explains the cause. - `failed` — an unrecoverable error occurred. `outcome-reason` is required. From 36a5bf338b11b23320ddea404c7ce0514464881d Mon Sep 17 00:00:00 2001 From: Stefano Demiliani Date: Mon, 19 Oct 2026 14:16:18 +0200 Subject: [PATCH 2/2] fix: narrow stream encoding guidance and complete AL samples Exclude byte-preserving transfers from encoding findings and exercise UTF-8 download through a default InStream as a clean control. Include independent demo table declarations with Blob fields and shipment status definitions in all eight affected samples. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- evaluation/README.md | 8 ++++++++ ...elds-blob-before-reading-its-stream.bad.al | 16 +++++++++++++++ ...lds-blob-before-reading-its-stream.good.al | 16 +++++++++++++++ ...ed-only-outside-a-write-transaction.bad.al | 20 +++++++++++++++++++ ...d-only-outside-a-write-transaction.good.al | 20 +++++++++++++++++++ ...ers-must-not-be-in-filtergroup-zero.bad.al | 17 ++++++++++++++++ ...rs-must-not-be-in-filtergroup-zero.good.al | 17 ++++++++++++++++ ...ing-on-streams-exchanged-externally.bad.al | 16 +++++++++++++++ ...ng-on-streams-exchanged-externally.good.al | 20 +++++++++++++++++-- ...ncoding-on-streams-exchanged-externally.md | 12 ++++++----- .../skills/review/al-web-services-review.md | 2 +- 11 files changed, 156 insertions(+), 8 deletions(-) diff --git a/evaluation/README.md b/evaluation/README.md index 5863fcc1..55323235 100644 --- a/evaluation/README.md +++ b/evaluation/README.md @@ -15,6 +15,14 @@ is mandatory. Model-facing preparation hashes case IDs, neutralizes `Good`/`Bad` object-name tokens, and removes full-line sample comments so neither the article slug, domain, nor expected outcome reveals the answer. +The core AL samples for BLOB loading, isolated shipment events, expense-page +scope filters, and stream encoding include their minimal demo tables in each +companion independently. BLOB field types and shipment status members therefore +survive preparation without relying on another sample. The stream-encoding +clean control deliberately writes UTF-8 text and downloads the existing bytes +through a default-encoding InStream: transport alone does not transcode text +and must not trigger the encoding rule. + The SCM `articles` override deliberately selects every rule in the initial functional domain, producing nine positive cases and nine clean controls. Business context is executable: document/status `TestField` guards, diff --git a/microsoft/knowledge/data-modeling/calcfields-blob-before-reading-its-stream.bad.al b/microsoft/knowledge/data-modeling/calcfields-blob-before-reading-its-stream.bad.al index a02f85f1..8af2f8fa 100644 --- a/microsoft/knowledge/data-modeling/calcfields-blob-before-reading-its-stream.bad.al +++ b/microsoft/knowledge/data-modeling/calcfields-blob-before-reading-its-stream.bad.al @@ -15,3 +15,19 @@ codeunit 50188 "Payload Reader Bad" exit(PayloadText); end; } + +table 50188 "Integration Log Demo" +{ + DataClassification = CustomerContent; + + fields + { + field(1; "Entry No."; Integer) { } + field(2; Payload; Blob) { } + } + + keys + { + key(PK; "Entry No.") { Clustered = true; } + } +} diff --git a/microsoft/knowledge/data-modeling/calcfields-blob-before-reading-its-stream.good.al b/microsoft/knowledge/data-modeling/calcfields-blob-before-reading-its-stream.good.al index eed88a22..41d6c522 100644 --- a/microsoft/knowledge/data-modeling/calcfields-blob-before-reading-its-stream.good.al +++ b/microsoft/knowledge/data-modeling/calcfields-blob-before-reading-its-stream.good.al @@ -15,3 +15,19 @@ codeunit 50189 "Payload Reader Good" exit(PayloadText); end; } + +table 50189 "Integration Log Demo" +{ + DataClassification = CustomerContent; + + fields + { + field(1; "Entry No."; Integer) { } + field(2; Payload; Blob) { } + } + + keys + { + key(PK; "Entry No.") { Clustered = true; } + } +} diff --git a/microsoft/knowledge/events/isolated-events-are-isolated-only-outside-a-write-transaction.bad.al b/microsoft/knowledge/events/isolated-events-are-isolated-only-outside-a-write-transaction.bad.al index d946ac53..a6714cb9 100644 --- a/microsoft/knowledge/events/isolated-events-are-isolated-only-outside-a-write-transaction.bad.al +++ b/microsoft/knowledge/events/isolated-events-are-isolated-only-outside-a-write-transaction.bad.al @@ -18,3 +18,23 @@ codeunit 50180 "Shipment Confirmation Bad" begin end; } + +table 50180 "Shipment Header Demo" +{ + DataClassification = CustomerContent; + + fields + { + field(1; "No."; Code[20]) { } + field(2; "Carrier Code"; Code[10]) { } + field(3; Status; Option) + { + OptionMembers = Open,Confirmed; + } + } + + keys + { + key(PK; "No.") { Clustered = true; } + } +} diff --git a/microsoft/knowledge/events/isolated-events-are-isolated-only-outside-a-write-transaction.good.al b/microsoft/knowledge/events/isolated-events-are-isolated-only-outside-a-write-transaction.good.al index 531002eb..881416b6 100644 --- a/microsoft/knowledge/events/isolated-events-are-isolated-only-outside-a-write-transaction.good.al +++ b/microsoft/knowledge/events/isolated-events-are-isolated-only-outside-a-write-transaction.good.al @@ -26,3 +26,23 @@ codeunit 50181 "Shipment Confirmation Good" begin end; } + +table 50181 "Shipment Header Demo" +{ + DataClassification = CustomerContent; + + fields + { + field(1; "No."; Code[20]) { } + field(2; "Carrier Code"; Code[10]) { } + field(3; Status; Option) + { + OptionMembers = Open,Confirmed; + } + } + + keys + { + key(PK; "No.") { Clustered = true; } + } +} diff --git a/microsoft/knowledge/security/page-scope-filters-must-not-be-in-filtergroup-zero.bad.al b/microsoft/knowledge/security/page-scope-filters-must-not-be-in-filtergroup-zero.bad.al index 7fc5b384..d28c94dc 100644 --- a/microsoft/knowledge/security/page-scope-filters-must-not-be-in-filtergroup-zero.bad.al +++ b/microsoft/knowledge/security/page-scope-filters-must-not-be-in-filtergroup-zero.bad.al @@ -18,6 +18,23 @@ page 50184 "My Expense Claims Bad" field("Employee User ID"; Rec."Employee User ID") { } field(Amount; Rec.Amount) { } } + + table 50184 "Expense Claim Demo" + { + DataClassification = CustomerContent; + + fields + { + field(1; "No."; Code[20]) { } + field(2; "Employee User ID"; Text[50]) { } + field(3; Amount; Decimal) { } + } + + keys + { + key(PK; "No.") { Clustered = true; } + } + } } } diff --git a/microsoft/knowledge/security/page-scope-filters-must-not-be-in-filtergroup-zero.good.al b/microsoft/knowledge/security/page-scope-filters-must-not-be-in-filtergroup-zero.good.al index 56dcb724..da0e2203 100644 --- a/microsoft/knowledge/security/page-scope-filters-must-not-be-in-filtergroup-zero.good.al +++ b/microsoft/knowledge/security/page-scope-filters-must-not-be-in-filtergroup-zero.good.al @@ -18,6 +18,23 @@ page 50185 "My Expense Claims Good" field("Employee User ID"; Rec."Employee User ID") { } field(Amount; Rec.Amount) { } } + + table 50185 "Expense Claim Demo" + { + DataClassification = CustomerContent; + + fields + { + field(1; "No."; Code[20]) { } + field(2; "Employee User ID"; Text[50]) { } + field(3; Amount; Decimal) { } + } + + keys + { + key(PK; "No.") { Clustered = true; } + } + } } } diff --git a/microsoft/knowledge/web-services/set-text-encoding-on-streams-exchanged-externally.bad.al b/microsoft/knowledge/web-services/set-text-encoding-on-streams-exchanged-externally.bad.al index 841cf7c1..4cebe5e6 100644 --- a/microsoft/knowledge/web-services/set-text-encoding-on-streams-exchanged-externally.bad.al +++ b/microsoft/knowledge/web-services/set-text-encoding-on-streams-exchanged-externally.bad.al @@ -24,3 +24,19 @@ codeunit 50190 "Customer CSV Export Bad" DownloadFromStream(CsvInStream, '', '', '', FileName); end; } + +table 50190 "Export Buffer Demo" +{ + DataClassification = CustomerContent; + + fields + { + field(1; "Entry No."; Integer) { } + field(2; Content; Blob) { } + } + + keys + { + key(PK; "Entry No.") { Clustered = true; } + } +} diff --git a/microsoft/knowledge/web-services/set-text-encoding-on-streams-exchanged-externally.good.al b/microsoft/knowledge/web-services/set-text-encoding-on-streams-exchanged-externally.good.al index cb2a2bc0..82ae7e42 100644 --- a/microsoft/knowledge/web-services/set-text-encoding-on-streams-exchanged-externally.good.al +++ b/microsoft/knowledge/web-services/set-text-encoding-on-streams-exchanged-externally.good.al @@ -1,4 +1,4 @@ -// Demonstration only. Shows the recommended pattern: exported text uses an explicit UTF-8 encoding on both streams. +// Demonstration only. UTF-8 text is downloaded byte-for-byte through a default-encoding InStream. codeunit 50191 "Customer CSV Export Good" { @@ -17,8 +17,24 @@ codeunit 50191 "Customer CSV Export Good" CsvOutStream.WriteText(); until Customer.Next() = 0; - ExportBuffer.Content.CreateInStream(CsvInStream, TextEncoding::UTF8); + ExportBuffer.Content.CreateInStream(CsvInStream); FileName := 'customers.csv'; DownloadFromStream(CsvInStream, '', '', '', FileName); end; } + +table 50191 "Export Buffer Demo" +{ + DataClassification = CustomerContent; + + fields + { + field(1; "Entry No."; Integer) { } + field(2; Content; Blob) { } + } + + keys + { + key(PK; "Entry No.") { Clustered = true; } + } +} diff --git a/microsoft/knowledge/web-services/set-text-encoding-on-streams-exchanged-externally.md b/microsoft/knowledge/web-services/set-text-encoding-on-streams-exchanged-externally.md index 65e0e757..0915e6cc 100644 --- a/microsoft/knowledge/web-services/set-text-encoding-on-streams-exchanged-externally.md +++ b/microsoft/knowledge/web-services/set-text-encoding-on-streams-exchanged-externally.md @@ -7,23 +7,25 @@ countries: [w1] application-area: [all] --- -# Set an explicit text encoding on BLOB streams whose bytes leave or enter Business Central +# Set the text encoding when converting exchanged text to or from BLOB bytes ## Description -`Blob.CreateInStream` and `Blob.CreateOutStream` take an optional `TextEncoding`; when it is omitted, the stream uses `MSDos` (OEM) encoding, whose code page depends on the server's system locale. Text written with `WriteText` and read back with `ReadText` through streams created with the same default round-trips correctly inside Business Central, so the problem stays hidden in tests. It appears when the bytes cross a system boundary: a downloaded CSV, a file sent to a bank, an HTTP body, or an imported file produced elsewhere. Accented and non-Latin characters (for example `à`, `ß`, `ø`, `ł`) are then corrupted or replaced. In Business Central online the server locale cannot be controlled, so Microsoft recommends UTF-8 or UTF-16. +`Blob.CreateInStream` and `Blob.CreateOutStream` take an optional `TextEncoding`; when it is omitted, the stream uses `MSDos` (OEM) encoding, whose code page depends on the server's system locale. Encoding matters when an operation converts between AL text and bytes, such as `WriteText` or `ReadText`. Text written and read through streams with the same default can round-trip inside Business Central, but writing MSDos bytes for a UTF-8 consumer or reading externally encoded UTF-8 bytes as MSDos can corrupt accented and non-Latin characters (for example `à`, `ß`, `ø`, `ł`). In Business Central online the server locale cannot be controlled, so Microsoft recommends UTF-8 or UTF-16. + +Creating a stream does not transcode the BLOB. `DownloadFromStream`, stream-based HTTP content, raw uploads, and `CopyStream` can transfer existing bytes without decoding or re-encoding text. A default-encoding InStream used only to download bytes written by a UTF-8 OutStream preserves those UTF-8 bytes. ## Best Practice -Pass `TextEncoding::UTF8` (or the encoding the counterpart requires, such as `UTF16` or `Windows`) whenever a BLOB stream carries text that is downloaded, uploaded, sent over HTTP, or exchanged with another system. Use the same encoding on both the writing and the reading side. Treat `MSDos` as legacy, chosen only when the receiving system demands an OEM code page. +Pass `TextEncoding::UTF8` (or the encoding the counterpart requires, such as `UTF16` or `Windows`) on the stream that performs an encoding-sensitive text write or read. Match the writer to the external consumer's expected encoding and the reader to the actual encoding of the incoming bytes. If both sides convert text, configure compatible encodings on both. Treat `MSDos` as legacy, chosen only when the counterpart demands an OEM code page. -Do not flag the default encoding when the stream carries binary content (images, PDFs, ZIP archives) or when text is written and read back only inside Business Central through streams with the same encoding. +Do not flag the default encoding when the stream carries binary content (images, PDFs, ZIP archives), when text is written and read back only inside Business Central through streams with the same encoding, or when already encoded data is transferred byte-for-byte without text conversion. An omitted encoding on a stream used only for `DownloadFromStream`, stream-based HTTP transfer, raw upload, or `CopyStream` is not evidence of a defect. Trace the actual text conversion, not merely the transport API. See sample: [`set-text-encoding-on-streams-exchanged-externally.good.al`](set-text-encoding-on-streams-exchanged-externally.good.al). ## Anti Pattern -`CreateOutStream(OutStr)` or `CreateInStream(InStr)` without an encoding argument on a BLOB whose text is then passed to `DownloadFromStream`, `File.DownloadFromStream`, an `HttpContent`, or a file-exchange or bank-export routine, or that was filled by `UploadIntoStream` from an external file. The defect is invisible with ASCII-only test data. +An encoding-sensitive text write (such as `OutStr.WriteText`) or text read (such as `InStr.ReadText`) through a BLOB stream with a missing or incompatible encoding, where the external consumer expects a different encoding or the incoming bytes use a different encoding. For example, default MSDos `WriteText` output downloaded as a UTF-8 CSV, or UTF-8 uploaded bytes decoded with a default MSDos `ReadText`. Mere stream creation or byte-preserving transfer is not this anti-pattern. The defect can be invisible with ASCII-only test data. See sample: [`set-text-encoding-on-streams-exchanged-externally.bad.al`](set-text-encoding-on-streams-exchanged-externally.bad.al). diff --git a/microsoft/skills/review/al-web-services-review.md b/microsoft/skills/review/al-web-services-review.md index 42cf1da5..310c604d 100644 --- a/microsoft/skills/review/al-web-services-review.md +++ b/microsoft/skills/review/al-web-services-review.md @@ -41,7 +41,7 @@ Narrow the relevant files to the subset that applies to the changes under review - The changed properties and triggers, weighted toward API page metadata (`APIPublisher`, `APIGroup`, `APIVersion`, `EntityName`, `EntitySetName`, `ODataKeyFields`, `SourceTable`, `SourceTableTemporary`), navigation metadata (`SubPageLink`, `Multiplicity`, and visible singleton or collection semantics), CRUD guards (`InsertAllowed`, `ModifyAllowed`, `DeleteAllowed`, `Editable`), the `OnOpenPage` trigger, and `OnValidate` triggers on exposed fields. - Outbound HTTP integration code that constructs or sends requests, captures a client method's optional Boolean result, checks an `HttpResponseMessage`, or reads and parses response content. - Integration code that converts values to or from exchanged text: `Format` or `Evaluate` on a request URL, body, or file line, and `JsonToken`/`JsonValue` conversions of payload properties. -- Integration code that writes or reads exchanged text through `Blob` streams: `CreateOutStream`/`CreateInStream` without a `TextEncoding` argument whose content reaches `DownloadFromStream`, `File.DownloadFromStream`, an `HttpContent`, or a file/bank export, or that was filled by `UploadIntoStream` — `set-text-encoding-on-streams-exchanged-externally`. Do not match binary content or text that is only written and read back inside Business Central. +- Integration code that converts exchanged text to or from `Blob` bytes: trace encoding-sensitive operations such as `WriteText`/`ReadText` to `CreateOutStream`/`CreateInStream` with a missing or incompatible `TextEncoding`, relative to the counterpart's expected encoding or the incoming bytes — `set-text-encoding-on-streams-exchanged-externally`. Do not flag mere stream creation, binary content, compatible internal text round-trips, or byte-preserving download/HTTP/upload/`CopyStream` transfers of already encoded data. In particular, an explicit UTF-8 writer followed by a default-encoding InStream used only for download preserves the original bytes. - Webhook subscriber handlers and subscription lifecycle code, especially code that creates or renews subscriptions, handles `validationToken`, schedules from `expirationDateTime`, or targets resources whose eligibility is visible in the diff. - An API page (`PageType = API`) is added or changed and its `InsertAllowed`/`ModifyAllowed`/`DeleteAllowed` properties are left at their default `true` (or explicitly set `true`) for an operation the endpoint's stated purpose does not need — `api-page-least-privilege-write-access.md`. The signal is an operation left enabled beyond what the endpoint's own described purpose requires, not the mere presence of these properties; a page that genuinely needs full CRUD and grants it deliberately is not a violation. - An API page (`PageType = API`) with `InsertAllowed = true` lists a field in `ODataKeyFields` and that same field control sets `Editable = false` — `api-page-key-fields-must-be-editable-on-insert.md`. A system-generated key such as `SystemId` marked read-only is not this anti-pattern.