dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.43k stars 10.02k forks source link

Invalid html markup string throws unhandled exception in Blazor webassembly and Maui #52888

Closed zachneu closed 10 months ago

zachneu commented 10 months ago

Is there an existing issue for this?

Describe the bug

If i create a new .net 8 Blazor project (web assembly or in Maui project) and add this code in any razor component @((MarkupString)@"<div style=""background-image: url('data;border-bottom: 1px;');""/>") It will result in unhandled exception in blazor webassembly and crashing the Maui application

image

Expected Behavior

Should just render the invalid html code

Steps To Reproduce

  1. Create a new "Blazor Web App" project
  2. Select .NET 8 as the framework
  3. Select WebAssembly as interactive render mode
  4. image

  5. Add this code to Counter.razor page @((MarkupString)@"<div style=""background-image: url('data;border-bottom: 1px;');""/>")
  6. Start the app and navigate to the Counter page. This will now throw an exception in the console. For Maui app this will crash the whole app.

Exceptions (if any)

In Blazor WASM blazor.web.js:1 Uncaught (in promise) Error: Found malformed component comment at Blazor:{"type":"webassembly","prerenderId":"82e3e05fbf2343fcb19d1cdd0bbd38f6","key":{"locationHash":"F2CF437FFE53A709788AFA6A2CA2B22589586E68:0","formattedComponentKey":""},"assembly":"BlazorApp2.Client","typeName":"BlazorApp2.Client.Pages.Counter","parameterDefinitions":"W10=","parameterValues":"W10="} at Dt (blazor.web.js:1:42661) at Tt (blazor.web.js:1:41241) at Tt (blazor.web.js:1:41314) at Tt (blazor.web.js:1:41314) at Tt (blazor.web.js:1:41314) at Tt (blazor.web.js:1:41314) at Tt (blazor.web.js:1:41314) at blazor.web.js:1:40504 at _t (blazor.web.js:1:40556) at Ci (blazor.web.js:1:171459)

In MAUI The parameter is incorrect.

An item cannot be found with the specified name (wwwroot.\data;border-bottom:%201px%20solid%20transparent;background-color:%20inherit;).

.NET Version

8

Anything else?

No response

ghost commented 10 months ago

Thank you for filing this issue. In order for us to investigate this issue, please provide a minimal repro project that illustrates the problem without unnecessary code. Please share with us in a public GitHub repo because we cannot open ZIP attachments, and don't include any confidential content.

MariovanZeist commented 10 months ago

Hi @zachneu You are getting the error because the markup you generate is a self-closing <div/> tag, which is not permitted in HTML.

Adding@((MarkupString)"<div/>") to the razor file will result in the same error. If you add <div/> to a razor file, the razor compiler will change it to <div></div>, but it won't do the same for a MarkupString.

If you change your markupstring to @((MarkupString)@"<div style=""background-image: url('data;border-bottom: 1px;');""></div>") the page will load without problem.

ghost commented 10 months ago

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment. If it is closed, feel free to comment when you are able to provide the additional information and we will re-investigate.

See our Issue Management Policies for more information.

tariqalsoahmed commented 9 months ago

I get this same error when calling (MarkupString)item.DescriptionShortened(ShortDescLength) in a loop.

Seems the MarkupString is randomly injecting a "b" tag into the results then rightly deciding the HTML is invalid (screenshot below)

image

tariqalsoahmed commented 9 months ago

P.S. You can clearly see comparing "View Source" (left) to "DOM Inspection" (right) ... Server Pre-rendering never emitted a "b" tag ... The "b" tag was only injected from the client-side MarkupString

image

Oddly enough the only actual call to MarkupString is nested inside the div ... so not sure why the "b" tag is injected outside the div ??? But whatever the reason, when line 55 is commented and 56 is uncommented the problem goes away... Appears to definitely be a bug with MarkupString far as i can troubleshoot this one.

image

MariovanZeist commented 9 months ago

@tariqalsoahmed Would it be possible for you to create a small repro of this issue?

tariqalsoahmed commented 9 months ago

I can't reproduce in a repo. I'm happy to jump on a call and walk through the issue with you. Below is the error being generated.

blazor.webassembly.js:1 Error: Found malformed component comment at Blazor:{"type":"webassembly","prerenderId":"da28410f78794bd18c6f30a6c37bec3a","key":{"locationHash":"4EE4D0430FFCBB1B4A06FB2BEE9F499B53DA581B:0","formattedComponentKey":""},"assembly":"Web.Client","typeName":"Web.Client.App","parameterDefinitions":"W10=","parameterValues":"W10="}

The error does not occur if I simply remove MarkupString. That's what I'm doing as a temporary workaround :/

MariovanZeist commented 9 months ago

@tariqalsoahmed Can you post the string that's returned from item.DescriptionShortened(ShortDescLength) here?

tariqalsoahmed commented 9 months ago

Thanks @MariovanZeist seems you found the problem! Results attached MarkupStringProblem.txt

Calling MarkupString in a loop and truncating descriptions for brevity (less data downloaded). Because truncating is "fixed" string length, the descriptions are cutoff before HTML element closing tag. Apparently the "unclosed" elements are confusing MarkupString.

We don't want to render the entire description because its a VARCHAR(MAX) in the DB and could be way to much data to get rendered in a loop :(

Thoughts?

MariovanZeist commented 9 months ago

@tariqalsoahmed As you noticed a MarkupString must include valid HTML otherwise an error will be generated.

I have some personal recommendations which are my own. In general, I would advise against marking any string as MarkupString unless you are very sure it contains valid HTML. So to that extent, I would never pass any data from an external source (e.g. database or any other method, like user input or text file) as MarkupString for Blazor to render.

A solution could be to add a specific varchar column with a specified length e.g. VARCHAR(100) in your database that contains a summary. And render that as a normal string (not as a MarkupString).

tariqalsoahmed commented 9 months ago

Hi @MariovanZeist I just wrote an extension method to strip HTML tags and just render the raw string values. Full details on stack overflow https://stackoverflow.com/a/77834840/9841182

Thanks for your help!