Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

TagBuilder fail with NPE if attribute have null value #4710

Closed
justdmitry opened this issue May 22, 2016 · 7 comments
Closed

TagBuilder fail with NPE if attribute have null value #4710

justdmitry opened this issue May 22, 2016 · 7 comments
Labels
3 - Done bug up-for-grabs Members of our awesome commnity can handle this issue
Milestone
1.1.0-preview1

Comments

@justdmitry
Copy link

justdmitry commented May 22, 2016 •
edited by Eilon
Loading

Sample .cshtml code:

<html><body>
    @{
        var tb = new TagBuilder("span");
        tb.Attributes.Add("dummy", null);
        tb.InnerHtml.Append("Hello, World!");
    }
    @tb
</body></html>

This tag builder will fail with NullPointerException while rendering with stacktrace:

System.NullReferenceException: Object reference not set to an instance of an object.
   at Microsoft.AspNetCore.Mvc.Rendering.TagBuilder.AppendAttributes(TextWriter writer, HtmlEncoder encoder)
   at Microsoft.AspNetCore.Mvc.Rendering.TagBuilder.WriteTo(TextWriter writer, HtmlEncoder encoder)
   at Microsoft.AspNetCore.Mvc.ViewFeatures.Internal.ViewBuffer.<WriteToAsync>d__18.MoveNext()

Tested on Microsoft.AspNetCore.Mvc 1.0.0-rc2-final.

Investigation:

Line 208 of TagBuilder.AppendAttributes calls for HtmlEncoder.Encode(writer, attribute.Value).

But TextEncoder.Encode(TextWriter output, string value) is overload, requesting value.Length.
@Eilon Eilon added bug 1 - Ready up-for-grabs Members of our awesome commnity can handle this issue labels May 23, 2016
@Eilon Eilon added this to the 1.0.1 milestone May 23, 2016
@Eilon
Copy link
Member

Eilon commented May 23, 2016

Good find! The workaround seems like it would be to just pass in string.Empty or "" in the meantime.

@ctyar
Copy link
Contributor

ctyar commented May 23, 2016

if these behaviors are ok, I can send a PR

tagBuilder.Attributes.Add("attribute", "value")
<p attribute="HtmlEncode[[value]]"></p>

tagBuilder.Attributes.Add("attribute", null)
<p attribute></p>

tagBuilder.Attributes.Add("attribute", string.Empty)
<p attribute=""></p>

@justdmitry
Copy link
Author

justdmitry commented May 23, 2016 •
edited
Loading

Issue #4078 says:

In any case TagBuilder does not support minimized attributes, sorts the generated attributes alphabetically by name, and more.

Is this (not support minimized attributes) "by design" or not?

@dougbu
Copy link
Member

dougbu commented May 23, 2016

Is this (not support minimized attributes) "by design" or not?

All of the quoted points are by design. TagBuilder is intended for C# helpers that create entire HTML elements. And it has generated XHTML-compliant syntax forever.

tagBuilder.Attributes.Add("attribute", null)
<p attribute></p>

This is incorrect and not aligned with the rest of the TagBuilder class. A minimized attribute is shorthand for attribute="attribute" and not attribute="@null". The generated HTML in this case should be identical to that for tagBuilder.Attributes.Add("attribute", string.Empty).

@Eilon
Copy link
Member

Eilon commented May 23, 2016

@ctyar yes that behavior is by design.

I think a point of comparison is to see what ASP.NET MVC 5.x did, and replicate that behavior.

@Eilon
Copy link
Member

Eilon commented May 23, 2016

Looks like in MVC 5.x a null was treated the same as "", which is to render an empty quoted value.

I.e. this:

    var x = new TagBuilder("myTag");
    x.Attributes["x"] = "v";
    x.Attributes["y"] = "";
    x.Attributes["z"] = null;

Renders this:

<myTag x="v" y="" z=""></myTag>

So let's preserve that behavior.

@ctyar ctyar mentioned this issue May 23, 2016
Closed
pranavkm pushed a commit that referenced this issue May 26, 2016
@ctyar @pranavkm
Fixed a bug in TagBuilder when attribute value is null
42dd449 
 #4710
@pranavkm
Copy link
Contributor

Fixed in 42dd449.

@ctyar ctyar mentioned this issue Jun 2, 2016
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
3 - Done bug up-for-grabs Members of our awesome commnity can handle this issue
Projects
None yet
Milestone
1.1.0-preview1
Development

No branches or pull requests
5 participants
@pranavkm @Eilon @ctyar @justdmitry @dougbu