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.

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

Jump to bottom

dnsovergetaddrinfo: fill fake DNSResponse with CNAME #2226

Closed
bassosimone opened this issue Aug 23, 2022 · 0 comments · Fixed by ooni/probe-cli#876
Closed

dnsovergetaddrinfo: fill fake DNSResponse with CNAME #2226

bassosimone opened this issue Aug 23, 2022 · 0 comments · Fixed by ooni/probe-cli#876
Assignees
@bassosimone
Labels
enhancement New feature request or improvement to existing functionality ooni/probe-engine priority/high Important issue that needs attention soon user feedback requests that have been added to the backlog as a direct result of user feedback or testing

Comments

@bassosimone
Copy link
Contributor

The current dnsovergetaddrinfo.go code supports passing a CNAME over to the context based trace but does not fill the field yet. We now want to address this issue by properly filling the field. This issue is part of #1516.
@bassosimone bassosimone self-assigned this Aug 23, 2022
@bassosimone bassosimone added enhancement New feature request or improvement to existing functionality priority/high Important issue that needs attention soon user feedback requests that have been added to the backlog as a direct result of user feedback or testing ooni/probe-engine labels Aug 23, 2022
bassosimone added a commit to ooni/probe-cli that referenced this issue Aug 23, 2022
@bassosimone
feat(dnsovergetaddrinfo): collect the CNAME
0856640 
This diff modifies how dnsovergetaddrinfo.go works such that the
returned DNSResponse includes the CNAME.

Closes ooni/probe#2226.

While there, recognize that we can remove getaddrinfoLookupHost and
always call getaddrinfoLookupANY everywhere. (This simplification is
why we did #874.)
@bassosimone bassosimone mentioned this issue Aug 23, 2022
Merged
bassosimone added a commit to ooni/probe-cli that referenced this issue Aug 23, 2022
@bassosimone
feat(dnsovergetaddrinfo): collect the CNAME (#876)
080abf9 
* feat(dnsovergetaddrinfo): collect the CNAME

This diff modifies how dnsovergetaddrinfo.go works such that the
returned DNSResponse includes the CNAME.

Closes ooni/probe#2226.

While there, recognize that we can remove getaddrinfoLookupHost and
always call getaddrinfoLookupANY everywhere. (This simplification is
why we did #874.)

* fix: extra debugging because of failing CI

Everything is OK locally (on macOS). However, maybe things are a bit
different on GNU/Linux perhaps?

Here's the error:

```
--- FAIL: TestPass (0.11s)
    resolver_test.go:113: unexpected rcode
FAIL
coverage: 95.7% of statements
FAIL	github.com/ooni/probe-cli/v3/internal/cmd/jafar/resolver	0.242s
```

I'm a bit confused because jafar's resolver is _unrelated_. But actually this
error never occurred again after a committed the debugging diff.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bassosimone bassosimone

Labels
enhancement New feature request or improvement to existing functionality ooni/probe-engine priority/high Important issue that needs attention soon user feedback requests that have been added to the backlog as a direct result of user feedback or testing
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat(dnsovergetaddrinfo): collect the CNAME ooni/probe-cli
1 participant
@bassosimone