fixup! fixup! fixup! fixup! fixup! fixup! fixup! Library improvements.

2026-10-12 00:45:31 +00:00 · 2025-05-23 18:47:52 +01:00
parent dc3ea548c4
commit 5da571351d
5 changed files with 354 additions and 14 deletions
@@ -29,7 +29,7 @@ var (
 		MaxIdleConns:        100,
 		MaxIdleConnsPerHost: 100,
 		IdleConnTimeout:     90 * time.Second,
-		DisableCompression:  false, // Enable automatic compression for consistent handling
+		DisableCompression:  true, // Disable automatic request compression to prevent "trailing garbage" errors
 		ForceAttemptHTTP2:   true,
 		TLSClientConfig:     &tls.Config{InsecureSkipVerify: true}, // Skip TLS verification for test environments
 	}
@@ -66,10 +66,10 @@ func (qe *QueryExecutor) executeQuery() ([]byte, error) {
 		httpRequest.Header.Set("Content-Type", "application/json")
 	}

-	// Set Accept-Encoding header to request gzip compression
-	if httpRequest.Header.Get("Accept-Encoding") == "" {
-		httpRequest.Header.Set("Accept-Encoding", "gzip")
-	}
+	// Explicitly remove Accept-Encoding header to prevent automatic request compression
+	// Response decompression is still handled intelligently based on Content-Encoding header
+	// and gzip magic bytes detection in the response processing logic below
+	httpRequest.Header.Set("Accept-Encoding", "identity")

 	retriesMax := 1
 	if qe.Retries {
@@ -83,10 +83,20 @@ func (qe *QueryExecutor) executeQuery() ([]byte, error) {
 			buf.Reset()
 			buf.Write(qe.Query)

-			// Set the body of the request
+			// Set the body of the request (plain JSON, no compression)
 			httpRequest.Body = io.NopCloser(bytes.NewReader(buf.Bytes()))
 			httpRequest.ContentLength = int64(buf.Len())

+			// Debug log to confirm request is sent as plain JSON
+			qe.Logger.Debug(&libpack_logger.LogMessage{
+				Message: "Sending GraphQL request as plain JSON",
+				Pairs: map[string]interface{}{
+					"content_length":       httpRequest.ContentLength,
+					"content_type":         httpRequest.Header.Get("Content-Type"),
+					"compression_disabled": true,
+				},
+			})
+
 			// Use default client if custom client is not set
 			client := qe.client
 			if client == nil {
@@ -19,7 +19,7 @@ func (b *BaseClient) createHttpClient() (http_client *http.Client) {
 		IdleConnTimeout:       30 * time.Second, // Increased for better reuse
 		ResponseHeaderTimeout: 10 * time.Second, // Reduced for faster timeouts
 		DisableKeepAlives:     false,
-		DisableCompression:    false,
+		DisableCompression:    true, // Disable automatic request compression to prevent "trailing garbage" errors
 		WriteBufferSize:       4096, // Optimize buffer size
 		ReadBufferSize:        4096, // Optimize buffer size
 	}
@@ -42,11 +42,12 @@ func (b *BaseClient) createHttpClient() (http_client *http.Client) {
 			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
+			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
@@ -27,7 +27,7 @@ func (suite *Tests) TestBaseClient_createHttpClient() {
 		assert.Equal(30*time.Second, transport.IdleConnTimeout)
 		assert.Equal(10*time.Second, transport.ResponseHeaderTimeout)
 		assert.False(transport.DisableKeepAlives)
-		assert.False(transport.DisableCompression)
+		assert.True(transport.DisableCompression) // Request compression disabled to prevent "trailing garbage" errors
 		assert.Equal(4096, transport.WriteBufferSize)
 		assert.Equal(4096, transport.ReadBufferSize)
 	})
@@ -106,7 +106,7 @@ func (suite *Tests) TestBaseClient_createHttpClient_transportSettings() {
 		assert.Equal(30*time.Second, transport.IdleConnTimeout)
 		assert.Equal(10*time.Second, transport.ResponseHeaderTimeout)
 		assert.False(transport.DisableKeepAlives)
-		assert.False(transport.DisableCompression)
+		assert.True(transport.DisableCompression) // Request compression disabled to prevent "trailing garbage" errors
 		assert.Equal(4096, transport.WriteBufferSize)
 		assert.Equal(4096, transport.ReadBufferSize)
 	})
@@ -0,0 +1,83 @@
+package gql
+
+import (
+	"encoding/json"
+	"testing"
+
+	assertions "github.com/stretchr/testify/assert"
+)
+
+// TestQueryJSONEncoding verifies that the Query struct serializes correctly
+// without including the JsonQuery field, preventing "Trailing garbage" errors
+func TestQueryJSONEncoding(t *testing.T) {
+	assert := assertions.New(t)
+	client := CreateTestClient()
+
+	query := &Query{
+		Query: "query GetUser($id: ID!) { user(id: $id) { id name } }",
+		Variables: map[string]interface{}{
+			"id": "123",
+		},
+	}
+
+	// Set JsonQuery to simulate the compiled query
+	query.JsonQuery = client.convertToJSON(query)
+
+	// Serialize the query to JSON
+	jsonBytes, err := json.Marshal(query)
+	assert.NoError(err)
+
+	// Parse the JSON back to verify structure
+	var parsed map[string]interface{}
+	err = json.Unmarshal(jsonBytes, &parsed)
+	assert.NoError(err)
+
+	// Verify that only query and variables are included, NOT jsonQuery
+	assert.Contains(parsed, "query")
+	assert.Contains(parsed, "variables")
+	assert.NotContains(parsed, "jsonQuery", "JsonQuery field should be excluded from JSON serialization")
+
+	// Verify the content is correct
+	assert.Equal(query.Query, parsed["query"])
+	assert.Equal(query.Variables["id"], parsed["variables"].(map[string]interface{})["id"])
+
+	// Verify the JSON is valid GraphQL format
+	assert.True(json.Valid(jsonBytes), "Generated JSON should be valid")
+}
+
+// TestQueryJSONEncodingPreventsTrailingGarbage ensures the fix prevents the specific error
+func TestQueryJSONEncodingPreventsTrailingGarbage(t *testing.T) {
+	assert := assertions.New(t)
+	client := CreateTestClient()
+
+	// Create a query with complex variables
+	query := &Query{
+		Query: "mutation storeMessage($content: String!, $userID: Int!) { insert_message(content: $content, user_id: $userID) { id } }",
+		Variables: map[string]interface{}{
+			"content": "Test message with special chars: {}[]\"'",
+			"userID":  12345,
+		},
+	}
+
+	// Compile the query (this sets JsonQuery)
+	query.JsonQuery = client.convertToJSON(query)
+
+	// Serialize to JSON - this should NOT include JsonQuery field
+	jsonBytes, err := json.Marshal(query)
+	assert.NoError(err)
+
+	// The JSON should be valid and parseable
+	var graphqlRequest map[string]interface{}
+	err = json.Unmarshal(jsonBytes, &graphqlRequest)
+	assert.NoError(err)
+
+	// Should have exactly 2 fields: query and variables
+	assert.Len(graphqlRequest, 2)
+	assert.Contains(graphqlRequest, "query")
+	assert.Contains(graphqlRequest, "variables")
+
+	// The JSON should be suitable for sending to a GraphQL server
+	jsonString := string(jsonBytes)
+	assert.NotContains(jsonString, "jsonQuery", "JSON should not contain jsonQuery field")
+	assert.NotContains(jsonString, "eyJ", "JSON should not contain base64 encoded data")
+}
@@ -0,0 +1,246 @@
+package gql
+
+import (
+	"bytes"
+	"compress/gzip"
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+
+	assertions "github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/suite"
+)
+
+type RequestCompressionTestSuite struct {
+	suite.Suite
+	assert *assertions.Assertions
+}
+
+func TestRequestCompressionSuite(t *testing.T) {
+	suite.Run(t, new(RequestCompressionTestSuite))
+}
+
+func (suite *RequestCompressionTestSuite) SetupTest() {
+	suite.assert = assertions.New(suite.T())
+}
+
+func (suite *RequestCompressionTestSuite) TestRequestCompression() {
+	suite.T().Run("should disable request compression for HTTP clients", func(t *testing.T) {
+		assert := assertions.New(t)
+		// Track what the server receives
+		var receivedHeaders http.Header
+		var receivedBody []byte
+		var receivedContentEncoding string
+
+		// Create test server that captures request details
+		server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			receivedHeaders = r.Header.Clone()
+			receivedContentEncoding = r.Header.Get("Content-Encoding")
+
+			// Read the request body
+			body, err := io.ReadAll(r.Body)
+			if err != nil {
+				t.Errorf("Failed to read request body: %v", err)
+				return
+			}
+			receivedBody = body
+
+			// Send back a simple JSON response
+			w.Header().Set("Content-Type", "application/json")
+			w.WriteHeader(http.StatusOK)
+			w.Write([]byte(`{"data":{"viewer":{"login":"testuser"}}}`))
+		}))
+		defer server.Close()
+
+		// Test both HTTP and HTTPS endpoints
+		endpoints := []string{
+			strings.Replace(server.URL, "http://", "http://", 1),  // HTTP
+			strings.Replace(server.URL, "http://", "https://", 1), // HTTPS (will use HTTP/2)
+		}
+
+		for _, endpoint := range endpoints {
+			t.Run("endpoint_"+endpoint, func(t *testing.T) {
+				// Reset captured data
+				receivedHeaders = nil
+				receivedBody = nil
+				receivedContentEncoding = ""
+
+				client := CreateTestClient()
+				client.endpoint = endpoint
+
+				// Create HTTP client using the same logic as the library
+				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")
+					}
+				} else {
+					// HTTP/2 transport - this is where the issue might be
+					// We need to ensure HTTP/2 transport also has compression disabled
+				}
+
+				qe := &QueryExecutor{
+					BaseClient: client,
+					Query:      []byte(`{"query":"query { viewer { login } }"}`),
+					Headers:    map[string]interface{}{"Content-Type": "application/json"},
+					CacheKey:   "no-cache",
+					Retries:    false,
+				}
+				qe.endpoint = server.URL    // Use HTTP server URL regardless of test endpoint
+				qe.client = server.Client() // Use server's client to avoid TLS issues
+				qe.cache = client.cache
+
+				result, err := qe.executeQuery()
+				assert.NoError(err, "Query should execute successfully")
+				assert.NotNil(result, "Result should not be nil")
+
+				// Verify request was sent as plain JSON (not compressed)
+				assert.Empty(receivedContentEncoding, "Request should not have Content-Encoding header")
+				assert.NotContains(receivedHeaders.Get("Accept-Encoding"), "gzip", "Should not automatically request gzip encoding")
+
+				// Verify request body is plain JSON (not gzip compressed)
+				assert.True(len(receivedBody) > 0, "Should have received request body")
+
+				// Check that body is NOT gzip compressed by checking magic bytes
+				if len(receivedBody) >= 2 {
+					assert.False(receivedBody[0] == 0x1f && receivedBody[1] == 0x8b,
+						"Request body should NOT have gzip magic bytes (0x1f 0x8b), got: 0x%02x 0x%02x",
+						receivedBody[0], receivedBody[1])
+				}
+
+				// Verify body is valid JSON
+				bodyStr := string(receivedBody)
+				assert.True(strings.HasPrefix(bodyStr, "{"), "Request body should start with '{' (JSON)")
+				assert.Contains(bodyStr, "query", "Request body should contain 'query' field")
+				assert.Contains(bodyStr, "viewer", "Request body should contain the query content")
+
+				// Verify no automatic compression headers were added
+				assert.Empty(receivedHeaders.Get("Content-Encoding"), "Request should not have Content-Encoding header")
+
+				// The Accept-Encoding header should either be empty or not contain gzip
+				acceptEncoding := receivedHeaders.Get("Accept-Encoding")
+				if acceptEncoding != "" {
+					assert.NotContains(acceptEncoding, "gzip", "Accept-Encoding should not request gzip compression")
+				}
+			})
+		}
+	})
+
+	suite.T().Run("should handle gzip responses correctly while sending plain requests", func(t *testing.T) {
+		assert := assertions.New(t)
+		var receivedBody []byte
+		var receivedContentEncoding string
+
+		// Server that captures request and sends gzip response
+		server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			// Capture request details
+			receivedContentEncoding = r.Header.Get("Content-Encoding")
+			body, _ := io.ReadAll(r.Body)
+			receivedBody = body
+
+			// Send gzip compressed response
+			w.Header().Set("Content-Type", "application/json")
+			w.Header().Set("Content-Encoding", "gzip")
+
+			var buf bytes.Buffer
+			gzipWriter := gzip.NewWriter(&buf)
+			gzipWriter.Write([]byte(`{"data":{"viewer":{"login":"gzipresponseuser"}}}`))
+			gzipWriter.Close()
+
+			w.WriteHeader(http.StatusOK)
+			w.Write(buf.Bytes())
+		}))
+		defer server.Close()
+
+		client := CreateTestClient()
+
+		qe := &QueryExecutor{
+			BaseClient: client,
+			Query:      []byte(`{"query":"query { viewer { login } }"}`),
+			Headers:    map[string]interface{}{"Content-Type": "application/json"},
+			CacheKey:   "no-cache",
+			Retries:    false,
+		}
+		qe.endpoint = server.URL
+		qe.client = server.Client()
+		qe.cache = client.cache
+
+		result, err := qe.executeQuery()
+		assert.NoError(err, "Should handle gzip response correctly")
+		assert.NotNil(result, "Result should not be nil")
+		assert.Contains(string(result), "gzipresponseuser", "Should decompress gzip response")
+
+		// Verify request was sent as plain JSON
+		assert.Empty(receivedContentEncoding, "Request should not be compressed")
+		assert.True(len(receivedBody) > 0, "Should have received request body")
+
+		// Verify request is not gzip compressed
+		if len(receivedBody) >= 2 {
+			assert.False(receivedBody[0] == 0x1f && receivedBody[1] == 0x8b,
+				"Request should not be gzip compressed")
+		}
+
+		// Verify request body is valid JSON
+		bodyStr := string(receivedBody)
+		assert.True(strings.HasPrefix(bodyStr, "{"), "Request should be JSON")
+		assert.Contains(bodyStr, "query", "Request should contain query")
+	})
+
+	suite.T().Run("should prevent trailing garbage errors from compressed requests", func(t *testing.T) {
+		assert := assertions.New(t)
+		var receivedBody []byte
+
+		// Server that would fail if it received compressed requests
+		server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			body, _ := io.ReadAll(r.Body)
+			receivedBody = body
+
+			// Check if request looks like gzip (which would cause trailing garbage errors)
+			if len(body) >= 2 && body[0] == 0x1f && body[1] == 0x8b {
+				// Simulate server that can't handle gzip requests properly
+				w.WriteHeader(http.StatusBadRequest)
+				w.Write([]byte(`{"error":"trailing garbage after JSON"}`))
+				return
+			}
+
+			// Normal JSON response for plain requests
+			w.Header().Set("Content-Type", "application/json")
+			w.WriteHeader(http.StatusOK)
+			w.Write([]byte(`{"data":{"viewer":{"login":"notrailinggarbage"}}}`))
+		}))
+		defer server.Close()
+
+		client := CreateTestClient()
+
+		qe := &QueryExecutor{
+			BaseClient: client,
+			Query:      []byte(`{"query":"query { viewer { login } }"}`),
+			Headers:    map[string]interface{}{"Content-Type": "application/json"},
+			CacheKey:   "no-cache",
+			Retries:    false,
+		}
+		qe.endpoint = server.URL
+		qe.client = server.Client()
+		qe.cache = client.cache
+
+		result, err := qe.executeQuery()
+		assert.NoError(err, "Should not get trailing garbage errors")
+		assert.NotNil(result, "Result should not be nil")
+		assert.Contains(string(result), "notrailinggarbage", "Should get successful response")
+
+		// Verify request was plain JSON (not gzip)
+		assert.True(len(receivedBody) > 0, "Should have received request body")
+		if len(receivedBody) >= 2 {
+			assert.False(receivedBody[0] == 0x1f && receivedBody[1] == 0x8b,
+				"Request should not be gzip compressed to prevent trailing garbage errors")
+		}
+	})
+}