From cd16b92dca0d153f5dfafaef59037df5e8bd4792 Mon Sep 17 00:00:00 2001 From: Lukasz Raczylo Date: Mon, 3 Nov 2025 23:33:28 +0000 Subject: [PATCH] Add support of h2c over cleartext http --- execute_query.go | 34 ++++++------------ execute_query_test.go | 9 ++--- http.go | 70 ++++++++++++++++--------------------- http_test.go | 42 ++++++++++------------ request_compression_test.go | 22 ++++++------ 5 files changed, 77 insertions(+), 100 deletions(-) diff --git a/execute_query.go b/execute_query.go index 9f3c050..9a37a85 100644 --- a/execute_query.go +++ b/execute_query.go @@ -3,7 +3,6 @@ package gql import ( "bytes" "compress/gzip" - "crypto/tls" "errors" "fmt" "io" @@ -74,23 +73,8 @@ func isRetryableError(errors []struct { return false } -var ( - // Shared HTTP transport with optimized settings - defaultTransport = &http.Transport{ - MaxIdleConns: 100, - MaxIdleConnsPerHost: 100, - IdleConnTimeout: 90 * time.Second, - DisableCompression: true, // Disable automatic request compression to prevent "trailing garbage" errors - ForceAttemptHTTP2: true, - TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, // Skip TLS verification for test environments - } - - // Shared HTTP client with timeouts - defaultClient = &http.Client{ - Transport: defaultTransport, - Timeout: 30 * time.Second, - } -) +// Note: defaultClient removed - all clients should be created via NewConnection() +// which properly configures HTTP/2 transport with correct settings func (qe *QueryExecutor) executeQuery() ([]byte, error) { // Reuse buffer from pool to avoid allocations @@ -173,12 +157,16 @@ func (qe *QueryExecutor) executeQuery() ([]byte, error) { }, }) - // Use default client if custom client is not set - client := qe.client - if client == nil { - client = defaultClient + // Client should always be set via NewConnection(), but check for safety + if qe.client == nil { + qe.Logger.Error(&libpack_logger.LogMessage{ + Message: "HTTP client not initialized - use NewConnection() to create client", + Pairs: nil, + }) + return fmt.Errorf("HTTP client not initialized") } - httpResponse, err := client.Do(httpRequest) + + httpResponse, err := qe.client.Do(httpRequest) if err != nil { return err } diff --git a/execute_query_test.go b/execute_query_test.go index 92d01dd..ee875da 100644 --- a/execute_query_test.go +++ b/execute_query_test.go @@ -185,7 +185,7 @@ func (suite *Tests) TestQueryExecutor_executeQuery() { assert.Equal(3, attempts) // Should have retried twice before succeeding }) - suite.T().Run("should use default client when none provided", func(t *testing.T) { + suite.T().Run("should return error when client not initialized", func(t *testing.T) { client := CreateTestClient() qe := &QueryExecutor{ @@ -196,12 +196,13 @@ func (suite *Tests) TestQueryExecutor_executeQuery() { Retries: false, } qe.endpoint = mockServer.URL - qe.client = nil // No client provided + qe.client = nil // No client provided (should always use NewConnection()) qe.cache = client.cache result, err := qe.executeQuery() - assert.NoError(err) - assert.NotNil(result) + assert.Error(err) + assert.Nil(result) + assert.Contains(err.Error(), "HTTP client not initialized") }) suite.T().Run("should set default Content-Type header", func(t *testing.T) { diff --git a/http.go b/http.go index 81b7d1a..cad2034 100644 --- a/http.go +++ b/http.go @@ -11,60 +11,50 @@ import ( ) func (b *BaseClient) createHttpClient() (http_client *http.Client) { - // Optimized connection pool settings for better performance - httpTransport := &http.Transport{ - MaxIdleConns: 100, // Reduced from 512 - MaxConnsPerHost: 50, // Reduced from 512 - MaxIdleConnsPerHost: 10, // Reduced from 512 - IdleConnTimeout: 30 * time.Second, // Increased for better reuse - ResponseHeaderTimeout: 10 * time.Second, // Reduced for faster timeouts - DisableKeepAlives: false, - DisableCompression: true, // Disable automatic request compression to prevent "trailing garbage" errors - WriteBufferSize: 4096, // Optimize buffer size - ReadBufferSize: 4096, // Optimize buffer size + // Create TLS config for HTTPS endpoints + var tlsClientConfig *tls.Config + if strings.HasPrefix(b.endpoint, "https://") { + tlsClientConfig = &tls.Config{ + InsecureSkipVerify: true, // TODO: Make this configurable via environment variable + } } + // Use HTTP/2 transport for both http:// (h2c) and https:// endpoints + // AllowHTTP=true enables HTTP/2 Cleartext (h2c) for unencrypted connections + http2Transport := &http2.Transport{ + AllowHTTP: true, // Enable h2c for http:// endpoints + TLSClientConfig: tlsClientConfig, // nil for http://, configured for https:// + ReadIdleTimeout: 30 * time.Second, // Increased for better connection reuse + PingTimeout: 10 * time.Second, // Reduced for faster dead connection detection + WriteByteTimeout: 10 * time.Second, // Write timeout for better error handling + DisableCompression: true, // Disable automatic compression to prevent trailing garbage errors + } + + http_client = &http.Client{ + Timeout: 30 * time.Second, // Overall request timeout + Transport: http2Transport, + CheckRedirect: func(req *http.Request, via []*http.Request) error { + return http.ErrUseLastResponse // Don't follow redirects automatically + }, + } + + // Log which protocol is being used if strings.HasPrefix(b.endpoint, "http://") { - http_client = &http.Client{ - Timeout: 30 * time.Second, // Increased for better reliability - Transport: httpTransport, - CheckRedirect: func(req *http.Request, via []*http.Request) error { - return http.ErrUseLastResponse - }, - } b.Logger.Debug(&libpack_logger.LogMessage{ - Message: "Using HTTP/1.1 over http", + Message: "Using HTTP/2 Cleartext (h2c) over http", Pairs: nil, }) } else if strings.HasPrefix(b.endpoint, "https://") { - tlsClientConfig := &tls.Config{} - if strings.HasPrefix(b.endpoint, "https://") { - tlsClientConfig.InsecureSkipVerify = true - } - http2Transport := &http2.Transport{ - AllowHTTP: true, - TLSClientConfig: tlsClientConfig, - ReadIdleTimeout: 30 * time.Second, // Increased for better reuse - PingTimeout: 10 * time.Second, // Reduced for faster detection - WriteByteTimeout: 10 * time.Second, // Add write timeout - DisableCompression: true, // Disable automatic request compression to prevent "trailing garbage" errors - } - http_client = &http.Client{ - Timeout: 30 * time.Second, // Increased for better reliability - Transport: http2Transport, - CheckRedirect: func(req *http.Request, via []*http.Request) error { - return http.ErrUseLastResponse - }, - } b.Logger.Debug(&libpack_logger.LogMessage{ - Message: "Using HTTP/2 over https", + Message: "Using HTTP/2 over TLS (https)", Pairs: nil, }) } else { b.Logger.Critical(&libpack_logger.LogMessage{ - Message: "Invalid endpoint - neither http or https", + Message: "Invalid endpoint - must start with http:// or https://", }) return nil } + return http_client } diff --git a/http_test.go b/http_test.go index de7dc5a..a728a61 100644 --- a/http_test.go +++ b/http_test.go @@ -8,8 +8,10 @@ import ( "golang.org/x/net/http2" ) +// Note: http.Transport import is kept for http.ErrUseLastResponse usage in redirect tests + func (suite *Tests) TestBaseClient_createHttpClient() { - suite.T().Run("should create HTTP/1.1 client for http endpoints", func(t *testing.T) { + suite.T().Run("should create HTTP/2 client for http endpoints (h2c)", func(t *testing.T) { client := NewConnection() client.SetEndpoint("http://example.com/graphql") @@ -18,18 +20,15 @@ func (suite *Tests) TestBaseClient_createHttpClient() { assert.Equal(30*time.Second, httpClient.Timeout) assert.NotNil(httpClient.Transport) - // Check that it's using regular HTTP transport - transport, ok := httpClient.Transport.(*http.Transport) + // Check that it's using HTTP/2 transport with h2c (HTTP/2 Cleartext) + transport, ok := httpClient.Transport.(*http2.Transport) assert.True(ok) - assert.Equal(100, transport.MaxIdleConns) - assert.Equal(50, transport.MaxConnsPerHost) - assert.Equal(10, transport.MaxIdleConnsPerHost) - assert.Equal(30*time.Second, transport.IdleConnTimeout) - assert.Equal(10*time.Second, transport.ResponseHeaderTimeout) - assert.False(transport.DisableKeepAlives) + assert.True(transport.AllowHTTP) // Required for h2c + assert.Nil(transport.TLSClientConfig) // No TLS for http:// + assert.Equal(30*time.Second, transport.ReadIdleTimeout) + assert.Equal(10*time.Second, transport.PingTimeout) + assert.Equal(10*time.Second, transport.WriteByteTimeout) assert.True(transport.DisableCompression) // Request compression disabled to prevent "trailing garbage" errors - assert.Equal(4096, transport.WriteBufferSize) - assert.Equal(4096, transport.ReadBufferSize) }) suite.T().Run("should create HTTP/2 client for https endpoints", func(t *testing.T) { @@ -92,23 +91,20 @@ func (suite *Tests) TestBaseClient_createHttpClient() { } func (suite *Tests) TestBaseClient_createHttpClient_transportSettings() { - suite.T().Run("should configure HTTP transport correctly", func(t *testing.T) { + suite.T().Run("should configure HTTP/2 transport correctly for http://", func(t *testing.T) { client := NewConnection() client.SetEndpoint("http://example.com/graphql") httpClient := client.createHttpClient() - transport := httpClient.Transport.(*http.Transport) + transport := httpClient.Transport.(*http2.Transport) - // Verify all transport settings - assert.Equal(100, transport.MaxIdleConns) - assert.Equal(50, transport.MaxConnsPerHost) - assert.Equal(10, transport.MaxIdleConnsPerHost) - assert.Equal(30*time.Second, transport.IdleConnTimeout) - assert.Equal(10*time.Second, transport.ResponseHeaderTimeout) - assert.False(transport.DisableKeepAlives) + // Verify all HTTP/2 transport settings for h2c + assert.True(transport.AllowHTTP) + assert.Nil(transport.TLSClientConfig) // No TLS for http:// + assert.Equal(30*time.Second, transport.ReadIdleTimeout) + assert.Equal(10*time.Second, transport.PingTimeout) + assert.Equal(10*time.Second, transport.WriteByteTimeout) assert.True(transport.DisableCompression) // Request compression disabled to prevent "trailing garbage" errors - assert.Equal(4096, transport.WriteBufferSize) - assert.Equal(4096, transport.ReadBufferSize) }) suite.T().Run("should configure HTTP/2 transport correctly", func(t *testing.T) { @@ -129,7 +125,7 @@ func (suite *Tests) TestBaseClient_createHttpClient_transportSettings() { } func (suite *Tests) TestBaseClient_createHttpClient_logging() { - suite.T().Run("should log HTTP/1.1 usage", func(t *testing.T) { + suite.T().Run("should log HTTP/2 h2c usage", func(t *testing.T) { client := NewConnection() client.SetEndpoint("http://example.com/graphql") diff --git a/request_compression_test.go b/request_compression_test.go index 59a0f68..98d2fce 100644 --- a/request_compression_test.go +++ b/request_compression_test.go @@ -11,6 +11,7 @@ import ( assertions "github.com/stretchr/testify/assert" "github.com/stretchr/testify/suite" + "golang.org/x/net/http2" ) type RequestCompressionTestSuite struct { @@ -74,17 +75,18 @@ func (suite *RequestCompressionTestSuite) TestRequestCompression() { httpClient := client.createHttpClient() assert.NotNil(httpClient, "HTTP client should be created") - // Verify transport settings - if strings.HasPrefix(endpoint, "http://") { - // HTTP/1.1 transport - transport, ok := httpClient.Transport.(*http.Transport) - assert.True(ok, "Should be http.Transport for HTTP endpoints") - if ok { - assert.True(transport.DisableCompression, "DisableCompression should be true for HTTP transport") + // Verify transport settings - now using HTTP/2 for all endpoints + transport, ok := httpClient.Transport.(*http2.Transport) + assert.True(ok, "Should be http2.Transport for all endpoints (h2c for http://, TLS for https://)") + if ok { + assert.True(transport.DisableCompression, "DisableCompression should be true to prevent trailing garbage") + if strings.HasPrefix(endpoint, "http://") { + assert.True(transport.AllowHTTP, "AllowHTTP should be true for h2c (HTTP/2 Cleartext)") + assert.Nil(transport.TLSClientConfig, "TLSClientConfig should be nil for http:// endpoints") + } else { + assert.True(transport.AllowHTTP, "AllowHTTP should be true") + assert.NotNil(transport.TLSClientConfig, "TLSClientConfig should be set for https:// endpoints") } - } else { - // HTTP/2 transport - this is where the issue might be - // We need to ensure HTTP/2 transport also has compression disabled } qe := &QueryExecutor{