1486137 - TRR: wrong family define used in code sending the ECS

Status: RESOLVED FIXED

(Core :: Networking: DNS, enhancement, P2)

Description

[Daniel Stenberg [:bagder]]

A totally harmless mistake, but still:

In TRR.cpp we have this:

144:    aBody += AF_INET; // FAMILY, 2 octets

... but that family field does not use the AF defines, it uses the IANA family numbers registry [1] so by setting it to AF_INET (2) it actually means IPv6, while it should set it to (1) to mean IPv4.

Since the code is sending a zero size address, sending IPv6 or IPv4 doesn't matter but by using this constant, the code is misleading the reader.

[1] = https://www.iana.org/assignments/address-family-numbers/address-family-numbers.xhtml

Comment 1

[Daniel Stenberg [:bagder]]

Attachment: bug 1486137 - TRR: fix the FAMILY field in the ECS code

... since AF_INET wouldn't actually use IPv4!

MozReview-Commit-ID: FCf7psawENI

Comment 2

[Patrick McManus [:mcmanus]]

make this block on the original ecs bug and set the affected flags please.

Comment 3

[Patrick McManus [:mcmanus]]

Comment on attachment 9004197 [details]
bug 1486137 - TRR: fix the FAMILY field in the ECS code

Patrick McManus [:mcmanus] has approved the revision.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0192efea1b66
TRR: fix the FAMILY field in the ECS code r=mcmanus

Comment 5

[Raul Gurzau (:RaulG)]

Backed out changeset 0192efea1b66 (bug 1486137) for failing at netwerk/test/unit/test_trr.js on a CLOSED TREE

Backout link: https://hg.mozilla.org/integration/autoland/rev/a360d3279b9f2ac82a7e81f3878a6eff63a466e8

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=0192efea1b664db8d20a691401ec5d3f9d3a2122

Log link: https://treeherder.mozilla.org/logviewer.html#?job_id=196188366&repo=autoland&lineNumber=2106

Log snippet: 

[task 2018-08-28T13:19:41.850Z] 13:19:41     INFO -  TEST-START | netwerk/test/unit/test_trr.js
[task 2018-08-28T13:19:42.315Z] 13:19:42  WARNING -  TEST-UNEXPECTED-FAIL | netwerk/test/unit/test_trr.js | xpcshell return code: 0
[task 2018-08-28T13:19:42.316Z] 13:19:42     INFO -  TEST-INFO took 464ms
[task 2018-08-28T13:19:42.317Z] 13:19:42     INFO -  >>>>>>>
[task 2018-08-28T13:19:42.317Z] 13:19:42     INFO -  (xpcshell/head.js) | test MAIN run_test pending (1)
[task 2018-08-28T13:19:42.318Z] 13:19:42     INFO -  PID 13323 | start!
[task 2018-08-28T13:19:42.321Z] 13:19:42     INFO -  TEST-PASS | netwerk/test/unit/test_trr.js | run_test - [run_test : 18] "44462" != null
[task 2018-08-28T13:19:42.322Z] 13:19:42     INFO -  TEST-PASS | netwerk/test/unit/test_trr.js | run_test - [run_test : 19] "44462" != ""
[task 2018-08-28T13:19:42.322Z] 13:19:42     INFO -  (xpcshell/head.js) | test pending (2)
[task 2018-08-28T13:19:42.322Z] 13:19:42     INFO -  PID 13323 | starting test 0

Comment 6

[Daniel Stenberg [:bagder]]

That was indeed a very stupid mistake. Fixing and doing a new take soon.

Comment 7

[Daniel Stenberg [:bagder]]

Attachment: bug 1486137 - TRR: fix the FAMILY field in the ECS code

... since AF_INET wouldn't actually use IPv4!

MozReview-Commit-ID: FCf7psawENI

Comment 8

[Valentin Gosu [:valentin] (he/him)]

Comment on attachment 9005578 [details]
bug 1486137 - TRR: fix the FAMILY field in the ECS code

Valentin Gosu [:valentin] has approved the revision.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d808a15ac4bc
TRR: fix the FAMILY field in the ECS code r=valentin

Comment 10

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/d808a15ac4bc

Comment 11

[Daniel Stenberg [:bagder]]

Attachment: bug 1486137 - fix the stream ID check when counting TRR streams

MozReview-Commit-ID: DzOu1dRlWtg

Comment 12

[u408661]

Comment on attachment 9008656 [details]
bug 1486137 - fix the stream ID check when counting TRR streams

Nicholas Hurley [:nwgh][:hurley] (he/him) has approved the revision.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4dfb623bb11e
fix the stream ID check when counting TRR streams r=nwgh

Comment 14

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/4dfb623bb11e

Comment 15

[Daniel Stenberg [:bagder]]

Argh, this commit landed with the wrong bug number. It is for bug 1489121.

Comment 16

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/0fc842ad0629
Backed out changeset 4dfb623bb11e for landing with the wrong bug number. a=backout