1200132 - [Presentation WebAPI] discovered host name is unresolvable.

Status: RESOLVED FIXED

(Core :: Networking: DNS, defect, P1)

Description

[Gary Chen [:xeonchen]]

The discovered host name (e.g. foo.local.) cannot be resolved to IP address on some platform (e.g. b2g).

Comment 1

[Sean Lin [:seanlin] (inactive from Mar 2016)]

Hi Josh,

Could you help mark 2.5+ to this one?

Comment 2

[Josh Cheng [:josh]]

Hi Sean,
Should we block this on 1184073?
Thanks

Comment 3

[Sean Lin [:seanlin] (inactive from Mar 2016)]

(In reply to Josh Cheng [:josh] from comment #2)
> Hi Sean,
> Should we block this on 1184073?
> Thanks
Yep. I just did it. Thanks.

Comment 4

[Kershaw Chang [:kershaw]]

Attachment: WIP: B2G part

Comment 5

[Kershaw Chang [:kershaw]]

Attachment: WIP: Fennec part

Comment 6

[Kershaw Chang [:kershaw]]

Attachment: Get address info after service resolved

Comment 7

[Richard Newman [:rnewman]]

Comment on attachment 8664156 [details] [diff] [review]
WIP: Fennec part

Review of attachment 8664156 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/base/mdns/MulticastDNSManager.java
@@ +205,5 @@
>  
>          InetAddress host = serviceInfo.getHost();
>          if (host != null) {
> +            obj.put("host", host.getCanonicalHostName());
> +            obj.put("isIPV6", String.valueOf(host instanceof Inet6Address));

JSONObject supports booleans, so remove the String.valueOf.

Comment 8

[Kershaw Chang [:kershaw]]

Attachment: Use adderss to create socketTransport

WIP patch.
Waiting for validation.

Comment 9

[Kershaw Chang [:kershaw]]

Hi SC,

Could you please take a look?

Thanks.

Comment 10

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Comment on attachment 8670118 [details] [diff] [review]
Use adderss to create socketTransport

Review of attachment 8670118 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/presentation/interfaces/nsITCPPresentationServer.idl
@@ +63,5 @@
>    nsIPresentationDevice createTCPDevice(in AUTF8String aId,
>                                          in AUTF8String aName,
>                                          in AUTF8String aType,
>                                          in AUTF8String aHost,
> +                                        in AUTF8String aAddress,

I'll prefer passing |nsINetAddr| directly.

Comment 11

[Kershaw Chang [:kershaw]]

Attachment: Use adderss to create socketTransport - v2

Summary:
1. Use nsINetAddr instead of string.
2. Modify test case.

Comment 12

[Junior [inactive]]

Comment on attachment 8672530 [details] [diff] [review]
Use adderss to create socketTransport - v2

Review of attachment 8672530 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with the comment addressed

::: dom/presentation/provider/TCPPresentationServer.js
@@ +175,5 @@
>      let socketTransport;
>      try {
>        socketTransport = sts.createTransport(null,
>                                              0,
> +                                            aDevice.address.address,

I'm wondering if it's good to keep |host| in the |nsIPresentationDevice|. If you'd like to do this way, I guess the client can decide to choose address or host. And the socket transport should connect to one of those. Connecting to the address is preferred, and connect to the host if address is null or undefined.

Otherwise, if you'd like to remove |host|, please file a bug as a follow-up.

Also put those to comments.

Comment 13

[Junior [inactive]]

Per bug 1136565, |host| might be used.

Comment 14

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Attachment: Use adderss to create socketTransport - v3

This is the updated patch based on the modification in bug 1136565. I now realized that TCPPresentationServer only need a string for host address to establish TCP socket. You might consider passing string from nsIDNSServiceDiscovery instead of nsINetAddr. It'll make your code simpler and less type conversion.

Comment 15

[Kershaw Chang [:kershaw]]

Comment on attachment 8675283 [details] [diff] [review]
Use adderss to create socketTransport - v3

Review of attachment 8675283 [details] [diff] [review]:
-----------------------------------------------------------------

lgtm.
I think I should land this bug until bug 1136565 is fixed?

Comment 16

[Kershaw Chang [:kershaw]]

Attachment: Part1: Get address info after service resolved, r=mcmanus

Rebase

Comment 17

[Kershaw Chang [:kershaw]]

Attachment: Part2: Fennec modification, r=mcmanus, rnewman

Comment 18

[Kershaw Chang [:kershaw]]

Attachment: Part1: Get address info after service resolved, r=mcmanus

Comment 19

[Kershaw Chang [:kershaw]]

Attachment: Part2: Fennec modification, r=mcmanus, rnewman

Comment 20

[Kershaw Chang [:kershaw]]

Attachment: Part3: Use adderss to create socketTransport - v4

Hi SC and Junior,

I'd like you to review it again. Thanks.

Comment 21

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Comment on attachment 8676672 [details] [diff] [review]
Part3: Use adderss to create socketTransport - v4

Review of attachment 8676672 [details] [diff] [review]:
-----------------------------------------------------------------

lgtm

Comment 22

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Comment on attachment 8676672 [details] [diff] [review]
Part3: Use adderss to create socketTransport - v4

Review of attachment 8676672 [details] [diff] [review]:
-----------------------------------------------------------------

Please add r=schien,junior in commit log before check in.

Comment 23

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

I also noticed that the commit log in patch part 1 & 2 are not meaningful. Please reflect your intention in commit log more clearly before check in.

Comment 24

[Junior [inactive]]

Comment on attachment 8676672 [details] [diff] [review]
Part3: Use adderss to create socketTransport - v4

Review of attachment 8676672 [details] [diff] [review]:
-----------------------------------------------------------------

r=me

Comment 25

[Kershaw Chang [:kershaw]]

Attachment: Part1: Get address info after service resolved, r=mcmanus

Update commit log.

Comment 26

[Kershaw Chang [:kershaw]]

Attachment: Part2: Get IP address by calling InetAddress.getHostAddress(), r=mcmanus, rnewman

Comment 27

[Kershaw Chang [:kershaw]]

Attachment: Part3: Use adderss to create socketTransport, r=schien, junior

Carry reviewer's name.

Comment 28

[Kershaw Chang [:kershaw]]

Try run:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=20457ec4d84c

Comment 29

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e4d008a5c6ab
https://hg.mozilla.org/integration/mozilla-inbound/rev/b26934635fa1
https://hg.mozilla.org/integration/mozilla-inbound/rev/dd0747ee0782

Comment 30

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/e4d008a5c6ab
https://hg.mozilla.org/mozilla-central/rev/b26934635fa1
https://hg.mozilla.org/mozilla-central/rev/dd0747ee0782