Skip to content

sock_dns: correctly report too short messages#10896

Merged
miri64 merged 1 commit intoRIOT-OS:masterfrom
miri64:sock_dns/fix/too-small-msg-error
Jan 29, 2019
Merged

sock_dns: correctly report too short messages#10896
miri64 merged 1 commit intoRIOT-OS:masterfrom
miri64:sock_dns/fix/too-small-msg-error

Conversation

@miri64
Copy link
Member

@miri64 miri64 commented Jan 29, 2019

Contribution description

While writing the tests I promised in #10740 I noticed this bug. sock_dns_query() returns the length of a too short DNS response instead of just reporting that it is wrong. While this is not severe, it can lead to confusing behavior, as the function does not report an error but just a number >0. This basically means, sock_dns_query() will tell the user that the address is whatever currently is on the stack ;-).

Testing procedure

After the interface configuration described in the README of tests/gnrc_sock_dns start the application (but not the dnsmasq) and the following command in a separate terminal

while sleep 1; do echo AACBAAAACpoAAAAA | base64 -d | sudo socat fd:0 udp6-sendto:'[<native-lladdr>%tap0]':49152,bind="[2001:db8::1]:53",sourceport=53; done

Without this PR the response the output will be

example.org resolves to ::

With it correctly reports

error resolving example.org

Issues/PRs references

None, but found while working on https://github.com/miri64/RIOT/tree/tests/enh/gnrc_sock_dns/tests/gnrc_sock_dns

@miri64 miri64 requested a review from kaspar030 January 29, 2019 18:17
@miri64 miri64 added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: network Area: Networking CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jan 29, 2019
@miri64 miri64 added this to the Release 2019.01 milestone Jan 29, 2019
@miri64 miri64 force-pushed the sock_dns/fix/too-small-msg-error branch from f75b654 to 20bcc16 Compare January 29, 2019 18:18
@miri64 miri64 removed the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jan 29, 2019
@kaspar030
Copy link
Contributor

LGTM, pls squash!

Copy link
Contributor

@kaspar030 kaspar030 left a comment

Choose a reason for hiding this comment

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

(untested) ACK.

@miri64 miri64 force-pushed the sock_dns/fix/too-small-msg-error branch from 1e47c6c to b30cdb5 Compare January 29, 2019 21:33
@miri64
Copy link
Member Author

miri64 commented Jan 29, 2019

Squashed

@kaspar030 kaspar030 added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jan 29, 2019
@miri64 miri64 merged commit 1481239 into RIOT-OS:master Jan 29, 2019
@miri64 miri64 deleted the sock_dns/fix/too-small-msg-error branch January 29, 2019 22:43
@miri64 miri64 added the Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch label Jan 29, 2019
@miri64
Copy link
Member Author

miri64 commented Jan 29, 2019

Backport provided in #10900

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: network Area: Networking CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants