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

encoding/xml: Add (*Encoder).Close() to check for unclosed elements #53346

Closed
Merovius opened this issue Jun 13, 2022 · 7 comments
Closed

encoding/xml: Add (*Encoder).Close() to check for unclosed elements #53346

Merovius opened this issue Jun 13, 2022 · 7 comments

Comments

@Merovius
Copy link
Contributor

When using (*xml.Encoder).EncodeToken, the Encoder checks for mismatched elements. That is, it returns an error if e.g. a <a> element is closed with a </b> token or if there is an extra </c> token after all elements have already been closed.

However, if the user simply forgets to close all elements, the Encoder emits incorrect XML without reporting an error. See this code, for an example. So currently, the user must track the tokens they encode themselves, to make sure the emitted XML is valid.

Flush returns an error, but as it is possible to encode more tokens after Flush has been called, it can't complain about unclosed elements itself.

I thus propose to add a new method to Encoder:

// Close the Encoder, indicating that no more data will be written. It flushes any buffered XML to the underlying writer
// and returns an error if the written XML is invalid (e.g. by containing unclosed elements).
// After Close has been called, any Encode method return errors.
func (enc *Encoder) Close() error

Naming the method Close is an indication that it should be called and the returned error should be checked by the user.

@gopherbot gopherbot added this to the Proposal milestone Jun 13, 2022
@ianlancetaylor ianlancetaylor added this to Incoming in Proposals (old) Jun 13, 2022
@rsc rsc moved this from Incoming to Active in Proposals (old) Jun 15, 2022
@rsc
Copy link
Contributor

rsc commented Jun 15, 2022

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rsc
Copy link
Contributor

rsc commented Jun 22, 2022

Overall this seems simple and reasonable to me.
I assume Close would also Flush, so that you can just call Close.

@Merovius
Copy link
Contributor Author

I assume Close would also Flush, so that you can just call Close.

Yes, that's definitely the intention.

@rsc
Copy link
Contributor

rsc commented Jul 1, 2022

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

@rsc rsc moved this from Active to Likely Accept in Proposals (old) Jul 1, 2022
@rsc rsc moved this from Likely Accept to Accepted in Proposals (old) Jul 13, 2022
@rsc
Copy link
Contributor

rsc commented Jul 13, 2022

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group

@rsc rsc changed the title proposal: encoding/xml: Add (*Encoder).Close() to check for unclosed elements encoding/xml: Add (*Encoder).Close() to check for unclosed elements Jul 13, 2022
@rsc rsc modified the milestones: Proposal, Backlog Jul 13, 2022
@Merovius
Copy link
Contributor Author

@rsc I'll try to send a CL, unless you tell me not to.

@gopherbot
Copy link

Change https://go.dev/cl/424777 mentions this issue: encoding/xml: add (*Encoder).Close

@golang golang locked and limited conversation to collaborators Aug 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
Development

No branches or pull requests

3 participants