middleman / middleman-livereload

LiveReload support for Middleman
http://middlemanapp.com
MIT License
117 stars 39 forks source link

Support secure sockets (WSS) #97

Closed defeated closed 7 years ago

defeated commented 7 years ago

We're using the --https option of Middleman 4, but also need to pass the SSL cert & key to the websocket server being run by middleman-livereload.

This PR adds support for :ssl_certificate & :ssl_private_key options (reusing the same names from middleman-core) and passes them to the EventMachine::WebSocket::Connection arguments in this format:

https://github.com/igrigorik/em-websocket#secure-server

(I confirmed this is "working on my machine", but this plugin doesn't have a test suite, so I regrettably have not added new tests. I hope that's OK!)

defeated commented 7 years ago

@tdreyno let me know if you prefer these squashed down, or if there's anything I can do to grease the wheels of progress ;) 🎩 Thanks!

defeated commented 7 years ago

(I should mention, initially I thought these settings could be inherited from the app but those :ssl_* keys in app.config return nil inside Middleman::LiveReloadExtension#new, so I'm not sure at what point they get merged w/ the CLI options)

defeated commented 7 years ago

@sandstrom thanks so much for the feedback!

As I made subsequent changes, all the wss_* references smelled a bit like "fields not defining state", so I've extracted it all to a new Middleman::LiveReloader::Wss class.

I also saw rspec was included as a dependency, so I added some specs at least around the new Wss class behavior.

(And then squashed everything into a single commit)

Let me know what you think?

defeated commented 7 years ago

@sandstrom this should include all feedback now!

sandstrom commented 7 years ago

@defeated Awesome!

Thanks for spending the time on the adjustments too. Merging! ⛵️

tdreyno commented 7 years ago

Nice. Thanks @defeated !