Skip to content

fix: Token.CharData should check underlying event type#14

Merged
orisano merged 1 commit into
orisano:mainfrom
costela:fix-token-chardata
Aug 20, 2025
Merged

fix: Token.CharData should check underlying event type#14
orisano merged 1 commit into
orisano:mainfrom
costela:fix-token-chardata

Conversation

@costela
Copy link
Copy Markdown
Contributor

@costela costela commented Aug 16, 2025

Checking the type of the token compiles and partially works because both event types and token types share a type and gosax.EventText == xmllb.CharData.

Fixes #15

Checking the type of the token compiles and partially works because both event types and token types share a type and gosax.EventText == xmllb.CharData.
@orisano
Copy link
Copy Markdown
Owner

orisano commented Aug 16, 2025

Could you describe the specific cases where you’re running into problems?

@costela
Copy link
Copy Markdown
Contributor Author

costela commented Aug 16, 2025

Sure! Just using the example code can lead to this:

tok, err := d.Token()
//...
switch tok.Type() {
/...
case xmlb.CharData: // ⚠️ this is also the case for gosax.EventCData
	t, _ := tok.CharData()
	fmt.Println("CharData", string(t)) // 💥 this prints <![CDATA...
}

The code above prints the <![CDATA content, even though CharData tries to remove it, because the gosax.EventCData case in that method can never be true (unless you're calling tok.CharData on a Token with Type() == ProcInst, since then - by chance - xmlb.ProcInst == gosax.EventCData).

@costela
Copy link
Copy Markdown
Contributor Author

costela commented Aug 16, 2025

I can create an issue if you want to have a clearer trail for future reference.

@orisano
Copy link
Copy Markdown
Owner

orisano commented Aug 16, 2025

I can create an issue if you want to have a clearer trail for future reference.

pls!

@costela
Copy link
Copy Markdown
Contributor Author

costela commented Aug 19, 2025

done: #15

@orisano
Copy link
Copy Markdown
Owner

orisano commented Aug 20, 2025

thanks!

@orisano orisano merged commit e6a3812 into orisano:main Aug 20, 2025
@costela
Copy link
Copy Markdown
Contributor Author

costela commented Aug 20, 2025

Thanks for merging! 🙇🏻
Any chance we can get a new tag with the fix?

@orisano
Copy link
Copy Markdown
Owner

orisano commented Aug 20, 2025

https://github.com/orisano/gosax/releases/tag/v1.1.4 released!

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.

xmlb.Token.CharData does not remove CDATA markers

2 participants