Skip to content

Commit

Permalink
Add support for non-binary WebMKS websocket
Browse files Browse the repository at this point in the history
Until now, 'binary' protocol for websockets was assumed. But VMware vCloud
console access only works via legacy 'uint8utf8' protocol. To make it even
more interesting, vCloud's JavaScript SDK actually thinks it supports 'binary',
but in fact it doesn't. Well 'binary' protocol works when connecting to vCenter
(i.e. `Vmware::InfraManager`), but not for vCloud (`Vmware::CloudManager`).

With this commit we implement websocket proxy client for 'uint8utf8' protocol
and remove 'binary' from list of offered protocols when 'uint8utf8' is used to
make sure JavaScript SDK doesn't overestimate itself (and then fail).

Signed-off-by: Miha Pleško <[email protected]>
  • Loading branch information
miha-plesko committed Mar 28, 2018
1 parent 1d647ba commit 05d8575
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 4 deletions.
17 changes: 14 additions & 3 deletions lib/websocket_proxy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,11 @@ def initialize(env, console, logger)
secure = Rack::Request.new(env).ssl?
scheme = secure ? 'wss:' : 'ws:'
@url = scheme + '//' + env['HTTP_HOST'] + env['REQUEST_URI']
@driver = WebSocket::Driver.rack(self, :protocols => %w(binary))

# VMware vCloud WebMKS SDK (console access) grabs 'binary' if offered, but then fails because in fact it's not
# able to use it :) We workaround this by only forcing the 'uint8utf8' protocol which actually works.
protocol = console.protocol.to_s.end_with?('uint8utf8') ? 'uint8utf8' : 'binary'
@driver = WebSocket::Driver.rack(self, :protocols => [protocol])

begin
# Hijack the socket from the Rack middleware
Expand All @@ -24,6 +28,8 @@ def initialize(env, console, logger)
@console.ssl ? WebsocketSSLSocket : WebsocketSocket
when 'webmks'
WebsocketWebmks
when 'webmks-uint8utf8'
WebsocketWebmksUint8utf8
end
@right = adapter.new(@sock, @console)
rescue => ex
Expand All @@ -33,7 +39,12 @@ def initialize(env, console, logger)

@driver.on(:open) { @console.update(:opened => true) }

@driver.on(:message) { |msg| @right.issue(msg.data.pack('C*')) }
# TODO: Move binary <-> string interpretation into client class, don't do it here (reusability).
if console.protocol.to_s.end_with?('uint8utf8')
@driver.on(:message) { |msg| @right.issue(msg.data) }
else
@driver.on(:message) { |msg| @right.issue(msg.data.pack('C*')) }
end

@driver.on(:close) { cleanup }
end
Expand All @@ -51,7 +62,7 @@ def transmit(sockets, is_ws)
@driver.parse(data)
else
@right.fetch(64.kilobytes) do |data|
@driver.binary(data)
@driver.frame(data)
end
end
end
Expand Down
31 changes: 31 additions & 0 deletions lib/websocket_webmks_uint8utf8.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
class WebsocketWebmksUint8utf8 < WebsocketSSLSocket
attr_accessor :url

def initialize(socket, model)
super(socket, model)
@url = URI::Generic.build(:scheme => 'wss',
:host => @model.host_name,
:port => @model.port,
:path => @model.url).to_s

@driver = WebSocket::Driver.client(self, :protocols => ['uint8utf8'])
@driver.on(:close) { socket.close unless socket.closed? }
@driver.start
end

def fetch(length)
# WebSocket::Driver requires an event handler that should be registered only once
@driver.on(:message) { |msg| yield(msg.data) } if @driver.listeners(:message).length.zero?

data = @ssl.sysread(length)
@driver.parse(data)
end

def issue(data)
@driver.frame(data)
end

def write(data)
@ssl.syswrite(data)
end
end
41 changes: 40 additions & 1 deletion spec/lib/websocket_proxy_spec.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,31 @@
shared_examples 'picking protocol' do |requested_protocol, adapter_klass, protocols, pack|
before do
allow_any_instance_of(klass).to receive(:initialize)
end

let(:klass) { adapter_klass }
let(:proxy) { described_class.new(env, FactoryGirl.create(:system_console, :protocol => requested_protocol), logger) }
let(:right) { double('right socket') }
let(:msg) { 'BANANA' }
let(:msg_binary) { msg.unpack('C*') }

it "#{requested_protocol} sets protocols #{protocols}" do
expect(proxy.instance_variable_get(:@driver).instance_variable_get(:@options)[:protocols]).to eq(protocols)
end

it "#{requested_protocol} sets adapter class #{adapter_klass}" do
expect(proxy.instance_variable_get(:@right)).to be_an_instance_of adapter_klass
end

it "#{requested_protocol} sets message callback (#{pack ? 'interpreted' : 'as-is'})" do
callbacks = proxy.instance_variable_get(:@driver).listeners(:message)
expect(callbacks.size).to eq(1)
expect(right).to receive(:issue).with(msg)
proxy.instance_variable_set(:@right, right)
callbacks.first.call(double('message', :data => pack ? msg_binary : msg))
end
end

describe WebsocketProxy do
let(:console) { FactoryGirl.create(:system_console) }
let(:host) { '127.0.0.1:8080' }
Expand All @@ -8,9 +36,20 @@
subject { described_class.new(env, console, logger) }

describe '#initialize' do
before do
allow(TCPSocket).to receive(:open) # prevent real sockets from opening
end

it 'sets the URL' do
expect(subject.url).to eq("ws://#{host}#{uri}")
end

describe 'picks expected protocol and driver' do
it_behaves_like('picking protocol', 'vnc', WebsocketSocket, ['binary'], true)
it_behaves_like('picking protocol', 'spice', WebsocketSocket, ['binary'], true)
it_behaves_like('picking protocol', 'webmks', WebsocketWebmks, ['binary'], true)
it_behaves_like('picking protocol', 'webmks-uint8utf8', WebsocketWebmksUint8utf8, ['uint8utf8'], false)
end
end

describe '#cleanup' do
Expand Down Expand Up @@ -69,7 +108,7 @@

it 'reads from the socket and sends the result to the driver' do
expect(sock).to receive(:recv_nonblock).and_return(123)
expect(driver).to receive(:binary).with(123)
expect(driver).to receive(:frame).with(123)

subject.transmit([sock], is_ws)
end
Expand Down

0 comments on commit 05d8575

Please sign in to comment.