-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Update loopback.js #666
base: master
Are you sure you want to change the base?
Update loopback.js #666
Conversation
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 all the way to the end of loopbackAnswer, and when replaced for an empty string it corrupts the json. 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).
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). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
@googlebot I signed it! |
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
Wrong fix, I think. |
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... |
Thanks Philipp! |
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).