Page MenuHomeFreeBSD

Changes to allow getaddrinfo(3) to distinguish error cases
ClosedPublic

Authored by karels on Oct 26 2022, 7:27 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Dec 11, 8:32 AM
Unknown Object (File)
Mon, Dec 9, 7:53 PM
Unknown Object (File)
Nov 19 2024, 7:18 AM
Unknown Object (File)
Nov 16 2024, 1:48 AM
Unknown Object (File)
Oct 9 2024, 1:47 AM
Unknown Object (File)
Sep 30 2024, 7:29 PM
Unknown Object (File)
Sep 21 2024, 11:59 AM
Unknown Object (File)
Sep 21 2024, 11:58 AM

Details

Reviewers
ume
bz
pauamma_gundo.com
Group Reviewers
network
manpages
Summary

Rework getaddrinfo(3) to return different error values for unresolvable
names (same as before, EAI_NONAME) and those without a requested addr
(EAI_ADDRFAMILY) when using DNS. This review collects changes that
are not expected to be in the same commit, but rather a series of 4.
I put them in one review to make the context clear. The expected commit
messages for those commits follow:


netdb.h: re-enable EAI_ADDRFAMILY, EAI_NODATA

EAI_ADDRFAMILY and EAI_NODATA are not in RFC 3493, but are available
and used in many other systems. It is desirable to have at least one
of them in order to distinguish between names that do not resolve and
those that do not have the requested address type. A change to
getaddrinfo() will use EAI_ADDRFAMILY. Both were "#if 0"; re-enable,
conditioned on __BSD_VISIBLE, and update comments.


gai_strerror.[c3]: re-enable EAI_ADDRFAMILY, EAI_NODATA

gai_strerror.c still has messages for EAI_ADDRFAMILY and EAI_NODATA,
but not the man page. Re-add to the man page, and update comments
in the source.


getaddrinfo: distinguish missing addrs from unresolvable names
[getaddrinfo.c, nsswitch.h]

Rework getaddrinfo(3) to return different error values for unresolvable
names (same as before, EAI_NONAME) and those without a requested addr
(EAI_ADDRFAMILY) when using DNS. This is implemented via an added
error in the nsswitch layer, NS_ADDRFAMILY, which is used only by
getaddrinfo(). The error is passed through nsdispatch(3), but that
routine has no changes to handle this error. The error originates in
the getaddrinfo DNS layer called via nsdispatch(), and is processed
by the search layer that calls nsdispatch().

While here, add a little style to returns in functions that were
modified.


fetch: support EAI_ADDRFAMILY error, correct two error messages

With the change to return EAI_ADDRFAMILY from getaddrinfo(), fetch
would print "Unknown resolver error" for that error. Add that error
and its string to libfetch's table, using an #ifdef just in case.
Correct error strings for EAI_NODATA (although it is currently unused)
and EAI_NONAME. Should maybe rework the code to use gai_strerror(3),
but that doesn't map directly, and the current strings are shortened.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 48112
Build 44999: arc lint + arc unit

Event Timeline

Can you take the extra () changes out of the diff and simply commit them right away as non-functional changes. Initially I thought I don't mind but it'll make the changes in the switch easier to see long-term.

PS: while not technically reviewed by your description I like the change.

There are a lot of "return foo" statements in getaddrinfo.c, and I only fixed those in routines that I was changing. I think splitting those into a separate commit would look pretty random.

(I hoe all the local comments were indeed deleted again; I had marked the non-functional changes so I wouldn't be confused. I still think adding the () in this file is noise and not a good idea as part of these commits).

I think anything but the two changes in getaddrinfo,c returning the obsolete return codes again is not questionable and fine.
I love the extra more helpful message in the fetch case.

My main concern is what "new" applications not expecting these two EAIs will do when receiving them? In terms of fault tolerant programming I'd say that a future extension bares the same problem so this should be fine as well. I'd probably be more explicit in the man page about them now being local extensions.

lib/libc/net/gai_strerror.3
95

I would probably add a sentence/subclause that we (still/again) support them as local extensions?

This revision is now accepted and ready to land.Oct 31 2022, 11:53 PM

Thanks. As a compromise on the parens, I think I'll take out the first three return changes, which are not as close to the "real" changes, and leave the ones within a couple of lines of a new or changed return. Is that acceptable?

I'll add some text about the symbols being gone and then back again. Note, they are not just local extensions; they are in NetBSD, OpenBSD and Linux as well. Also, there were a number of references to EAI_NODATA in base already; they were on #ifdefs, or (in one case), it was #undef'ed and then defined.

Any programs that are attempting to print errors should be using gai_strerror, or libfetch. It's possible that a program might be attempting to catch specific error cases, but there are a lot of them, so there should be a default.

I just noticed that the last three EAI_* errors are not in POSIX or RFC 34933. I will add a comment, but I think I should also add #if __BSD_VISIBLE. Any comments?

Changes:

  • add #if __BSD_VISIBLE to last three EAI errors in netdb.h
  • document which EAI errors are not in standards in gai_strerror.3
  • remove style changes in getaddrinfo.c that are not close to other changes
This revision now requires review to proceed.Nov 1 2022, 5:57 PM

Sounds good to me; I'd mention the other BSDs and Linux having the symbols in the commit message.

This revision is now accepted and ready to land.Nov 1 2022, 6:05 PM

Manual page changes LGTM, language-wise. Can't vouch for their consistency with implementation.

pushed in commits listed in previous comment