LVPN-7288: Fix HTTP/3 error detection in long responses

cmd/daemon/main.go

@@ -228,9 +228,10 @@ func main() {
 
 	resolver := network.NewResolver(fw, threatProtectionLiteServers)
 
-	if err := kernel.SetParameter(netCoreRmemMaxKey, netCodeRmemMaxValue); err != nil {
-		log.Println(internal.WarningPrefix, err)
+	if err := SetBufferSizeForHTTP3(); err != nil {
+		log.Println(internal.WarningPrefix, "failed to set buffer size for HTTP/3:", err)
 	}
+
 	httpClientWithRotator := request.NewStdHTTP()
 	httpClientWithRotator.Transport = createTimedOutTransport(resolver, cfg.FirewallMark, httpCallsSubject, daemonEvents.Service.Connect)
 

cmd/daemon/transports.go

@@ -27,10 +27,23 @@ import (
 
 const (
 	netCoreRmemMaxKey    = "net.core.rmem_max"
-	netCodeRmemMaxValue  = 2500000
+	netCoreWmemMaxKey    = "net.core.wmem_max"
+	netCoreMemMaxValue   = 7500000
 	envHTTPTransportsKey = "HTTP_TRANSPORTS"
 )
 
+// SetBufferSizeForHTTP3 increase receive buffer size to roughly 7.5 MB, as recommended for quic-go library.
+// see: https://github.com/quic-go/quic-go/wiki/UDP-Receive-Buffer-Size
+func SetBufferSizeForHTTP3() error {
+	if err := kernel.SetParameter(netCoreRmemMaxKey, netCoreMemMaxValue); err != nil {
+		return fmt.Errorf("setting receive buffer: %w", err)
+	}
+	if err := kernel.SetParameter(netCoreWmemMaxKey, netCoreMemMaxValue); err != nil {
+		return fmt.Errorf("setting write buffer: %w", err)
+	}
+	return nil
+}
+
 func createH1Transport(resolver network.DNSResolver, fwmark uint32) func() http.RoundTripper {
 	return func() http.RoundTripper {
 		var operr error
@@ -136,34 +149,29 @@ func createTimedOutTransport(
 	containsH1 := slices.Contains(transportTypes, "http1")
 	containsH3 := slices.Contains(transportTypes, "http3")
 
-	var h1Transport http.RoundTripper
-	var h3Transport http.RoundTripper
+	var h1Transport *request.HTTPReTransport
+	var h3Transport *request.QuicTransport
 	if containsH1 {
-		h1ReTransport := request.NewHTTPReTransport(createH1Transport(resolver, fwmark))
-		connectSubject.Subscribe(h1ReTransport.NotifyConnect)
-		h1Transport = request.NewPublishingRoundTripper(
-			h1ReTransport,
-			httpCallsSubject,
-		)
+		h1Transport = request.NewHTTPReTransport(createH1Transport(resolver, fwmark))
+		connectSubject.Subscribe(h1Transport.NotifyConnect)
 		if !containsH3 {
-			return h1Transport
+			return request.NewPublishingRoundTripper(
+				h1Transport,
+				httpCallsSubject,
+			)
 		}
 	}
 	if containsH3 {
-		// For quic-go need to increase receive buffer size
-		// This command will increase the maximum receive buffer size to roughly 2.5 MB
-		// see: https://github.com/quic-go/quic-go/wiki/UDP-Receive-Buffer-Size
-		if err := kernel.SetParameter(netCoreRmemMaxKey, netCodeRmemMaxValue); err != nil {
-			log.Println(internal.WarningPrefix, err)
+		if err := SetBufferSizeForHTTP3(); err != nil {
+			log.Println(internal.WarningPrefix, "failed to set buffer size for HTTP/3:", err)
 		}
-		h3ReTransport := request.NewQuicTransport(createH3Transport)
-		connectSubject.Subscribe(h3ReTransport.NotifyConnect)
-		h3Transport = request.NewPublishingRoundTripper(
-			h3ReTransport,
-			httpCallsSubject,
-		)
+		h3Transport = request.NewQuicTransport(createH3Transport)
+		connectSubject.Subscribe(h3Transport.NotifyConnect)
 		if !containsH1 {
-			return h3Transport
+			return request.NewPublishingRoundTripper(
+				h3Transport,
+				httpCallsSubject,
+			)
 		}
 	}
 	// This should never happen as validation makes sure of that but it is here for nil panics
@@ -173,5 +181,6 @@ func createTimedOutTransport(
 		return nil
 	}
 
-	return request.NewRotatingRoundTripper(h1Transport, h3Transport, time.Hour)
+	rotatingRoundTriper := request.NewRotatingRoundTripper(h1Transport, h3Transport, time.Hour)
+	return request.NewPublishingRoundTripper(rotatingRoundTriper, httpCallsSubject)
 }

core/core.go

@@ -14,6 +14,7 @@ import (
 
 	"github.com/NordSecurity/nordvpn-linux/config"
 	"github.com/NordSecurity/nordvpn-linux/daemon/response"
+	"github.com/NordSecurity/nordvpn-linux/internal"
 	"github.com/NordSecurity/nordvpn-linux/request"
 	"github.com/google/uuid"
 )
@@ -566,8 +567,6 @@ func getData[T any](api *DefaultAPI, token string, url string) (T, error) {
 	return data, nil
 }
 
-const maxBytesLimit int64 = 1024 * 1024 * 10 // 10MB
-
 type ErrMaxBytesLimit struct {
 	Limit int64
 }
@@ -582,7 +581,7 @@ func (err *ErrMaxBytesLimit) Error() string {
 func MaxBytesReadAll(r io.Reader) ([]byte, error) {
 	limitedReader := &io.LimitedReader{
 		R: r,
-		N: maxBytesLimit + 1, // + 1 because we allow for values which are equal to the limit
+		N: internal.MaxBytesLimit,
 	}
 	data, err := io.ReadAll(limitedReader)
 	if err != nil {
@@ -594,7 +593,7 @@ func MaxBytesReadAll(r io.Reader) ([]byte, error) {
 	// limit reached       - limitedReader.N <= 0
 	// io.Reader is empty  - limitedReader.N > 0
 	if limitedReader.N <= 0 {
-		return nil, &ErrMaxBytesLimit{Limit: maxBytesLimit}
+		return nil, &ErrMaxBytesLimit{Limit: internal.MaxBytesLimit}
 	}
 
 	return data, nil

core/core_test.go

@@ -11,6 +11,7 @@ import (
 	"testing"
 
 	"github.com/NordSecurity/nordvpn-linux/daemon/response"
+	"github.com/NordSecurity/nordvpn-linux/internal"
 	"github.com/NordSecurity/nordvpn-linux/test/category"
 	"github.com/google/uuid"
 
@@ -232,9 +233,11 @@ func TestMaxBytes(t *testing.T) {
 		size        int64
 		expectedErr error
 	}{
-		"too big input":               {size: maxBytesLimit + 1, expectedErr: &ErrMaxBytesLimit{Limit: maxBytesLimit}},
-		"input size within limits":    {size: maxBytesLimit - 1, expectedErr: nil},
-		"input size exact limit size": {size: maxBytesLimit, expectedErr: nil},
+		"too big input": {
+			size:        internal.MaxBytesLimit + 1,
+			expectedErr: &ErrMaxBytesLimit{Limit: internal.MaxBytesLimit}},
+		"input size within limits":    {size: internal.MaxBytesLimit - 2, expectedErr: nil},
+		"input size exact limit size": {size: internal.MaxBytesLimit - 1, expectedErr: nil},
 	}
 	for name, test := range tests {
 		t.Run(name, func(t *testing.T) {

internal/constants.go

@@ -97,6 +97,8 @@ const (
 	AllowlistMaxPort = 65535
 
 	NordWhisperInterfaceName = "qtun"
+
+	MaxBytesLimit int64 = 1024*1024*10 + 1 // 10MB + 1 because we allow for values which are equal to the limit
 )
 
 var (

request/rotating.go

@@ -1,9 +1,14 @@
 package request
 
 import (
+	"bytes"
+	"io"
+	"log"
 	"net/http"
 	"sync/atomic"
 	"time"
+
+	"github.com/NordSecurity/nordvpn-linux/internal"
 )
 
 type RotatingRoundTripper struct {
@@ -28,17 +33,41 @@ func NewRotatingRoundTripper(
 	}
 }
 
+func (rt *RotatingRoundTripper) roundTripH3(req *http.Request) (*http.Response, error) {
+	resp, err := rt.roundTripperH3.RoundTrip(req)
+	if err != nil {
+		return nil, err
+	}
+
+	defer resp.Body.Close()
+
+	// HTTP/3 responses are streamed, so error can occur while reading the response body. We need to attempt a read here
+	// so all potential errors can be detected and we can rotate to HTTP/1
+	var buf bytes.Buffer
+	reader := io.LimitReader(resp.Body, internal.MaxBytesLimit)
+	_, err = io.Copy(&buf, reader)
+	if err != nil {
+		return nil, err
+	}
+
+	resp.Body.Close()
+
+	resp.Body = io.NopCloser(&buf)
+	return resp, nil
+}
+
 func (rt *RotatingRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
 	if !rt.isCurrentH3.Load() && time.Now().After(time.UnixMilli(rt.lastH3AttemptMilli.Load()).Add(rt.h3ReviveTime)) {
 		now := time.Now()
 		rt.lastH3AttemptMilli.Store(now.UnixMilli())
 		rt.isCurrentH3.Store(true)
 	}
 	if rt.isCurrentH3.Load() {
-		resp, err := rt.roundTripperH3.RoundTrip(req)
+		resp, err := rt.roundTripH3(req)
 		if err == nil {
 			return resp, err
 		}
+		log.Println(internal.ErrorPrefix, "HTTP/3 request failed:", err, "rotating to HTTP/1")
 		rt.isCurrentH3.Store(false)
 	}
 	return rt.roundTripperH1.RoundTrip(req)

request/rotating_test.go

@@ -2,7 +2,9 @@ package request
 
 import (
 	"fmt"
+	"io"
 	"net/http"
+	"strings"
 	"sync"
 	"sync/atomic"
 	"testing"
@@ -12,21 +14,42 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
+const (
+	h1RespBody = "h1 body"
+	h3RespBody = "h3 body"
+)
+
+type responseTemplate struct {
+	protoMajor int
+	body       string
+}
+
 var (
-	err1   = fmt.Errorf("error1")
-	respH1 = &http.Response{ProtoMajor: 1}
-	respH3 = &http.Response{ProtoMajor: 3}
+	err1           = fmt.Errorf("error1")
+	respH1Template = responseTemplate{
+		protoMajor: 1,
+		body:       h1RespBody,
+	}
+	respH3Template = responseTemplate{
+		protoMajor: 3,
+		body:       h3RespBody,
+	}
 )
 
 type mockRoundTripper struct {
-	duration time.Duration
-	resp     *http.Response
-	err      error
+	duration         time.Duration
+	responseTemplate responseTemplate
+	err              error
 }
 
 func (m mockRoundTripper) RoundTrip(*http.Request) (*http.Response, error) {
 	time.Sleep(m.duration)
-	return m.resp, m.err
+
+	resp := http.Response{
+		ProtoMajor: m.responseTemplate.protoMajor,
+		Body:       io.NopCloser(strings.NewReader(m.responseTemplate.body)),
+	}
+	return &resp, m.err
 }
 
 func newAtomicBool(val bool) *atomic.Bool {
@@ -46,61 +69,61 @@ func TestRotatingRoundTripper_RoundTrip(t *testing.T) {
 	for _, tt := range []struct {
 		name         string
 		roundTripper *RotatingRoundTripper
-		resp         *http.Response
+		resp         responseTemplate
 		err          error
 	}{
 		{
 			name: "h3",
 			roundTripper: NewRotatingRoundTripper(
-				mockRoundTripper{resp: respH1},
-				mockRoundTripper{resp: respH3},
+				mockRoundTripper{responseTemplate: respH1Template},
+				mockRoundTripper{responseTemplate: respH3Template},
 				time.Duration(0),
 			),
-			resp: respH3,
+			resp: respH3Template,
 		},
 		{
 			name: "h3 without switch",
 			roundTripper: &RotatingRoundTripper{
-				roundTripperH1:     mockRoundTripper{resp: respH1},
-				roundTripperH3:     mockRoundTripper{resp: respH3},
+				roundTripperH1:     mockRoundTripper{responseTemplate: respH1Template},
+				roundTripperH3:     mockRoundTripper{responseTemplate: respH3Template},
 				isCurrentH3:        newAtomicBool(true),
 				lastH3AttemptMilli: &atomic.Int64{},
 				h3ReviveTime:       time.Duration(0),
 			},
-			resp: respH3,
+			resp: respH3Template,
 		},
 		{
 			name: "h1",
 			roundTripper: &RotatingRoundTripper{
-				roundTripperH1:     mockRoundTripper{resp: respH1},
-				roundTripperH3:     mockRoundTripper{resp: respH3},
+				roundTripperH1:     mockRoundTripper{responseTemplate: respH1Template},
+				roundTripperH3:     mockRoundTripper{responseTemplate: respH3Template},
 				isCurrentH3:        newAtomicBool(false),
 				lastH3AttemptMilli: newAtomicInt64(time.Now().Add(-time.Second).UnixMilli()),
 				h3ReviveTime:       time.Second * 2,
 			},
-			resp: respH1,
+			resp: respH1Template,
 		},
 		{
 			name: "h1 fails while on h3",
 			roundTripper: &RotatingRoundTripper{
 				roundTripperH1:     mockRoundTripper{err: err1},
-				roundTripperH3:     mockRoundTripper{resp: respH3},
+				roundTripperH3:     mockRoundTripper{responseTemplate: respH3Template},
 				isCurrentH3:        newAtomicBool(false),
 				lastH3AttemptMilli: &atomic.Int64{},
 				h3ReviveTime:       time.Duration(0),
 			},
-			resp: respH3,
+			resp: respH3Template,
 		},
 		{
 			name: "h3 fails",
 			roundTripper: &RotatingRoundTripper{
-				roundTripperH1:     mockRoundTripper{resp: respH1},
+				roundTripperH1:     mockRoundTripper{responseTemplate: respH1Template},
 				roundTripperH3:     mockRoundTripper{err: err1},
 				isCurrentH3:        newAtomicBool(false),
 				lastH3AttemptMilli: &atomic.Int64{},
 				h3ReviveTime:       time.Duration(0),
 			},
-			resp: respH1,
+			resp: respH1Template,
 		},
 	} {
 		t.Run(tt.name, func(t *testing.T) {
@@ -109,7 +132,11 @@ func TestRotatingRoundTripper_RoundTrip(t *testing.T) {
 				defer resp.Body.Close()
 			}
 			assert.ErrorIs(t, tt.err, err)
-			assert.Equal(t, tt.resp, resp)
+			assert.Equal(t, tt.resp.protoMajor, resp.ProtoMajor)
+
+			body, err := io.ReadAll(resp.Body)
+			assert.NoError(t, err)
+			assert.Equal(t, tt.resp.body, string(body))
 		})
 	}
 }
@@ -127,7 +154,7 @@ func TestRotatingRoundTripper_RoundTripThreadSafety(t *testing.T) {
 			n:    5,
 			roundTripper: NewRotatingRoundTripper(
 				mockRoundTripper{err: err1},
-				mockRoundTripper{resp: respH3, duration: time.Millisecond * 100},
+				mockRoundTripper{responseTemplate: respH3Template, duration: time.Millisecond * 100},
 				time.Duration(0),
 			),
 			duration: time.Millisecond * 200,
@@ -136,7 +163,7 @@ func TestRotatingRoundTripper_RoundTripThreadSafety(t *testing.T) {
 			name: "http3 fails and subsequent calls wait for every h1 rt",
 			n:    5,
 			roundTripper: NewRotatingRoundTripper(
-				mockRoundTripper{resp: respH1, duration: time.Millisecond * 100},
+				mockRoundTripper{responseTemplate: respH1Template, duration: time.Millisecond * 100},
 				mockRoundTripper{err: err1, duration: time.Millisecond * 200},
 				time.Duration(0),
 			),
@@ -146,7 +173,7 @@ func TestRotatingRoundTripper_RoundTripThreadSafety(t *testing.T) {
 			name: "http3 fails and subsequent calls wait for h3 once",
 			n:    5,
 			roundTripper: NewRotatingRoundTripper(
-				mockRoundTripper{resp: respH1, duration: time.Millisecond * 100},
+				mockRoundTripper{responseTemplate: respH1Template, duration: time.Millisecond * 100},
 				mockRoundTripper{err: err1, duration: time.Millisecond * 200},
 				time.Minute,
 			),