Skip to content
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.

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

webconnectivity: Go implementation wrongly handles the TH's HTTP status code #1825

Closed
bassosimone opened this issue Oct 18, 2021 · 0 comments · Fixed by ooni/probe-cli#579
Closed
Assignees
Labels
bug Something isn't working data quality

Comments

@bassosimone
Copy link
Contributor

bassosimone commented Oct 18, 2021

The check should be <= 0 rather than == 0 because the old TH uses a -1 status code on HTTP failure. (Unfortunately, this specific aspect is not mentioned inside the spec; checking for failure != null would have been better.)

This happens throughout httpanalysis.go.

@bassosimone bassosimone added bug Something isn't working data quality labels Oct 18, 2021
@bassosimone bassosimone self-assigned this Oct 18, 2021
@bassosimone bassosimone added this to the Sprint 50 - Amphipoda milestone Oct 18, 2021
@bassosimone bassosimone changed the title webconnectivity: Go implementation wrongly handles the TH status code webconnectivity: Go implementation wrongly handles the TH's HTTP status code Oct 18, 2021
@bassosimone bassosimone removed this from the Sprint 50 - Amphipoda milestone Oct 22, 2021
bassosimone added a commit to ooni/probe-cli that referenced this issue Nov 5, 2021
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 ooni/probe#1825
bassosimone added a commit to ooni/spec that referenced this issue Nov 5, 2021
bassosimone added a commit to ooni/probe-cli that referenced this issue Nov 5, 2021
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 ooni/probe#1825
bassosimone added a commit to ooni/probe-cli that referenced this issue Nov 5, 2021
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 ooni/probe#1825

This backports 3b27780 from `master`.
bassosimone added a commit to ooni/probe-cli that referenced this issue Nov 5, 2021
…580)

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 ooni/probe#1825

This backports 3b27780 from `master`.
ainghazal pushed a commit to ainghazal/probe-cli that referenced this issue Mar 8, 2022
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 ooni/probe#1825
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working data quality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant