Get rid of CoffeeScript from the JS proxy

The semantics are a barrier to contribution and good parts are all upstreamed in ES2015 and beyond.

added component::circumvention/snowflake owner::arlolra priority::medium resolution::fixed reviewer::dcf severity::normal status::closed type::defect labels

I'm in agreement with this idea.

One proposal for how to do it is: take the current CoffeeScript-generated JS, run it through a pretty-printer, and commit. Then do a second manual commit for formatting and refactoring to remove autogeneration-isms. This way, by looking at the diffs, we can be reasonably sure that the port from CoffeeScript to JS doesn't change any semantics.

There's also the build and test suite runner to think about, which is currently using a Cakefile, https://gitweb.torproject.org/pluggable-transports/snowflake.git/tree/proxy/Cakefile

Trac:
Status: new to assigned
Owner: N/A to arlolra

Here's a WIP branch that mostly gets this done, https://github.com/keroserene/snowflake/commits/coffee

Still a few loose ends to tie up but some early review would go a long way since rebasing this is going to be nightmare if we make any significant changes.

Thanks

Trac:
Status: assigned to needs_review

Still a few loose ends to tie up

Ok, taking this out of WIP

Trac:
Reviewer: N/A to dcf

Great job on this. You covered some details that I would have missed. I looked at 4b97c9d524268f5e1242d0e9b537a4dc6486c01f. Just a couple of questions.

1. b5934883d4 "Don't overwrite global location": The commit also adds checks for snowflake !== null. Are those related to the location change, or could they be split out?
2. d1af00da67 "Don't specify port for stun server": How was this working before? Was there a hardcoded default inside libwebrtc?

I did the CoffeeScript rebuild step myself starting from 60fe1ae18b^ and got the same output as your 60fe1ae18b.

In order to download the newer version of CoffeeScript needed, I had to modify package.json. It might be worth sneaking in a commit to this effect, as documentation in the log.

-    "coffee-script": "^1.12.2",
+    "coffeescript": "^2",

Trac:
Status: needs_review to needs_information

1. b5934883d4 "Don't overwrite global location": The commit also adds checks for snowflake !== null. Are those related to the location change, or could they be split out?

What was happening was the location bug would throw before window.init initialized snowflake and so window.unload would be called with it still as null. I can split them apart if you prefer since they're only related in that the one surfaced the other.

2. d1af00da67 "Don't specify port for stun server": How was this working before? Was there a hardcoded default inside libwebrtc?

It wasn't working before because, prior to 7f5cd81, it wasn't being set at all. Fixing that "broke" things.

For what it's worth though, I don't think removing the port actually fixes anything. It probably makes the url invalid in some way that it falls back to the default you alluded to in libwebrtc.

My investigation sort of ended when discovering it was that commit that was preventing this work from going through. We're going to look at it in more detail in comment:10:ticket:31100

I did the CoffeeScript rebuild step myself starting from 60fe1ae18b^ and got the same output as your 60fe1ae18b.

In order to download the newer version of CoffeeScript needed, I had to modify package.json. It might be worth sneaking in a commit to this effect, as documentation in the log. {{{#!diff

• "coffee-script": "^1.12.2",

• "coffeescript": "^2", }}}

Sure, the commit message for 60fe1ae does mention this though.

I looked at ​4b97c9d524268f5e1242d0e9b537a4dc6486c01f. Just a couple of questions.

Here's a newly rebased branch with the changes you requested, https://github.com/keroserene/snowflake/commits/c2

Trac:
Status: needs_information to needs_review

All good.

Trac:
Status: needs_review to merge_ready

Here's the diff of the merge, https://gitweb.torproject.org/pluggable-transports/snowflake.git/diff/?id=fab39ae57c8c8ba0a65ec381e8b9678468e45164&id2=dcea8b74fae5590a2c76d2964b5097be070f0c3a

Trac:
Status: merge_ready to closed
Resolution: N/A to fixed

closed

mentioned in issue #31087 (moved)

moved to tpo/anti-censorship/pluggable-transports/snowflake#30933 (closed)