Skip to content

[TT-14991] Fixed decode settings to fallback to global settings #901

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
lghiur wants to merge 1 commit into from
Closed

[TT-14991] Fixed decode settings to fallback to global settings #901

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 34 additions & 20 deletions main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -348,12 +348,42 @@
}
}

func getDecodingSettings(pump pumps.Pump) (decodingRequest, decodingResponse bool) {
decodingResponse = pump.GetDecodedResponse() || SystemConfig.DecodeRawResponse
decodingRequest = pump.GetDecodedRequest() || SystemConfig.DecodeRawRequest
return decodingRequest, decodingResponse
}

Check warning on line 355 in main.go

View check run for this annotation

probelabs / Visor: security

security Issue 

The configuration fallback logic may cause unintentional decoding of sensitive data. If a pump-level decoding setting is explicitly `false` but the global setting is `true`, the data will still be decoded, potentially exposing sensitive information.
Raw output 
Modify the configuration logic to distinguish between an unset value and an explicit `false`. This can be achieved by using a pointer to a boolean (`*bool`) for the decoding settings in the pump configuration. The `getDecodingSettings` function should then check if the pump-level setting is `nil`. If it is, fall back to the global setting; otherwise, use the pump-level value.

func decodeBase64Data(record *analytics.AnalyticsRecord, decodeRequest, decodeResponse bool) {
// DECODING RAW REQUEST AND RESPONSE FROM BASE 64
if decodeRequest {
rawRequest, err := base64.StdEncoding.DecodeString(record.RawRequest)
if err == nil {
record.RawRequest = string(rawRequest)
}
}
if decodeResponse {
rawResponse, err := base64.StdEncoding.DecodeString(record.RawResponse)
if err == nil {
record.RawResponse = string(rawResponse)
}
}
}

func trimRawData(record *analytics.AnalyticsRecord, pump pumps.Pump) {
if pump.GetMaxRecordSize() != 0 {
record.TrimRawData(pump.GetMaxRecordSize())
} else {
record.TrimRawData(SystemConfig.MaxRecordSize)
}
}

func filterData(pump pumps.Pump, keys []interface{}) []interface{} {

Check warning on line 381 in main.go

View check run for this annotation

probelabs / Visor: performance

performance Issue 

The introduction of `Clone()` inside this loop creates a new `AnalyticsRecord` object for every record in every batch, for every pump. This will significantly increase memory allocations and garbage collector pressure in a performance-critical path, potentially degrading throughput.
Raw output 
While cloning is necessary for correctness to prevent data corruption between pumps, its performance impact should be acknowledged and monitored. Consider a lazy-cloning approach where the record is only copied if a modification (like trimming or decoding) is actually required for the specific pump configuration. This would avoid allocations for pumps that do not alter the record.

Check failure on line 381 in main.go

View check run for this annotation

probelabs / Visor: performance

logic Issue 

The code calls the method `Clone()` on `analytics.AnalyticsRecord`, but this method does not appear to be defined in the repository or its dependencies based on the provided context. This will cause a compilation error.
Raw output 
Ensure the `Clone()` method implementation is included in this pull request or that the relevant dependency providing it has been updated. The PR cannot be merged in its current state.
shouldTrim := SystemConfig.MaxRecordSize != 0 || pump.GetMaxRecordSize() != 0
filters := pump.GetFilters()
ignoreFields := pump.GetIgnoreFields()
getDecodingResponse := pump.GetDecodedResponse()
getDecodingRequest := pump.GetDecodedRequest()
getDecodingRequest, getDecodingResponse := getDecodingSettings(pump)

// Checking to see if all the config options are empty/false
if !getDecodingRequest && !getDecodingResponse && !filters.HasFilter() && !pump.GetOmitDetailedRecording() && !shouldTrim && len(ignoreFields) == 0 {
return keys
Expand All @@ -371,11 +401,7 @@
decoded.RawResponse = ""
} else {
if shouldTrim {
if pump.GetMaxRecordSize() != 0 {
decoded.TrimRawData(pump.GetMaxRecordSize())
} else {
decoded.TrimRawData(SystemConfig.MaxRecordSize)
}
trimRawData(&decoded, pump)
}
}
if filters.ShouldFilter(decoded) {
Expand All @@ -384,19 +410,7 @@
if len(ignoreFields) > 0 {
decoded.RemoveIgnoredFields(ignoreFields)
}
// DECODING RAW REQUEST AND RESPONSE FROM BASE 64
if getDecodingRequest {
rawRequest, err := base64.StdEncoding.DecodeString(decoded.RawRequest)
if err == nil {
decoded.RawRequest = string(rawRequest)
}
}
if getDecodingResponse {
rawResponse, err := base64.StdEncoding.DecodeString(decoded.RawResponse)
if err == nil {
decoded.RawResponse = string(rawResponse)
}
}
decodeBase64Data(&decoded, getDecodingRequest, getDecodingResponse)
filteredKeys[newLenght] = decoded
newLenght++
}
Expand Down
119 changes: 119 additions & 0 deletions main_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -376,3 +376,122 @@ func TestDecodedKey(t *testing.T) {
})
}
}

func TestGetDecodingSettings(t *testing.T) {
originalSystemConfig := SystemConfig
t.Cleanup(func() {
SystemConfig = originalSystemConfig
})

testCases := []struct {
name string
pumpDecodeRequest bool
pumpDecodeResponse bool
globalDecodeRequest bool
globalDecodeResponse bool
expectedDecodeRequest bool
expectedDecodeResponse bool
}{
{
name: "pump level settings both enabled",
pumpDecodeRequest: true,
pumpDecodeResponse: true,
globalDecodeRequest: false,
globalDecodeResponse: false,
expectedDecodeRequest: true,
expectedDecodeResponse: true,
},
{
name: "pump level settings both disabled, global both enabled",
pumpDecodeRequest: false,
pumpDecodeResponse: false,
globalDecodeRequest: true,
globalDecodeResponse: true,
expectedDecodeRequest: true,
expectedDecodeResponse: true,
},
{
name: "pump level settings both disabled, global both disabled",
pumpDecodeRequest: false,
pumpDecodeResponse: false,
globalDecodeRequest: false,
globalDecodeResponse: false,
expectedDecodeRequest: false,
expectedDecodeResponse: false,
},
{
name: "pump request enabled, pump response disabled, global both enabled",
pumpDecodeRequest: true,
pumpDecodeResponse: false,
globalDecodeRequest: true,
globalDecodeResponse: true,
expectedDecodeRequest: true,
expectedDecodeResponse: true,
},
{
name: "pump request disabled, pump response enabled, global both enabled",
pumpDecodeRequest: false,
pumpDecodeResponse: true,
globalDecodeRequest: true,
globalDecodeResponse: true,
expectedDecodeRequest: true,
expectedDecodeResponse: true,
},
{
name: "pump request enabled, pump response disabled, global request disabled, global response enabled",
pumpDecodeRequest: true,
pumpDecodeResponse: false,
globalDecodeRequest: false,
globalDecodeResponse: true,
expectedDecodeRequest: true,
expectedDecodeResponse: true,
},
{
name: "pump request disabled, pump response enabled, global request enabled, global response disabled",
pumpDecodeRequest: false,
pumpDecodeResponse: true,
globalDecodeRequest: true,
globalDecodeResponse: false,
expectedDecodeRequest: true,
expectedDecodeResponse: true,
},
{
name: "pump request enabled, pump response disabled, global both disabled",
pumpDecodeRequest: true,
pumpDecodeResponse: false,
globalDecodeRequest: false,
globalDecodeResponse: false,
expectedDecodeRequest: true,
expectedDecodeResponse: false,
},
{
name: "pump request disabled, pump response enabled, global both disabled",
pumpDecodeRequest: false,
pumpDecodeResponse: true,
globalDecodeRequest: false,
globalDecodeResponse: false,
expectedDecodeRequest: false,
expectedDecodeResponse: true,
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
SystemConfig = TykPumpConfiguration{
DecodeRawRequest: tc.globalDecodeRequest,
DecodeRawResponse: tc.globalDecodeResponse,
}

mockedPump := &MockedPump{}
mockedPump.SetDecodingRequest(tc.pumpDecodeRequest)
mockedPump.SetDecodingResponse(tc.pumpDecodeResponse)

actualDecodeRequest, actualDecodeResponse := getDecodingSettings(mockedPump)

assert.Equal(t, tc.expectedDecodeRequest, actualDecodeRequest,
"DecodeRequest mismatch: expected %v, got %v", tc.expectedDecodeRequest, actualDecodeRequest)
assert.Equal(t, tc.expectedDecodeResponse, actualDecodeResponse,
"DecodeResponse mismatch: expected %v, got %v", tc.expectedDecodeResponse, actualDecodeResponse)
})
}
}
Loading