From e94839e036b00ae3dc33e0095a7a2795bacb9a4d Mon Sep 17 00:00:00 2001 From: Mudit Joshi Date: Tue, 8 Oct 2024 15:28:52 +0530 Subject: [PATCH 1/5] chore: add response_code check before obfuscating payload --- trace/backend_collector.go | 2 +- trace/util.go | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/trace/backend_collector.go b/trace/backend_collector.go index 435262f..f180eb2 100644 --- a/trace/backend_collector.go +++ b/trace/backend_collector.go @@ -286,7 +286,7 @@ func (c *BackendCollector) queueUpload(w *witnessWithInfo) { } } - if !c.sendWitnessPayloads { + if !c.sendWitnessPayloads || !hasErrorResponses(w.witness.GetMethod()) { // Obfuscate the original value so type inference engine can use it on the // backend without revealing the actual value. obfuscate(w.witness.GetMethod()) diff --git a/trace/util.go b/trace/util.go index 15c5a94..321786e 100644 --- a/trace/util.go +++ b/trace/util.go @@ -1 +1,21 @@ package trace + +import pb "github.com/akitasoftware/akita-ir/go/api_spec" + +// Determines whether a given method has error (4xx or 5xx) response codes. Returns true if the method has at least one response and all response codes are 4xx or 5xx. +// Here method will only have single response object because in agent each witness stores single request-response pair. +func hasErrorResponses(method *pb.Method) bool { + responses := method.GetResponses() + if len(responses) == 0 { + return false + } + + for _, response := range responses { + responseCode := response.Meta.GetHttp().GetResponseCode() + if responseCode < 400 { + return false + } + } + + return true +} From e5b81437096cd1ebcabb7e1a04af1a5da460d191 Mon Sep 17 00:00:00 2001 From: Mudit Joshi Date: Tue, 8 Oct 2024 15:51:22 +0530 Subject: [PATCH 2/5] chore: add test case --- trace/backend_collector_test.go | 152 ++++++++++++++++++++++++++++++++ 1 file changed, 152 insertions(+) diff --git a/trace/backend_collector_test.go b/trace/backend_collector_test.go index 8db3874..22c8ee5 100644 --- a/trace/backend_collector_test.go +++ b/trace/backend_collector_test.go @@ -2,6 +2,7 @@ package trace import ( "encoding/base64" + "fmt" "net/url" "sync" "testing" @@ -299,3 +300,154 @@ func TestFlushExit(t *testing.T) { b.periodicFlush() // Test should exit immediately } + +func TestOnlyObfuscateNonErrorResponses(t *testing.T) { + ctrl := gomock.NewController(t) + mockClient := mockrest.NewMockLearnClient(ctrl) + defer ctrl.Finish() + + var rec witnessRecorder + mockClient. + EXPECT(). + AsyncReportsUpload(gomock.Any(), gomock.Any(), gomock.Any()). + Do(rec.recordAsyncReportsUpload). + AnyTimes(). + Return(nil) + + streamID := uuid.New() + req := akinet.ParsedNetworkTraffic{ + Content: akinet.HTTPRequest{ + StreamID: streamID, + Seq: 1203, + Method: "POST", + URL: &url.URL{ + Path: "/v1/doggos", + }, + Host: "example.com", + Header: map[string][]string{ + "Content-Type": {"application/json"}, + }, + Body: memview.New([]byte(`{"name": "prince", "number": 6119717375543385000}`)), + }, + } + + resp := akinet.ParsedNetworkTraffic{ + Content: akinet.HTTPResponse{ + StreamID: streamID, + Seq: 1203, + StatusCode: 200, + Header: map[string][]string{ + "Content-Type": {"application/json"}, + }, + Body: memview.New([]byte(`{"homes": ["burbank, ca", "jeuno, ak", "versailles"]}`)), + }, + } + + errStreamID := uuid.New() + errReq := akinet.ParsedNetworkTraffic{ + Content: akinet.HTTPRequest{ + StreamID: errStreamID, + Seq: 1204, + Method: "POST", + URL: &url.URL{ + Path: "/v1/doggos", + }, + Host: "example.com", + Header: map[string][]string{ + "Content-Type": {"application/json"}, + }, + Body: memview.New([]byte(`{"name": "error", "number": 202410081550}`)), + }, + } + + errResp := akinet.ParsedNetworkTraffic{ + Content: akinet.HTTPResponse{ + StreamID: errStreamID, + Seq: 1204, + StatusCode: 404, + Header: map[string][]string{ + "Content-Type": {"application/json"}, + }, + Body: memview.New([]byte(`{"homes": ["error", "happened", "here"]}`)), + }, + } + + col := NewBackendCollector(fakeSvc, fakeLrn, mockClient, optionals.None[int](), NewPacketCounter(), true, nil) + assert.NoError(t, col.Process(req)) + assert.NoError(t, col.Process(resp)) + assert.NoError(t, col.Process(errReq)) + assert.NoError(t, col.Process(errResp)) + assert.NoError(t, col.Close()) + + expectedWitnesses := []*pb.Witness{ + { + Method: &pb.Method{ + Id: &pb.MethodID{ + ApiType: pb.ApiType_HTTP_REST, + }, + Args: map[string]*pb.Data{ + "nxnOc5Qy3D4=": newTestBodySpecFromStruct(0, pb.HTTPBody_JSON, "application/json", map[string]*pb.Data{ + "name": dataFromPrimitive(spec_util.NewPrimitiveString("")), + "number": dataFromPrimitive(spec_util.NewPrimitiveInt64(0)), + }), + }, + Responses: map[string]*pb.Data{ + "AyBUQkT0SHU=": newTestBodySpecFromStruct(200, pb.HTTPBody_JSON, "application/json", map[string]*pb.Data{ + "homes": dataFromList( + dataFromPrimitive(spec_util.NewPrimitiveString("")), + dataFromPrimitive(spec_util.NewPrimitiveString("")), + dataFromPrimitive(spec_util.NewPrimitiveString("")), + ), + }), + }, + Meta: &pb.MethodMeta{ + Meta: &pb.MethodMeta_Http{ + Http: &pb.HTTPMethodMeta{ + Method: "POST", + PathTemplate: "/v1/doggos", + Host: "example.com", + }, + }, + }, + }, + }, + { + Method: &pb.Method{ + Id: &pb.MethodID{ + ApiType: pb.ApiType_HTTP_REST, + }, + Args: map[string]*pb.Data{ + "MWeG2T99uHI=": newTestBodySpecFromStruct(0, pb.HTTPBody_JSON, "application/json", map[string]*pb.Data{ + "name": dataFromPrimitive(spec_util.NewPrimitiveString("error")), + "number": dataFromPrimitive(spec_util.NewPrimitiveInt64(202410081550)), + }), + }, + Responses: map[string]*pb.Data{ + "T7Jfr4mf1Zs=": newTestBodySpecFromStruct(404, pb.HTTPBody_JSON, "application/json", map[string]*pb.Data{ + "homes": dataFromList( + dataFromPrimitive(spec_util.NewPrimitiveString("error")), + dataFromPrimitive(spec_util.NewPrimitiveString("happened")), + dataFromPrimitive(spec_util.NewPrimitiveString("here")), + ), + }), + }, + Meta: &pb.MethodMeta{ + Meta: &pb.MethodMeta_Http{ + Http: &pb.HTTPMethodMeta{ + Method: "POST", + PathTemplate: "/v1/doggos", + Host: "example.com", + }, + }, + }, + }, + }, + } + + for i := range expectedWitnesses { + fmt.Println(i) + expected := proto.MarshalTextString(expectedWitnesses[i]) + actual := proto.MarshalTextString(rec.witnesses[i]) + assert.Equal(t, expected, actual) + } +} From 1f910594258f20d67e5a2df5a10ba5ed0786888d Mon Sep 17 00:00:00 2001 From: Mudit Joshi Date: Tue, 8 Oct 2024 23:26:47 +0530 Subject: [PATCH 3/5] chore: update test cases --- trace/backend_collector_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/trace/backend_collector_test.go b/trace/backend_collector_test.go index d2e6ed2..ad819f9 100644 --- a/trace/backend_collector_test.go +++ b/trace/backend_collector_test.go @@ -2,7 +2,6 @@ package trace import ( "encoding/base64" - "fmt" "net/url" "sync" "testing" @@ -407,6 +406,7 @@ func TestOnlyObfuscateNonErrorResponses(t *testing.T) { Method: "POST", PathTemplate: "/v1/doggos", Host: "example.com", + Obfuscation: pb.HTTPMethodMeta_ZERO_VALUE, }, }, }, @@ -438,6 +438,7 @@ func TestOnlyObfuscateNonErrorResponses(t *testing.T) { Method: "POST", PathTemplate: "/v1/doggos", Host: "example.com", + Obfuscation: pb.HTTPMethodMeta_NONE, }, }, }, @@ -446,7 +447,6 @@ func TestOnlyObfuscateNonErrorResponses(t *testing.T) { } for i := range expectedWitnesses { - fmt.Println(i) expected := proto.MarshalTextString(expectedWitnesses[i]) actual := proto.MarshalTextString(rec.witnesses[i]) assert.Equal(t, expected, actual) From c47718b3b43941b881f21d313f8ef6421be86eed Mon Sep 17 00:00:00 2001 From: Mudit Joshi Date: Tue, 8 Oct 2024 23:29:04 +0530 Subject: [PATCH 4/5] chore: address PR comments --- trace/backend_collector.go | 2 +- trace/util.go | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/trace/backend_collector.go b/trace/backend_collector.go index 00f6b23..2605447 100644 --- a/trace/backend_collector.go +++ b/trace/backend_collector.go @@ -289,7 +289,7 @@ func (c *BackendCollector) queueUpload(w *witnessWithInfo) { } } - if !c.sendWitnessPayloads || !hasErrorResponses(w.witness.GetMethod()) { + if !c.sendWitnessPayloads || !hasOnlyErrorResponses(w.witness.GetMethod()) { // Obfuscate the original value so type inference engine can use it on the // backend without revealing the actual value. obfuscate(w.witness.GetMethod()) diff --git a/trace/util.go b/trace/util.go index 321786e..7b30e89 100644 --- a/trace/util.go +++ b/trace/util.go @@ -2,9 +2,10 @@ package trace import pb "github.com/akitasoftware/akita-ir/go/api_spec" -// Determines whether a given method has error (4xx or 5xx) response codes. Returns true if the method has at least one response and all response codes are 4xx or 5xx. +// Determines whether a given method has only error (4xx or 5xx) response codes. +// Returns true if the method has at least one response and all response codes are 4xx or 5xx. // Here method will only have single response object because in agent each witness stores single request-response pair. -func hasErrorResponses(method *pb.Method) bool { +func hasOnlyErrorResponses(method *pb.Method) bool { responses := method.GetResponses() if len(responses) == 0 { return false From ebe4f900f478896c14927e02a8263d5c4334ebbd Mon Sep 17 00:00:00 2001 From: Mudit Joshi Date: Tue, 8 Oct 2024 23:31:51 +0530 Subject: [PATCH 5/5] chore: address PR comments --- trace/util.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/trace/util.go b/trace/util.go index 7b30e89..3995ac3 100644 --- a/trace/util.go +++ b/trace/util.go @@ -13,7 +13,7 @@ func hasOnlyErrorResponses(method *pb.Method) bool { for _, response := range responses { responseCode := response.Meta.GetHttp().GetResponseCode() - if responseCode < 400 { + if !(400 <= responseCode && responseCode < 600) { return false } }