Skip to content

Commit 5adc149

Browse files
mterharcodebotenaxw
authored
Allow Content-Encoding to be unset when CompressionAlgorithms: []string{} (#14132)
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description When a receiver uses the empty array for CompessionAlgorithm, it tells the middleware to pass any compressed body through to the receiver. This works fine for any body that has a `Conent-Encoding` header set, but when that header is missing, an empty string, or `Identity`, it will return a 400 response to the client without passing it along to the receiver. This change bypasses the error when Content-Encoding is unset, empty, or Identity. <!-- Issue number if applicable --> #### Link to tracking issue Fixes #14131 and open-telemetry/opentelemetry-collector-contrib#44010 <!--Describe what testing was performed and which tests were added.--> #### Testing Added some test cases to ensure the missing content-encoding doesn't error out. The test fails with the original code and passes with the minimal change in the `compression.go` file. <!--Describe the documentation added.--> #### Documentation <!--Please delete paragraphs that you did not use before submitting.--> --------- Co-authored-by: Alex Boten <[email protected]> Co-authored-by: Andrew Wilkins <[email protected]>
1 parent d3992cf commit 5adc149

File tree

3 files changed

+167
-0
lines changed

3 files changed

+167
-0
lines changed
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
change_type: enhancement
2+
component: pkg/config/confighttp
3+
note: Setting `compression_algorithms` to an empty list now disables automatic decompression, ignoring Content-Encoding
4+
5+
# One or more tracking issues or pull requests related to the change
6+
issues: [14131]
7+
8+
# (Optional) One or more lines of additional information to render under the primary note.
9+
# These lines will be padded with 2 spaces and then inserted directly into the document.
10+
# Use pipe (|) for multiline entries.
11+
subtext:
12+
13+
# Optional: The change log or logs in which this entry should be included.
14+
# e.g. '[user]' or '[user, api]'
15+
# Include 'user' if the change is relevant to end users.
16+
# Include 'api' if there is a change to a library API.
17+
# Default: '[user]'
18+
change_logs: [user]

config/confighttp/compression.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,11 @@ func (d *decompressor) ServeHTTP(w http.ResponseWriter, r *http.Request) {
278278
}
279279

280280
func (d *decompressor) newBodyReader(r *http.Request) (io.ReadCloser, error) {
281+
if len(d.decoders) == 0 {
282+
return nil, nil // Signal: don't replace r.Body
283+
}
281284
encoding := r.Header.Get(headerContentEncoding)
285+
282286
decoder, ok := d.decoders[encoding]
283287
if !ok {
284288
return nil, fmt.Errorf("unsupported %s: %s", headerContentEncoding, encoding)

config/confighttp/compression_test.go

Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -386,6 +386,151 @@ func TestHTTPContentDecompressionHandler(t *testing.T) {
386386
}
387387
}
388388

389+
// TestEmptyCompressionAlgorithmsAllowsUncompressed verifies that when CompressionAlgorithms
390+
// is set to an empty array, requests without Content-Encoding header are accepted.
391+
func TestEmptyCompressionAlgorithmsAllowsUncompressed(t *testing.T) {
392+
testBody := []byte(`{"message": "test data"}`)
393+
394+
tests := []struct {
395+
name string
396+
compressionAlgorithms []string
397+
contentEncoding string // empty string means no header
398+
expectedStatus int
399+
expectedError string
400+
compressionBypassed bool // If true, don't compress the body (simulates bypass behavior)
401+
}{
402+
// Case 1: Empty array should bypass decompression
403+
{
404+
name: "EmptyArray_NoContentEncoding_Accepted",
405+
compressionAlgorithms: []string{},
406+
contentEncoding: "",
407+
expectedStatus: http.StatusOK,
408+
},
409+
{
410+
name: "EmptyArray_Gzip_PassedThrough",
411+
compressionAlgorithms: []string{},
412+
contentEncoding: "gzip",
413+
expectedStatus: http.StatusOK,
414+
compressionBypassed: true, // Empty array bypasses decompression
415+
},
416+
417+
{
418+
name: "EmptyArray_RandomEncoding_Accepted",
419+
compressionAlgorithms: []string{},
420+
contentEncoding: "randomstuff",
421+
expectedStatus: http.StatusOK,
422+
},
423+
// Case 2: Explicit list with only compressed formats should reject uncompressed
424+
{
425+
name: "OnlyZstd_NoContentEncoding_Rejected",
426+
compressionAlgorithms: []string{"zstd"},
427+
contentEncoding: "",
428+
expectedStatus: http.StatusBadRequest,
429+
expectedError: "unsupported Content-Encoding",
430+
},
431+
{
432+
name: "OnlyZstd_Zstd_Accepted",
433+
compressionAlgorithms: []string{"zstd"},
434+
contentEncoding: "zstd",
435+
expectedStatus: http.StatusOK,
436+
},
437+
{
438+
name: "OnlyZstd_GzipContentEncoding_Rejected",
439+
compressionAlgorithms: []string{"zstd"},
440+
contentEncoding: "gzip",
441+
expectedStatus: http.StatusBadRequest,
442+
expectedError: "unsupported Content-Encoding",
443+
},
444+
// Case 3: Explicit list including empty string should accept uncompressed
445+
{
446+
name: "WithEmptyString_NoContentEncoding_Accepted",
447+
compressionAlgorithms: []string{"", "gzip", "zstd"},
448+
contentEncoding: "",
449+
expectedStatus: http.StatusOK,
450+
},
451+
{
452+
name: "WithEmptyString_Gzip_Accepted",
453+
compressionAlgorithms: []string{"", "gzip"},
454+
contentEncoding: "gzip",
455+
expectedStatus: http.StatusOK,
456+
},
457+
}
458+
459+
for _, tt := range tests {
460+
t.Run(tt.name, func(t *testing.T) {
461+
// Create handler that echoes back the request body
462+
// If there's an error reading, it returns 500 which the test will catch
463+
handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
464+
body, err := io.ReadAll(r.Body)
465+
if err != nil {
466+
http.Error(w, "failed to read body", http.StatusInternalServerError)
467+
return
468+
}
469+
w.WriteHeader(http.StatusOK)
470+
_, _ = w.Write(body)
471+
})
472+
473+
// Create ServerConfig with the specified CompressionAlgorithms
474+
serverConfig := &ServerConfig{
475+
Endpoint: "localhost:0",
476+
CompressionAlgorithms: tt.compressionAlgorithms,
477+
}
478+
479+
srv, err := serverConfig.ToServer(
480+
context.Background(),
481+
componenttest.NewNopHost(),
482+
componenttest.NewNopTelemetrySettings(),
483+
handler,
484+
)
485+
require.NoError(t, err)
486+
487+
// Create test server
488+
testSrv := httptest.NewServer(srv.Handler)
489+
defer testSrv.Close()
490+
491+
// Compress the body if needed for the test
492+
requestBody := testBody
493+
if !tt.compressionBypassed && tt.expectedStatus == http.StatusOK {
494+
switch tt.contentEncoding {
495+
case "gzip":
496+
requestBody = compressGzip(t, testBody).Bytes()
497+
case "zstd":
498+
requestBody = compressZstd(t, testBody).Bytes()
499+
}
500+
}
501+
502+
// Create request
503+
req, err := http.NewRequest(http.MethodPost, testSrv.URL, bytes.NewReader(requestBody))
504+
require.NoError(t, err)
505+
req.Header.Set("Content-Type", "application/json")
506+
507+
// Set Content-Encoding header if specified
508+
if tt.contentEncoding != "" {
509+
req.Header.Set("Content-Encoding", tt.contentEncoding)
510+
}
511+
512+
// Send request
513+
client := &http.Client{}
514+
resp, err := client.Do(req)
515+
require.NoError(t, err)
516+
defer resp.Body.Close()
517+
518+
// Verify response
519+
assert.Equal(t, tt.expectedStatus, resp.StatusCode, "Unexpected status code")
520+
521+
body, err := io.ReadAll(resp.Body)
522+
require.NoError(t, err)
523+
524+
if tt.expectedError != "" {
525+
assert.Contains(t, string(body), tt.expectedError, "Expected error message not found")
526+
} else {
527+
// For successful requests, body should be echoed back
528+
assert.Equal(t, testBody, body, "Response body should match request body")
529+
}
530+
})
531+
}
532+
}
533+
389534
func TestHTTPContentCompressionRequestWithNilBody(t *testing.T) {
390535
compressedGzipBody := compressGzip(t, []byte{})
391536
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

0 commit comments

Comments
 (0)