faye / websocket-driver-ruby

WebSocket protocol handler with pluggable I/O
Other
223 stars 43 forks source link

Doesn't seem to work with wss #87

Closed route closed 2 years ago

route commented 2 years ago

The example in the example folder doesn't work with browserless wss websocket:

require 'bundler/setup'
require 'websocket/driver'
require 'socket'
require 'uri'

class WSClient
  DEFAULT_PORTS = { 'ws' => 80, 'wss' => 443 }

  attr_reader :url, :thread

  def initialize(url)
    @url  = url
    @uri  = URI.parse(url)
    @port = @uri.port || DEFAULT_PORTS[@uri.scheme]

    @tcp  = TCPSocket.new(@uri.host, @port)
    @dead = false

    @driver = WebSocket::Driver.client(self)

    @driver.on(:open)    { |event| send "Hello world!" }
    @driver.on(:message) { |event| p [:message, event.data] }
    @driver.on(:close)   { |event| finalize(event) }

    @thread = Thread.new do
      @driver.parse(@tcp.read(1)) until @dead
    end

    @driver.start
  end

  def send(message)
    @driver.text(message)
  end

  def write(data)
    @tcp.write(data)
  end

  def close
    @driver.close
  end

  def finalize(event)
    p [:close, event.code, event.reason]
    @dead = true
    @thread.kill
  end
end

ws = WSClient.new(ARGV.first || "wss://chrome.browserless.io?token={token}")
ws.thread.join

failing with [:close, 1002, "Error during WebSocket handshake: Unexpected response code: 400"]

route commented 2 years ago

Instead the example should be something like:

class WSClient
  DEFAULT_PORTS = { 'ws' => 80, 'wss' => 443 }

  attr_reader :url, :thread

  def initialize(url)
    @url  = url
    @uri  = URI.parse(url)
    @port = @uri.port || DEFAULT_PORTS[@uri.scheme]

    if @port == 443
      tcp  = TCPSocket.new(@uri.host, @port)
      ssl_context = OpenSSL::SSL::SSLContext.new
      @sock = OpenSSL::SSL::SSLSocket.new(tcp, ssl_context)
      @sock.sync_close = true
      @sock.connect
    else
      @sock  = TCPSocket.new(@uri.host, @port)
    end

    @dead = false

    @driver = WebSocket::Driver.client(self)

    @driver.on(:open)    { |event| send "Hello world!" }
    @driver.on(:message) { |event| p [:message, event.data] }
    @driver.on(:close)   { |event| finalize(event) }

    @thread = Thread.new do
      @driver.parse(@sock.read(1)) until @dead
    end

    @driver.start
  end

  def send(message)
    @driver.text(message)
  end

  def write(data)
    @sock.write(data)
  end

  def close
    @driver.close
  end

  def finalize(event)
    p [:close, event.code, event.reason]
    @dead = true
    @thread.kill
  end
end

ws = WSClient.new(ARGV.first || "wss://chrome.browserless.io?token={token}")
ws.thread.join
jcoglan commented 2 years ago

This example intentionally doesn't implement TLS -- a decent implementation requires a lot more code than this and I'd rather not put a bad TLS example into the repository. It would be better if I just removed the dynamic port decision that implies it works with wss when it doesn't, like the EventMachine client example.

jcoglan commented 2 years ago

This is now fixed in 720a0f2.

ZilvinasKucinskas commented 1 year ago

@jcoglan Would you mind shedding some light on why the TLS example is bad? From the usability standpoint, wss example works - I can send and receive messages to WebSocket.

jcoglan commented 1 year ago

@ZilvinasKucinskas it lacks many of the things you need for a production TLS implementation. It might connect to a server and exchange messages successfully, but it's not secure. It doesn't perform certificate verification, it doesn't handle all sorts of edge cases and errors, and so on. Often it's fine for example code to be incomplete and not a full production implementation of something, but it's not great to have security-critical examples like this. Especially not if it's an implied recommended way of doing something by virtue of being in a repo's docs or examples. So, we removed it.

ZilvinasKucinskas commented 1 year ago

Thanks for the answer @jcoglan! So it seems that the issue is that we cannot trust the upstream when exchanging messages if I understood correctly. However, for some use cases such as scraping this might be acceptable.

As a side note, I have tried figuring out an example with a HTTP proxy but could not get it working.