Skip to content

Conversation

pranavkm
Copy link
Contributor

@pranavkm pranavkm commented Nov 10, 2021

As part of .NET 6, we made some tweaks to the razor parser to avoid string allocations. This change
results in a null-ref for bound properties without public setters. The parser change affects building
apps going back all the way to .NET 2.1, so we'd like to fix this.

Fixes #38194


Description / Customer Impact

Attempting to build an app that references certain kinds of tag helpers[1] in cshtml files results in a null reference exception in the Razor parser. With the 6.0 SDK installed, this causes the build to fail in < 6.0 apps and for the source generator to not add files in 6.0.

[1] - TagHelpers with IDictionary properties that do not have setters.

Regression

[x] Yes
[ ] No

Regression introduced in .NET 6 preview6

Testing

[x] Automated (added additional unit tests for this)
[x] Manual testing (patched the SDK and verified the fix)

Risk

  • High
  • Medium
  • Low

The change is primarily limited to build, not part of the shared fx. The null-ref happens with somewhat uncommon inputs so we have confidence this change addresses it.

…null. (dotnet#38264)

As part of .NET 6, we made some  tweaks to the razor parser to avoid string allocations. This change
results in a null-ref for bound properties without public setters. The parser change affects building
apps going back all the way to .NET 2.1, so we'd like to fix this.

Fixes dotnet#38194
@pranavkm pranavkm requested a review from Pilchie as a code owner November 10, 2021 22:13
@ghost ghost added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Nov 10, 2021
@ghost ghost added this to the 6.0.x milestone Nov 10, 2021
@ghost
Copy link

ghost commented Nov 10, 2021

Hi @pranavkm. If this is not a tell-mode PR, please make sure to follow the instructions laid out in the servicing process document.
Otherwise, please add tell-mode label.

@pranavkm pranavkm requested a review from a team November 10, 2021 22:13
@mkArtakMSFT mkArtakMSFT added area-signalr Includes: SignalR clients and servers Servicing-consider Shiproom approval is required for the issue labels Nov 10, 2021
@ghost
Copy link

ghost commented Nov 10, 2021

Hi @pranavkm. Please make sure you've updated the PR description to use the Shiproom Template. Also, make sure this PR is not marked as a draft and is ready-to-merge.

To learn more about how to prepare a servicing PR click here.

@mkArtakMSFT mkArtakMSFT removed the area-signalr Includes: SignalR clients and servers label Nov 10, 2021
@danmoseley danmoseley added Servicing-approved Shiproom has approved the issue and removed Servicing-consider Shiproom approval is required for the issue labels Nov 11, 2021
@danmoseley danmoseley modified the milestones: 6.0.x, 6.0.1 Nov 11, 2021
@pranavkm
Copy link
Contributor Author

@dotnet/aspnet-build this was approved for servicing. Could you merge this?

@wtgodbe wtgodbe merged commit 3147c11 into dotnet:release/6.0 Nov 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates Servicing-approved Shiproom has approved the issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants