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

make RenderAttributes() accept both []byte and string #448

Merged
merged 1 commit into from
Apr 3, 2024

Conversation

movsb
Copy link
Contributor

@movsb movsb commented Apr 2, 2024

The problem:

Since the Value of an Attribute is defined as of type interface{}, I may think that it accepts all usual types (such as []byte, string, and even numbers).

The problem is that when I SetAttributeString(`target`, `_blank`) of a Link, the RenderAttributes function force the Value to be []byte, which results in runtime panic.

_, _ = w.Write(util.EscapeHTML(attr.Value.([]byte)))

The solution:

I switch cased the type and rewritten the code as below:

		var value []byte
		switch typed := attr.Value.(type) {
		case []byte:
			value = typed
		case string:
			value = util.StringToReadOnlyBytes(typed)
		}
		_, _ = w.Write(util.EscapeHTML(value))

I cannot assume that people don't force the Value to be of type []byte, or that they already know that, and while SetAttributeString, use []byte. May god bless all those who switch case Node::AttributeString()'s return value.

@movsb movsb changed the title make SetAttributeString() accept both []byte and string make RenderAttributes() accept both []byte and string Apr 2, 2024
@yuin
Copy link
Owner

yuin commented Apr 3, 2024

goldmark is designed to be able to support various output formats by custom renderer implementations, so attribute values designed to be able to have any kind of values.

In HTML outputs, attribute values should be rendered as a string.

@yuin yuin merged commit c15e394 into yuin:master Apr 3, 2024
6 of 7 checks passed
@movsb movsb deleted the fix-attribute-string branch April 3, 2024 10:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants