Repository: syncthing/syncthing
File: lib/connections/quic_listen.go (Go)

Code before:
	if uri != nil && (existingAddress == nil || existingAddress.String() != uri.String()) {
		l.Infof("%s resolved external address %s (via %s)", t.uri, uri.String(), via)
		t.notifyAddressesChanged(t)
	} else if uri == nil && existingAddress != nil {
		t.notifyAddressesChanged(t)
	}
}

func (t *quicListener) serve(ctx context.Context) error {
	network := strings.ReplaceAll(t.uri.Scheme, "quic", "udp")

	udpAddr, err := net.ResolveUDPAddr(network, t.uri.Host)
	if err != nil {
		l.Infoln("Listen (BEP/quic):", err)
		return err
	}

	udpConn, err := net.ListenUDP(network, udpAddr)
	if err != nil {
		l.Infoln("Listen (BEP/quic):", err)
		return err
	}
	defer func() { _ = udpConn.Close() }()

	svc, conn := stun.New(t.cfg, t, udpConn)
	defer func() { _ = conn.Close() }()
	wrapped := &stunConnQUICWrapper{
		PacketConn: conn,
		underlying: udpConn,
	}

	go svc.Serve(ctx)

	registry.Register(t.uri.Scheme, wrapped)
	defer registry.Unregister(t.uri.Scheme, wrapped)

	listener, err := quic.Listen(wrapped, t.tlsCfg, quicConfig)
	if err != nil {
		l.Infoln("Listen (BEP/quic):", err)
		return err
	}
	t.notifyAddressesChanged(t)
	defer listener.Close()
	defer t.clearAddresses(t)

	l.Infof("QUIC listener (%v) starting", udpConn.LocalAddr())
	defer l.Infof("QUIC listener (%v) shutting down", udpConn.LocalAddr())

	acceptFailures := 0
	const maxAcceptFailures = 10


Review comment (suggestion):
I'm really confused about how this thing works. We use pfilter to separate out stun packages from other data. `stun.New` returns the "other data" connection which we call `conn` here and then never use again, except for the deferred close. Instead the `stunQUICWrapper` wraps the underlying UDP conn directly. This should result in two readers of that underlying conn: STUN via pfilter, and QUIC itself. It seems random which packets would end up where...

Code after:

	if uri != nil && (existingAddress == nil || existingAddress.String() != uri.String()) {
		l.Infof("%s resolved external address %s (via %s)", t.uri, uri.String(), via)
		t.notifyAddressesChanged(t)
	} else if uri == nil && existingAddress != nil {
		t.notifyAddressesChanged(t)
	}
}

func (t *quicListener) serve(ctx context.Context) error {
	network := strings.ReplaceAll(t.uri.Scheme, "quic", "udp")

	udpAddr, err := net.ResolveUDPAddr(network, t.uri.Host)
	if err != nil {
		l.Infoln("Listen (BEP/quic):", err)
		return err
	}

	udpConn, err := net.ListenUDP(network, udpAddr)
	if err != nil {
		l.Infoln("Listen (BEP/quic):", err)
		return err
	}
	defer func() { _ = udpConn.Close() }()

	svc, conn := stun.New(t.cfg, t, udpConn)
	defer conn.Close()

	quicWrapper := quicWrapper{
		PacketConn: conn,
		underlying: udpConn,
	}
	var wrapped net.PacketConn = &quicWrapper

	if oobConn, ok := conn.(oobConn); ok {
		l.Debugf("wrapping in oob conn")
		wrapped = &oobConnWrapper{
			quicWrapper, oobConn,
		}
	}

	go svc.Serve(ctx)

	registry.Register(t.uri.Scheme, wrapped)
	defer registry.Unregister(t.uri.Scheme, wrapped)

	listener, err := quic.Listen(wrapped, t.tlsCfg, quicConfig)
	if err != nil {
		l.Infoln("Listen (BEP/quic):", err)
		return err
	}