Skip to content

/ go

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

Jump to bottom

crypto/tls: Conn missing CloseRead / CloseWrite #8579

Closed
gopherbot opened this issue Aug 23, 2014 · 9 comments
Closed

crypto/tls: Conn missing CloseRead / CloseWrite #8579

gopherbot opened this issue Aug 23, 2014 · 9 comments
Labels
FrozenDueToAge NeedsFix
Milestone
Go1.8

Comments

@gopherbot
Copy link

@gopherbot gopherbot commented Aug 23, 2014

by justin@fathomdb.com:

CloseRead / CloseWrite functions are missing from tls.Conn.

Yet, this is the recommended way to proxy between TCP connections:
http://grokbase.com/t/gg/golang-nuts/131k81bse0/go-nuts-use-of-closed-network-connection-error-from-tcpconn-read

They should probably be defined on net.Conn
@bradfitz

This comment has been minimized.

Sign in to view
Copy link
Contributor

@bradfitz bradfitz commented Aug 24, 2014

Comment 1:

Labels changed: added repo-main.

Owner changed to @agl.

Status changed to Accepted.
@mikioh

This comment has been minimized.

Sign in to view
Copy link
Contributor

@mikioh mikioh commented Aug 25, 2014

Comment 2:

> They should probably be defined on net.Conn
unfortunately we can't break the go1 contract; http://golang.org/doc/go1compat/.
@rsc

This comment has been minimized.

Sign in to view
Copy link
Contributor

@rsc rsc commented Apr 10, 2015

I don't think this will happen. The input to TLS is a net.Conn, and we can't add them there. crypto/tls would have to sniff for them, and what if the underlying net.Conn doesn't support them?
@rsc rsc closed this Apr 10, 2015
@cpuguy83

This comment has been minimized.

Sign in to view
Copy link

@cpuguy83 cpuguy83 commented Nov 29, 2015

@rsc It may be helpful here to be able to get the underlying net.Conn?
We currently have our own copy of tls.Dial which we use to store the underlying conn and provide our own CloseWrite method.
@cpuguy83 cpuguy83 mentioned this issue Nov 29, 2015
Merged
@gopherbot

This comment has been minimized.

Sign in to view
Copy link
Author

@gopherbot gopherbot commented Jul 24, 2016

CL https://golang.org/cl/25159 mentions this issue.
gopherbot pushed a commit that referenced this issue Oct 17, 2016
@benburkert @bradfitz
crypto/tls: add CloseWrite method to Conn
c6185aa 
The CloseWrite method sends a close_notify alert record to the other
side of the connection. This record indicates that the sender has
finished sending on the connection. Unlike the Close method, the sender
may still read from the connection until it recieves a close_notify
record (or the underlying connection is closed). This is analogous to a
TCP half-close.

Updates #8579

Change-Id: I9c6bc193efcb25cc187f7735ee07170afa7fdde3
Reviewed-on: https://go-review.googlesource.com/25159
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@gopherbot

This comment has been minimized.

Sign in to view
Copy link
Author

@gopherbot gopherbot commented Oct 18, 2016

CL https://golang.org/cl/31318 mentions this issue.
@mikioh

This comment has been minimized.

Sign in to view
Copy link
Contributor

@mikioh mikioh commented Oct 19, 2016

Reopening, CL 25159 has been reverted.
@mikioh mikioh reopened this Oct 19, 2016
@quentinmit quentinmit added the NeedsFix label Oct 20, 2016
@quentinmit quentinmit added this to the Go1.8 milestone Oct 20, 2016
gopherbot pushed a commit that referenced this issue Oct 26, 2016
@benburkert @bradfitz
crypto/tls: add CloseWrite method to Conn
829aa67 
The CloseWrite method sends a close_notify alert record to the other
side of the connection. This record indicates that the sender has
finished sending on the connection. Unlike the Close method, the sender
may still read from the connection until it recieves a close_notify
record (or the underlying connection is closed). This is analogous to a
TCP half-close.

This is a rework of CL 25159 with fixes for the unstable test.

Updates #8579

Change-Id: I47608d2f82a88baff07a90fd64c280ed16a60d5e
Reviewed-on: https://go-review.googlesource.com/31318
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@rsc

This comment has been minimized.

Sign in to view
Copy link
Contributor

@rsc rsc commented Nov 3, 2016

Looks fixed to me (CloseRead doesn't make sense per discussion on CL 25159).

$ go doc tls.CloseWrite
func (c *Conn) CloseWrite() error
    CloseWrite shuts down the writing side of the connection. It should only be
    called once the handshake has completed and does not call CloseWrite on the
    underlying connection. Most callers should just use Close.
@cpuguy83

This comment has been minimized.

Sign in to view
Copy link

@cpuguy83 cpuguy83 commented Nov 3, 2016

Yep +1. Really happy to have CloseWrite in 1.8
@rsc rsc closed this Nov 3, 2016
@golang golang locked and limited conversation to collaborators Nov 3, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix
Projects
None yet
Milestone
  Go1.8
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants
@gopherbot @bradfitz @mikioh @rsc @cpuguy83 @quentinmit
You can’t perform that action at this time.