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

#21900: Fallback to local DNS when no other nameservers are known #273

Closed
wants to merge 12 commits into from

Conversation

rl1987
Copy link
Contributor

@rl1987 rl1987 commented Aug 13, 2018

@coveralls
Copy link

coveralls commented Aug 13, 2018

Coverage Status

Coverage increased (+2.5%) to 62.007% when pulling 444930e on rl1987:bug21900_hax into 57d0b8c on torproject:master.

return 0;
}

if (st.st_size == 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

If the stat() call failed above, then at this point, st.st_size will be uninitialized, and we're not allowed to look at it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Checking if stat() succeeded in 770aee4.

#endif /* defined(dns_option_hostsfile) && defined(use_libseccomp) */

if (!missing_resolv_conf) {
log_info(LD_EXIT, "parsing resolver configuration in '%s'", conf_fname);
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like this log message, and the messages after it, are still lower-cased in the latest version of the branch?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reverted the last changes to lowercase in 917eeed.

resolv_conf_fname = tor_strdup(conf_fname);
resolv_conf_mtime = st.st_mtime;
} else {
log_warn(LD_EXIT, "Could not read your resolv.conf - "
Copy link
Contributor

Choose a reason for hiding this comment

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

This message should say what actual filename it tried to read.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Logging filename in a1656c9.

}

tor_free(resolv_conf_fname);
Copy link
Contributor

Choose a reason for hiding this comment

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

This tor_free() here will clear the value of rsolv_conf_fname that you set above...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed - 4c1913a.

log_warn(LD_EXIT, "unable to find any nameservers in '%s'.",
conf_fname);
}
resolv_conf_fname = tor_strdup(conf_fname);
Copy link
Contributor

Choose a reason for hiding this comment

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

If resolv_conf_fname was already set, this will leak memory

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nmathewson
Copy link
Contributor

Closing in favor of PR 425

@nmathewson nmathewson closed this Oct 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants