Skip to content

sock_dns: correctly report too short messages [backport 2019.01]#10900

Merged
aabadie merged 1 commit intoRIOT-OS:2019.01-branchfrom
miri64:backport/2019.01/sock_dns/fix/too-small-msg-error
Jan 30, 2019
Merged

sock_dns: correctly report too short messages [backport 2019.01]#10900
aabadie merged 1 commit intoRIOT-OS:2019.01-branchfrom
miri64:backport/2019.01/sock_dns/fix/too-small-msg-error

Conversation

@miri64
Copy link
Member

@miri64 miri64 commented Jan 29, 2019

Backport of #10896

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 added 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: release backport Integration Process: The PR is a release backport of a change previously provided to master Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) labels Jan 29, 2019
@miri64 miri64 added this to the Release 2019.01 milestone Jan 29, 2019
@miri64 miri64 requested review from aabadie and kaspar030 January 29, 2019 22:52
@aabadie
Copy link
Contributor

aabadie commented Jan 30, 2019

is it normal that we have a big green merge button without having a reviewer ACK ? My guess is that a configuration is missing for the 2019.01-branch protection.

@miri64
Copy link
Member Author

miri64 commented Jan 30, 2019

is it normal that we have a big green merge button without having a reviewer ACK ? My guess is that a configuration is missing for the 2019.01-branch protection.

You are the release manager. Did you follow https://github.com/RIOT-OS/RIOT/wiki/%5Bdraft%5D-Managing-a-Release. It told you to do it ;-).

@miri64
Copy link
Member Author

miri64 commented Jan 30, 2019

You are the release manager. Did you follow https://github.com/RIOT-OS/RIOT/wiki/%5Bdraft%5D-Managing-a-Release. It told you to do it ;-).

It might be, that you need special permission for that though :-/. Maybe that's a bug in the script that it does not warn you, if you used that.

@miri64
Copy link
Member Author

miri64 commented Jan 30, 2019

Just noticed that GitHub now also allows to set branch-protection based on a pattern that matches the branch-name. I added the rule for the [0-9][0-9][0-9][0-9].[0-9][0-9]-branch pattern ;-).

@aabadie
Copy link
Contributor

aabadie commented Jan 30, 2019

Maybe that's a bug in the script that it does not warn you, if you used that.

I'm a lazy guy so I used that very useful script :)
I think only administrators of the project can change this kind of settings.

I added the rule for the [0-9][0-9][0-9][0-9].[0-9][0-9]-branch pattern

Thanks !

Copy link
Contributor

@aabadie aabadie left a comment

Choose a reason for hiding this comment

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

Changes are the same as in original PR.

ACK and go.

@aabadie aabadie merged commit 7883f15 into RIOT-OS:2019.01-branch Jan 30, 2019
@miri64 miri64 deleted the backport/2019.01/sock_dns/fix/too-small-msg-error branch January 30, 2019 12:59
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: release backport Integration Process: The PR is a release backport of a change previously provided to master 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