apprtc icon indicating copy to clipboard operation
apprtc copied to clipboard

Update loopback.js

Open jackzelenkin opened this issue 5 years ago • 6 comments

Description On a Mac, loopbackAnswer contains \r\n characters instead of proper line breaks. Because of that initial regex /a=crypto:[1-9]+ .*/g matches everything starting from a=crypto:1 and all the way to the end of loopbackAnswer, and when replaced for an empty string it corrupts the json.

Solution As a solution, I propose to first search-and-replace "mac-specific" crypto patterns (crypto all the way to first \r\n sequence) and only after that search-and-replace original patterns (crypto all the way to the end of the line).

jackzelenkin avatar Jun 17 '20 13:06 jackzelenkin

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

:memo: Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

googlebot avatar Jun 17 '20 13:06 googlebot

@googlebot I signed it!

jackzelenkin avatar Jun 17 '20 13:06 jackzelenkin

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

googlebot avatar Jun 17 '20 13:06 googlebot

Wrong fix, I think. \r\n is the specified line termination character in SDP, \n is exceptionally (but universally) accepted. @fippo should look at this - either the pattern leaves a blank line (just the \r\n or \n), or it eats the \n when there's no \r and doesn't eat it when it's present. I think you need extra regex modifiers to get substitutes to work past linebreaks (of any type).

alvestrand avatar Jun 17 '20 14:06 alvestrand

The code takes loopbackAnswer as wssMessage.msg which is the serialized JSON which escapes all the SDP crlfs as \r\n. That is a bit silly and unexpected... And yes, the pattern leaves blank lines, thank you! #667 should fix both

fippo avatar Jun 17 '20 14:06 fippo

Thanks Philipp!

jackzelenkin avatar Jun 17 '20 14:06 jackzelenkin