TooTallNate / node-icy

Node.js module for parsing and/or injecting ICY metadata
MIT License
291 stars 48 forks source link

Added support for https #26

Closed angelnu closed 8 years ago

angelnu commented 8 years ago

Added support for https streams

angelnu commented 8 years ago

I made a mistake and used https in both cases: fixing and updating pull-request

angelnu commented 8 years ago

I also added tests for https

angelnu commented 8 years ago

I changed the test to rejectUnauthorized: false for the first test case. In the second one, my objective is to test the Client with an string as input so I have to change the global setting there.

angelnu commented 8 years ago

The reason for the second test is to actually test the string part of the code:

function Client (options, cb) {
  var req;
  if ((typeof options == 'string' && options.indexOf("https")==0) ||
      (typeof options == 'object' && options.protocol == "https:"))

Another option would be to do:

function Client (options, cb) {
  if (typeof options == 'string')
      options = url.parse(options)
   var req;
   if (options.protocol == "https:")

What is your preference?

TooTallNate commented 8 years ago

I'd prefer the url.parse() call to be in the Client constructor as well, which would more or less render the "string" test unnecessary.

angelnu commented 8 years ago

Right. I changed the client code and removed the second testcase.

TooTallNate commented 8 years ago

Sweet. Looks good to me. Thanks! I guess I never ran into an https icecast stream before. Is it a public one out of curiosity?

angelnu commented 8 years ago

Thanks for merging!

It is a private stream served by my home apache/icecast setup. This way I only have to open/secure one single port.

TooTallNate commented 8 years ago

v2.1.0 published to npm

On Wed, Dec 30, 2015 at 1:37 PM, vegetto notifications@github.com wrote:

Thanks for merging!

It is a private stream served by my home apache/icecast setup. This way I only have to open/secure one single port.

— Reply to this email directly or view it on GitHub https://github.com/TooTallNate/node-icy/pull/26#issuecomment-168078331.

angelnu commented 8 years ago

Thanks! Picked up the new version in my project at https://github.com/angelnu/ioBroker.chromecast