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

gonet can write a empty data to a closed conn #10023

Open
lysShub opened this issue Feb 20, 2024 · 4 comments
Open

gonet can write a empty data to a closed conn #10023

lysShub opened this issue Feb 20, 2024 · 4 comments
Labels
type: bug Something isn't working

Comments

@lysShub
Copy link

lysShub commented Feb 20, 2024

Description

package main_test

import (
	"context"
	"io"
	"net"
	"testing"

	// gvisor.dev/gvisor v0.0.0-20240216000150-e3cf008ab186
	"github.com/stretchr/testify/require"
	"gvisor.dev/gvisor/pkg/tcpip"
	"gvisor.dev/gvisor/pkg/tcpip/adapters/gonet"
	"gvisor.dev/gvisor/pkg/tcpip/header"
	"gvisor.dev/gvisor/pkg/tcpip/link/channel"
	"gvisor.dev/gvisor/pkg/tcpip/network/ipv4"
	"gvisor.dev/gvisor/pkg/tcpip/stack"
	"gvisor.dev/gvisor/pkg/tcpip/transport/tcp"
)

func Test_Closed_TCP_Write_Empty(t *testing.T) {
	t.Run("system", func(t *testing.T) {
		var addr = &net.TCPAddr{IP: []byte{127, 0, 0, 1}, Port: 8080}
		go func() {
			l, err := net.ListenTCP("tcp", addr)
			require.NoError(t, err)
			defer l.Close()
			conn, err := l.AcceptTCP()
			require.NoError(t, err)

			io.Copy(conn, conn)
		}()
		time.Sleep(time.Second)

		conn, err := net.DialTCP("tcp", nil, addr)
		require.NoError(t, err)
		err = conn.Close()
		require.NoError(t, err)

		n, err := conn.Write([]byte{})
		require.Error(t, err)
		require.Zero(t, n)
	})

	t.Run("gvisor", func(t *testing.T) {
		var addr = tcpip.AddrFromSlice([]byte{10, 0, 0, 1})
		var st = stack.New(stack.Options{
			NetworkProtocols:   []stack.NetworkProtocolFactory{ipv4.NewProtocol},
			TransportProtocols: []stack.TransportProtocolFactory{tcp.NewProtocol},
			HandleLocal:        true,
		})

		const nicid tcpip.NICID = 1234
		e := st.CreateNIC(nicid, channel.New(4, 1536, ""))
		require.Nil(t, e)
		st.AddProtocolAddress(nicid, tcpip.ProtocolAddress{
			Protocol:          header.IPv4ProtocolNumber,
			AddressWithPrefix: addr.WithPrefix(),
		}, stack.AddressProperties{})
		st.SetRouteTable([]tcpip.Route{{Destination: header.IPv4EmptySubnet, NIC: nicid}})

		go func() {
			l, err := gonet.ListenTCP(
				st,
				tcpip.FullAddress{Addr: addr, Port: 8080},
				header.IPv4ProtocolNumber,
			)
			require.NoError(t, err)
			defer l.Close()

			conn, err := l.Accept()
			require.NoError(t, err)
			io.Copy(conn, conn)
		}()
		time.Sleep(time.Second)

		conn, err := gonet.DialTCPWithBind(
			context.Background(), st,
			tcpip.FullAddress{Addr: addr, Port: 12345},
			tcpip.FullAddress{Addr: addr, Port: 8080},
			header.IPv4ProtocolNumber,
		)
		require.NoError(t, err)
		err = conn.Close()
		require.NoError(t, err)

		n, err := conn.Write([]byte{})
		require.Error(t, err)
		require.Zero(t, n)
	})
}

Steps to reproduce

No response

runsc version

No response

docker version (if using docker)

No response

uname

No response

kubectl (if using Kubernetes)

No response

repo state (if built from source)

No response

runsc debug logs (if available)

No response

@lysShub lysShub added the type: bug Something isn't working label Feb 20, 2024
@lysShub
Copy link
Author

lysShub commented Feb 20, 2024

system test passed on windows and linux

@kevinGC
Copy link
Collaborator

kevinGC commented Feb 20, 2024

Can you help describe the problem in more detail? Is this contradicted in a doc somewhere, or does this differ from other implementers of net.Conn?

@lysShub
Copy link
Author

lysShub commented Feb 21, 2024

Can you help describe the problem in more detail? Is this contradicted in a doc somewhere, or does this differ from other implementers of net.Conn?

In most operating systems, writing to a closed TCP connection will return an error, including conn.Write([]byte{}),but gonet will return 0 and nil

@kevinGC
Copy link
Collaborator

kevinGC commented Mar 13, 2024

We'd definitely accept a PR with this change!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants