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 · 5 comments
Open

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

lysShub opened this issue Feb 20, 2024 · 5 comments
Labels
stale-issue This issue has not been updated in 120 days. 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!

Copy link

A friendly reminder that this issue had no activity for 120 days.

@github-actions github-actions bot added the stale-issue This issue has not been updated in 120 days. label Jul 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale-issue This issue has not been updated in 120 days. type: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants