Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix VS crash with malformed tag helpers #26779

Merged
merged 1 commit into from
Oct 12, 2020
Merged

Conversation

ajaybhargavb
Copy link
Contributor

@ajaybhargavb ajaybhargavb commented Oct 10, 2020

Fixes https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1226741

Example,

<a @onclick="...">
@if (true) {
    </a> <----
}
</a>

Consider the above example. In this case, the parser groups the start/end tags correctly (i.e. the </a> in the middle has no start tag).
But in the tag helper rewriting phase, we end up incorrectly identifying the middle </a> as the end tag for the start tag and pop the stack causing it to throw when it reaches the final </a> tag.
The fix: In addition to checking if the tag name matches, pop the stack only when there is a corresponding start tag available.

@ghost ghost added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Oct 10, 2020
{
return false;
}

// Validate that our end tag matches the currently scoped tag, if not we may need to error.
if (tagNameScope.Equals(tagName, StringComparison.OrdinalIgnoreCase))
if (startTag != null && tagNameScope.Equals(tagName, StringComparison.OrdinalIgnoreCase))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the actual fix. Everything else is just a bunch of renames.

Copy link
Contributor

@NTaylorMullen NTaylorMullen left a comment

Choose a reason for hiding this comment

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

Good catch!

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants