Bug 187302 - [libwebrtc] Allow IP mismatch for local connections on localhost
Summary: [libwebrtc] Allow IP mismatch for local connections on localhost
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebRTC (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Thibault Saunier
URL:
Keywords: InRadar
Depends on:
Blocks: 186932
  Show dependency tree / graph
 
Reported: 2018-07-03 14:31 PDT by Thibault Saunier
Modified: 2018-09-21 08:31 PDT (History)
6 users (show)

See Also:

Attachments
Patch (1.76 KB, patch)
2018-07-03 14:34 PDT, Thibault Saunier 		no flags Details | Formatted Diff | Diff
Patch (1.78 KB, patch)
2018-07-03 14:37 PDT, Thibault Saunier 		no flags Details | Formatted Diff | Diff
Patch (1.96 KB, patch)
2018-09-21 06:01 PDT, Thibault Saunier 		no flags Details | Formatted Diff | Diff
Patch (2.24 KB, patch)
2018-09-21 06:26 PDT, Thibault Saunier 		no flags Details | Formatted Diff | Diff
Show Obsolete (4) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Thibault Saunier 2018-07-03 14:31:37 PDT 
[libwebrtc] Allow IP mismatch for local connections on localhost
Comment 1 Thibault Saunier 2018-07-03 14:34:23 PDT 
Created attachment 344219 [details]
Patch
Comment 2 Thibault Saunier 2018-07-03 14:37:15 PDT 
Created attachment 344221 [details]
Patch
Comment 3 youenn fablet 2018-07-03 14:58:23 PDT 
Change makes sense, I guess it would make sense in the upstream repo as well.
Should we also consider the IPIsAny check?
Comment 4 Thibault Saunier 2018-07-03 15:05:39 PDT 
(In reply to youenn fablet from comment #3)
> Change makes sense, I guess it would make sense in the upstream repo as well.

I guess so yes, I will look into that.

> Should we also consider the IPIsAny check?

It is a good question, in the failure I was seeing `desired_addresses` was "any" fwiw. Do you want me to add it or let it the way it is and add it only if required in the future?
Comment 5 youenn fablet 2018-07-03 15:28:35 PDT 
Comment on attachment 344221 [details]
Patch

LGTM, let's land it as is for now.
If you have time to upstream this change, please also discuss the IPIsAny point at the same time.
Comment 6 WebKit Commit Bot 2018-07-04 08:47:28 PDT 
Comment on attachment 344221 [details]
Patch

Clearing flags on attachment: 344221

Committed r233506: <https://trac.webkit.org/changeset/233506>
Comment 7 WebKit Commit Bot 2018-07-04 08:47:30 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 8 Radar WebKit Bug Importer 2018-07-04 08:48:19 PDT 
<rdar://problem/41821903>
Comment 9 Thibault Saunier 2018-09-21 06:01:19 PDT 
Reopening to attach new patch.
Comment 10 Thibault Saunier 2018-09-21 06:01:21 PDT 
Created attachment 350350 [details]
Patch
Comment 11 Thibault Saunier 2018-09-21 06:01:51 PDT 
This patch was reverted during the last libwebrtc update.
Comment 12 Alejandro G. Castro 2018-09-21 06:18:31 PDT 
Comment on attachment 350350 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=350350&action=review

> Source/ThirdParty/libwebrtc/Source/webrtc/p2p/base/tcpport.cc:-354
> -    RTC_DCHECK(std::find_if(desired_addresses.begin(), desired_addresses.end(),

I think we can use the webkit compilation define WEBRTC_WEBKIT_BUILD, that way we can try to avoid the patch is removed again in the next merge. At least until we can merge upstream.
Comment 13 Thibault Saunier 2018-09-21 06:26:34 PDT 
Created attachment 350352 [details]
Patch
Comment 14 Thibault Saunier 2018-09-21 06:29:42 PDT 
New version of the patch has been proposed on https://bugs.webkit.org/show_bug.cgi?id=189828 as asked (so we are required to have 1 patch per bug)
Comment 15 youenn fablet 2018-09-21 08:31:57 PDT 
(In reply to Thibault Saunier from comment #11)
> This patch was reverted during the last libwebrtc update.

Yes, sorry.
We will need to come up with a better way of keeping these changes in our trunk when resyncing libwebrtc.