Skip to content

net: getaddrinfo: Query both IPv4 and IPv6 if hints is missing #16772

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

Merged

Conversation

jukkar
Copy link
Member

@jukkar jukkar commented Jun 12, 2019

We must query both IPv4 and IPv6 addresses if the hints parameter
is NULL i.e., user does not supply hints.

Fixes #16453

Signed-off-by: Jukka Rissanen [email protected]

@pfalcon
Copy link
Collaborator

pfalcon commented Jun 12, 2019

net: getaddrinfo: Query both IPv4 and IPv6 if hints is missing

Nitpick about commit message: fairly speaking, the matter should be that IPv4 and IPv6 should be both queried if hints has AF_UNSPEC address family (and that's default if hints is NULL at all).

Copy link
Collaborator

@pfalcon pfalcon left a comment

Choose a reason for hiding this comment

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

Thanks for picking this up. To avoid future confusion when working with this func, would be nice to add one, better both, of: a) code comments; b) testcase for this behavior.

As you know, I never insist on b), though that's a perfect example where we need this... (Again, not insisting.)

@jukkar jukkar force-pushed the bug-16453-getaddrinfo-af-unspec branch from 7c66f8f to 849bdf4 Compare June 13, 2019 11:49
@jukkar
Copy link
Member Author

jukkar commented Jun 13, 2019

Updated according to comments.

@jukkar jukkar force-pushed the bug-16453-getaddrinfo-af-unspec branch from 849bdf4 to b93ef5b Compare June 13, 2019 11:50
@zephyrbot zephyrbot added the area: Tests Issues related to a particular existing or missing test label Jun 13, 2019
Copy link
Collaborator

@pfalcon pfalcon left a comment

Choose a reason for hiding this comment

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

Thanks for the updates, looks great!

Removing platform_whitelist was a bit too brave and hits the same issue patch from @andrewboie hit earlier. Better to skip that change for now I guess.

@jukkar
Copy link
Member Author

jukkar commented Jun 13, 2019

Removing platform_whitelist was a bit too brave and hits the same issue patch from @andrewboie hit earlier. Better to skip that change for now I guess.

I have been removing the whitelists in various PRs. Created an issue yesterday for this one #16778

@jukkar jukkar force-pushed the bug-16453-getaddrinfo-af-unspec branch from b93ef5b to 982a407 Compare June 20, 2019 08:07
@jukkar
Copy link
Member Author

jukkar commented Jun 20, 2019

Rebased against latest master.

@pfalcon
Copy link
Collaborator

pfalcon commented Jun 20, 2019

As this no longer contains any changes not described in the topic of the PR, I'd suggest to merge it. (I guess @tbursztyka is on vacation, nor this is his known area of interest.)

@jukkar
Copy link
Member Author

jukkar commented Jun 20, 2019

recheck

jukkar added 2 commits June 20, 2019 13:34
We must query both IPv4 and IPv6 addresses if the hints parameter
is NULL i.e., user does not supply hints or if family is set to
AF_UNSPEC.

Fixes zephyrproject-rtos#16453

Signed-off-by: Jukka Rissanen <[email protected]>
If user supplies AF_UNSPEC, we need to do two queries, one for
IPv4 A record and one for IPv6 AAAA record.

Signed-off-by: Jukka Rissanen <[email protected]>
@jukkar jukkar force-pushed the bug-16453-getaddrinfo-af-unspec branch from 982a407 to ee8e8ab Compare June 20, 2019 10:34
@jukkar jukkar merged commit 2c4b2a1 into zephyrproject-rtos:master Jun 20, 2019
@jukkar jukkar deleted the bug-16453-getaddrinfo-af-unspec branch June 20, 2019 11:45
@backporting
Copy link

backporting bot commented Jun 20, 2019

The backport to v1.14-branch failed:

Commits ["cb52096f2cda2f9b69c73f4c5efab0aa1a30efe8","ee8e8abe4e05ae0aeab10c43705f41970be31172"] could not be cherry-picked on top of v1.14-branch

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub.
git fetch
# Create new working tree.
git worktree add .worktrees/backport v1.14-branch
# Navigate to the new directory.
cd .worktrees/backport
# Cherry-pick all the commits of this pull request and resolve the likely conflicts.
git cherry-pick cb52096f2cda2f9b69c73f4c5efab0aa1a30efe8 ee8e8abe4e05ae0aeab10c43705f41970be31172
# Create a new branch with these backported commits.
git checkout -b backport-16772-to-v1.14-branch
# Push it to GitHub.
git push --set-upstream origin backport-16772-to-v1.14-branch
# Go back to the original working tree.
cd ../..
# Delete the working tree.
git worktree remove .worktrees/backport

Then, create a pull request where the base branch is v1.14-branch and the compare/head branch is backport-16772-to-v1.14-branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Networking area: Tests Issues related to a particular existing or missing test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sockets: getaddrinfo: AF_UNSPEC handling was recently broken
3 participants