Skip to content

Conversation

@elinor-fung
Copy link
Member

When getting a property from the host, we'd end up with a string length that was greater than the actual property length. Close the buffer with the actual length written.

cc @dotnet/appmodel @AaronRobinsonMSFT

Copilot AI review requested due to automatic review settings January 29, 2026 21:15
@github-actions github-actions bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Jan 29, 2026
@elinor-fung elinor-fung added area-Host and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Jan 29, 2026
@dotnet-policy-service
Copy link
Contributor

Tagging subscribers to this area: @agocke, @jeffschwMSFT, @elinor-fung
See info in area-owners.md if you want to be subscribed.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes incorrect SString length handling when reading runtime properties from the host by aligning buffer open/close semantics with the host contract’s “length includes null terminator” behavior.

Changes:

  • Adjusts OpenUTF8Buffer sizing to account for SString’s implicit null terminator space.
  • Uses CloseBuffer(finalCount) with the actual number of bytes written (excluding the null terminator).
  • Adds tighter failure handling for invalid/failed host property reads.

Copy link
Member

@AaronRobinsonMSFT AaronRobinsonMSFT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One nit. Other than that, nice job tracking this mess down.

Copilot AI review requested due to automatic review settings January 29, 2026 22:17
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants