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: make sure Encoder.Encode reports Write errors #4112

Closed
gopherbot opened this Issue Sep 20, 2012 · 7 comments

Comments

Projects
None yet
4 participants
@gopherbot
Copy link

gopherbot commented Sep 20, 2012

by justin@fathomdb.com:

encoding.xml Escape() neither checks nor returns the error code from calling io.Writer
write.

Code for encoding.xml Escape is currently here:
http://golang.org/src/pkg/encoding/xml/xml.go?s=37968:38002#L1641
@rsc

This comment has been minimized.

Copy link
Contributor

rsc commented Sep 20, 2012

Comment 1:

We can't change the signature due to the API compatibility promise. We
could introduce a second function. Note that the usual use of this is
to write to a bytes.Buffer, and you can poll that after the fact for a
write error by using buf.Write(nil).

Labels changed: added priority-later, go1.1, removed priority-triage.

Status changed to Accepted.

@rsc

This comment has been minimized.

Copy link
Contributor

rsc commented Dec 9, 2012

Comment 3:

Let's leave Escape as it is (we have to), but make sure that Encoder.Encode returns I/O
errors. Clients that need to check I/O errors can either write to a bufio.Writer and use
buf.Write(nil) or else use NewEncoder+Encode and check that error result.
@rsc

This comment has been minimized.

Copy link
Contributor

rsc commented Dec 9, 2012

Comment 4:

I missed that this was about Escape, not Marshal. 
1. Add tests that Marshal and Encoder.Encode return I/O errors.
2. Add EscapeText that is like Escape but returns error.
3. Make Escape the obvious wrapper - still dropping error.
4. Make Escape description:
// Escape is like EscapeText but omits the error return value.
// It is provided for backwards compatibility with Go 1.0.
// Code targeting Go 1.1 or later should use EscapeText.
@rsc

This comment has been minimized.

Copy link
Contributor

rsc commented Dec 10, 2012

Comment 5:

Labels changed: added size-m.

@rsc

This comment has been minimized.

Copy link
Contributor

rsc commented Dec 10, 2012

Comment 6:

Labels changed: added suggested.

@davecheney

This comment has been minimized.

Copy link
Contributor

davecheney commented Jan 14, 2013

Comment 7:

Started by osaingre@gmail.com
https://golang.org/cl/7085053/

Status changed to Started.

@bradfitz

This comment has been minimized.

Copy link
Member

bradfitz commented Feb 20, 2013

Comment 8:

This issue was closed by revision afde71c.

Status changed to Fixed.

@rsc rsc added this to the Go1.1 milestone Apr 14, 2015

@rsc rsc removed the go1.1 label Apr 14, 2015

@golang golang locked and limited conversation to collaborators Jun 24, 2016

This issue was closed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.