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

fix(minipipeline): handle IP-addr URLs in classic linear analysis #1472

Merged
merged 6 commits into from
Jan 24, 2024

Conversation

bassosimone
Copy link
Contributor

@bassosimone bassosimone commented Jan 24, 2024

This diff fixes ooni/probe#1511 (comment).

This is the rationale of the diff: we need to track the origin of IP addresses:

  • "dns" if discovered using DNS;
  • "th" if discovered using the test helper;
  • null otherwise.

When filtering for classic analysis, we include "dns" entries if resolved using getaddrinfo, drop "th" entries because they're not relevant, and include null entries under the assumption that the probe discovered them either directly from the input URL or because a redirect redirected to an URL containing an IP address.

We also update the minipipeline test suite and show that the only changes are related to the test added by #1471.

@bassosimone bassosimone requested a review from hellais as a code owner January 24, 2024 16:21
@bassosimone bassosimone changed the title minipipeline: fix classic analysis with input URL containing IP addr fix(minipipeline): make classic linear analysis work with URL containing IP addr Jan 24, 2024
@bassosimone bassosimone changed the title fix(minipipeline): make classic linear analysis work with URL containing IP addr fix(minipipeline): make classic linear analysis WAI with IP-addr URL hostname Jan 24, 2024
@bassosimone bassosimone changed the title fix(minipipeline): make classic linear analysis WAI with IP-addr URL hostname fix(minipipeline): make classic linear analysis WAI with IP-addr URL Jan 24, 2024
@bassosimone bassosimone changed the title fix(minipipeline): make classic linear analysis WAI with IP-addr URL fix(minipipeline): make classic linear analysis WAI with IP addrs input Jan 24, 2024
@bassosimone bassosimone changed the title fix(minipipeline): make classic linear analysis WAI with IP addrs input fix(minipipeline): handle IP-addr input URL in classic analysis Jan 24, 2024
@bassosimone bassosimone changed the title fix(minipipeline): handle IP-addr input URL in classic analysis fix(minipipeline): handle IP-addr URLs in classic linear analysis Jan 24, 2024
"alt-svc": true,
"content-length": true
},
"Linear": [
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We clearly fixed the bug!

@@ -1,7 +1,75 @@
{
"DNSLookupFailures": [],
"DNSLookupSuccesses": [],
"KnownTCPEndpoints": {},
"KnownTCPEndpoints": {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We clearly fixed the bug!

@bassosimone bassosimone merged commit 2cc9231 into master Jan 24, 2024
11 checks passed
@bassosimone bassosimone deleted the issue/1511 branch January 24, 2024 16:35
Murphy-OrangeMud pushed a commit to Murphy-OrangeMud/probe-cli that referenced this pull request Feb 13, 2024
…ni#1472)

This diff fixes
ooni/probe#1511 (comment).

This is the rationale of the diff: we need to track the origin of IP
addresses:

* "dns" if discovered using DNS;
* "th" if discovered using the test helper;
* null otherwise.

When filtering for classic analysis, we include "dns" entries if
resolved using getaddrinfo, drop "th" entries because they're not
relevant, and include null entries under the assumption that the probe
discovered them either directly from the input URL or because a redirect
redirected to an URL containing an IP address.

We also update the minipipeline test suite and show that the only
changes are related to the test added by
ooni#1471.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

webconnectivity: investigate failure=nil and blocking=nil
1 participant