c/ooni-probe-cli
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(webconnectivity): ignore any status code <= 0 (#579)

Browse Source 
This diff changes the algorithm used by webconnectivity's
httpanalysis.go to ignore any status code <= 0 rather
than just ignoring the == 0 case.

Make sure we add test cases for when the control's status
code is negative rather than being zero.

While there, simplify code where boolean checks could be
more compact according to staticcheck.

Closes https://github.com/ooni/probe/issues/1825
This commit is contained in:
Simone Basso 2021-11-05 13:51:22 +01:00 committed by GitHub
parent 5b8f4546f3
commit 3b27780836
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 82 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
internal/engine/experiment/webconnectivity/httpanalysis.go
View File

@ -80,14 +80,14 @@ func HTTPStatusCodeMatch(tk urlgetter.TestKeys, ctrl ControlResponse) (out *bool
return // no real status code return // no real status code
} }
measurement := tk.Requests[0].Response.Code measurement := tk.Requests[0].Response.Code
if control == 0 { if control <= 0 {
return // no real status code return // no real status code
} }
if measurement == 0 { if measurement <= 0 {
return // no real status code return // no real status code
} }
value := control == measurement value := control == measurement
if value == true { if value {
// if the status codes are equal, they clearly match // if the status codes are equal, they clearly match
out = &value out = &value
return return
@ -110,10 +110,10 @@ func HTTPHeadersMatch(tk urlgetter.TestKeys, ctrl ControlResponse) *bool {
if len(tk.Requests) <= 0 { if len(tk.Requests) <= 0 {
return nil return nil
} }
if tk.Requests[0].Response.Code == 0 { if tk.Requests[0].Response.Code <= 0 {
return nil return nil
} }
if ctrl.HTTPRequest.StatusCode == 0 { if ctrl.HTTPRequest.StatusCode <= 0 {
return nil return nil
} }
control := ctrl.HTTPRequest.Headers control := ctrl.HTTPRequest.Headers
@ -203,13 +203,13 @@ func HTTPTitleMatch(tk urlgetter.TestKeys, ctrl ControlResponse) (out *bool) {
return return
} }
response := tk.Requests[0].Response response := tk.Requests[0].Response
if response.Code == 0 { if response.Code <= 0 {
return return
} }
if response.BodyIsTruncated { if response.BodyIsTruncated {
return return
} }
if ctrl.HTTPRequest.StatusCode == 0 { if ctrl.HTTPRequest.StatusCode <= 0 {
return return
} }
control := ctrl.HTTPRequest.Title control := ctrl.HTTPRequest.Title

74
internal/engine/experiment/webconnectivity/httpanalysis_test.go
View File

@ -70,6 +70,25 @@ func TestHTTPBodyLengthChecks(t *testing.T) {
}, },
}, },
lengthMatch: nil, lengthMatch: nil,
}, {
name: "control length is negative",
args: args{
tk: urlgetter.TestKeys{
Requests: []archival.RequestEntry{{
Response: archival.HTTPResponse{
Body: archival.MaybeBinaryValue{
Value: randx.Letters(768),
},
},
}},
},
ctrl: webconnectivity.ControlResponse{
HTTPRequest: webconnectivity.ControlHTTPRequestResult{
BodyLength: -1,
},
},
},
lengthMatch: nil,
}, { }, {
name: "match with bigger control", name: "match with bigger control",
args: args{ args: args{
@ -249,6 +268,22 @@ func TestStatusCodeMatch(t *testing.T) {
}}, }},
}, },
}, },
}, {
name: "with response status code and -1 as control status code",
args: args{
tk: urlgetter.TestKeys{
Requests: []archival.RequestEntry{{
Response: archival.HTTPResponse{
Code: 200,
},
}},
},
ctrl: webconnectivity.ControlResponse{
HTTPRequest: webconnectivity.ControlHTTPRequestResult{
StatusCode: -1,
},
},
},
}, { }, {
name: "with only control status code and no response status code", name: "with only control status code and no response status code",
args: args{ args: args{
@ -340,6 +375,26 @@ func TestHeadersMatch(t *testing.T) {
ctrl: webconnectivity.ControlResponse{}, ctrl: webconnectivity.ControlResponse{},
}, },
want: nil, want: nil,
}, {
name: "with negative control status code",
args: args{
tk: urlgetter.TestKeys{
Requests: []archival.RequestEntry{{
Response: archival.HTTPResponse{
Headers: map[string]archival.MaybeBinaryValue{
"Date": {Value: "Mon Jul 13 21:10:08 CEST 2020"},
},
Code: 200,
},
}},
},
ctrl: webconnectivity.ControlResponse{
HTTPRequest: webconnectivity.ControlHTTPRequestResult{
StatusCode: -1,
},
},
},
want: nil,
}, { }, {
name: "with no uncommon headers", name: "with no uncommon headers",
args: args{ args: args{
@ -748,6 +803,25 @@ func TestTitleMatch(t *testing.T) {
}, },
}, },
wantOut: &trueValue, wantOut: &trueValue,
}, {
name: "when the control status code is negative",
args: args{
tk: urlgetter.TestKeys{
Requests: []archival.RequestEntry{{
Response: archival.HTTPResponse{
Code: 200,
Body: archival.MaybeBinaryValue{
Value: "<HTML><TiTLe>La commUNity di MSN</tITLE></HTML>"},
},
}},
},
ctrl: webconnectivity.ControlResponse{
HTTPRequest: webconnectivity.ControlHTTPRequestResult{
StatusCode: -1,
},
},
},
wantOut: nil,
}} }}
for _, tt := range tests { for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) { t.Run(tt.name, func(t *testing.T) {

2
internal/engine/experiment/webconnectivity/summary.go
View File

@ -82,7 +82,7 @@ type Summary struct {
// //
// Because of that, we must preserve the original behaviour. // Because of that, we must preserve the original behaviour.
func DetermineBlocking(s Summary) interface{} { func DetermineBlocking(s Summary) interface{} {
if s.Accessible != nil && *s.Accessible == true { if s.Accessible != nil && *s.Accessible {
return false return false
} }
return s.BlockingReason return s.BlockingReason