Backport #32277 to 3.1

[TanayParikh]

Description

Update guard logic to permit writing 0 length buffers. This can happen in certain globalized scenarios (ex. ¿ with Portugese) where the first char needs to be encoded.

Patch #31299 for 3.1 by cherry-picking the #32277 squash commit.

Customer Impact

Writing 0-length char buffers leads to an ArgumentOutOfRangeException. This may occur in a globalization context with certain chars such as ¿.

#31299 (comment)

Also impacts Orchard: #31299 (comment)

Regression?

• Yes
• No

[If yes, specify the version the behavior has regressed from]

Risk

• High
• Medium
• Low

Low risk as we've just updated the logic slightly to permit 0 length buffers.

Verification

• Manual (required)
• Automated

Packaging changes reviewed?

• Yes
• No
• N/A

Addresses #31299

[ghost]

Hello human! Please make sure you've included the Shiproom Template in a comment or (preferably) the PR description. Also, make sure this PR is not marked as a draft and is ready-to-merge.

[TanayParikh]

The release/3.1 branch seems to be failing pretty consistently, any ideas what may be going on there?

https://dev.azure.com/dnceng/public/_build?definitionId=278&_a=summary&repositoryFilter=329&branchFilter=80972

cc/ @Tratcher

[Tratcher]

That's concerning...

MSBUILD : error MSB1025: An internal failure occurred while running MSBuild.
Microsoft.Build.Shared.InternalErrorException: MSB0001: Internal MSBuild Error: We shouldn't be accessing the ProjectInstance when the configuration is cached.
   at Microsoft.Build.Shared.ErrorUtilities.ThrowInternalError(String message, Exception innerException, Object[] args)
   at Microsoft.Build.BackEnd.BuildRequestConfiguration.get_Project()
   at Microsoft.Build.BackEnd.Logging.NodeLoggingContext.LogProjectStarted(BuildRequest request, BuildRequestConfiguration configuration)
   at Microsoft.Build.BackEnd.Logging.NodeLoggingContext.LogRequestHandledFromCache(BuildRequest request, BuildRequestConfiguration configuration, BuildResult result)
   at Microsoft.Build.BackEnd.Scheduler.LogRequestHandledFromCache(BuildRequest request, BuildResult result)
   at Microsoft.Build.BackEnd.Scheduler.HandleRequestBlockedByNewRequests(SchedulableRequest parentRequest, BuildRequestBlocker blocker, List`1 responses)
   at Microsoft.Build.BackEnd.Scheduler.ReportRequestBlocked(Int32 nodeId, BuildRequestBlocker blocker)
   at Microsoft.Build.Execution.BuildManager.HandleNewRequest(Int32 node, BuildRequestBlocker blocker)
   at Microsoft.Build.Execution.BuildManager.ProcessPacket(Int32 node, INodePacket packet)
   at Microsoft.Build.Execution.BuildManager.<>c__DisplayClass76_0.<Microsoft.Build.BackEnd.INodePacketHandler.PacketReceived>b__0()
   at Microsoft.Build.Execution.BuildManager.ProcessWorkQueue(Action action)
--- End of stack trace from previous location where exception was thrown ---
   at Microsoft.Build.Execution.BuildManager.EndBuild()

[pranavkm]

FYI @ladipro / @rainersigwald

[ladipro]

This looks like a bug introduced with dotnet/msbuild#5997. Did you guys recently upgrade VS/MSBuild?

[rainersigwald]

Try incorporating 8052396 to work around the MSBuild regression, please.

[TanayParikh]

Thanks @rainersigwald, added 75b7808.

Given this is a servicing release would you mind confirming if anything changes in the servicing template in the initial comment? (ie. Customer Impact, Regression, Risk, Packaging, etc)?

[rainersigwald]

It does increase risk because it changes more, but I don't think it is a substantial risk. I also don't think you're seeing this because of the change in this PR. I think you're seeing it in this branch because an updated VS was deployed in the pool you're using for CI builds, and that it would affect any PR targeting this branch. As such it's more in the "required infra change to proceed with servicing".

[TanayParikh]

Thanks @rainersigwald for the clarification, the CI is now passing!

I also don't think you're seeing this because of the change in this PR. I think you're seeing it in this branch because an updated VS was deployed in the pool you're using for CI builds, and that it would affect any PR targeting this branch. As such it's more in the "required infra change to proceed with servicing".

Yes, can confirm we were seeing this on the release/3.1 branch prior to this PR as well.

It does increase risk because it changes more, but I don't think it is a substantial risk.

@mkArtakMSFT do we need to go through the servicing approval process again for the 75b7808 additional change? If so, given the nature of the fix (fixes the branch rather than anything specific to this PR) I think it may be beneficial to create a separate PR with just that change (and revert 75b7808 in this PR).

[mkArtakMSFT]

@wtgodbe can you please make sure this is picked up 3.1 June release?

[wtgodbe]

Will do