Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add basic support for server-side #4

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ impl Event {
},
protocol::S2C::FramebufferUpdate { count } => {
for _ in 0..count {
let rectangle = try!(protocol::Rectangle::read_from(&mut stream));
let rectangle = try!(protocol::RectangleHeader::read_from(&mut stream));
debug!("<- {:?}", rectangle);

let dst = Rect {
Expand Down
36 changes: 36 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,21 @@ extern crate octavo;
#[cfg(feature = "apple-auth")]
extern crate crypto;

use std::io::Write;
use byteorder::{BigEndian, WriteBytesExt};

mod protocol;
mod zrle;
mod security;

pub mod client;
pub mod proxy;
pub mod server;

pub use protocol::{PixelFormat, Colour, Encoding};
pub use client::Client;
pub use proxy::Proxy;
pub use server::Server;

#[derive(Debug, Clone, Copy, Eq, PartialEq)]
pub struct Rect {
Expand All @@ -27,6 +32,37 @@ pub struct Rect {
pub height: u16
}

impl Rect {
/// Constructs new `Rect`.
pub fn new(left: u16, top: u16, width: u16, height: u16) -> Self {
Rect {
left: left,
top: top,
width: width,
height: height,
}
}

/// Constructs new zero-sized `Rect` placed at (0, 0).
pub fn new_empty() -> Self {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why isn't this a constant?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why should it be? It is jet another way of construction just like new. Compiler should optimize it so in binary code it will be just in-place initialization.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because if it's a constant then you can use it in other constants. Why have methods that always return the same value by design?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, both approaches have small pros and cons.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the pros of new_empty()? They should result in exact same machine code...

Rect {
left: 0,
top: 0,
width: 0,
height: 0,
}
}

/// Writes `Rect` to given stream.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also factor out reading a Rect, perhaps, for consistency.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With Message trait? This is not a message. Without? That method would be unused generating warning.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already use the Message trait for parts of messages as well: https://github.com/whitequark/rust-vnc/blob/master/src/protocol.rs#L486-L514

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I'll move to to protocol and reexport in lib.

fn write_to<W: Write>(&self, writer: &mut W) -> Result<()> {
try!(writer.write_u16::<BigEndian>(self.left));
try!(writer.write_u16::<BigEndian>(self.top));
try!(writer.write_u16::<BigEndian>(self.width));
try!(writer.write_u16::<BigEndian>(self.height));
Ok(())
}
}

#[derive(Debug)]
pub enum Error {
Io(std::io::Error),
Expand Down
73 changes: 68 additions & 5 deletions src/protocol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,40 @@ pub struct PixelFormat {
pub blue_shift: u8,
}

impl PixelFormat {
/// Creates RGB pixel format with 4 bytes per pixel and 3 bytes of depth.
pub fn new_rgb8888() -> Self {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two also should be constants.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same argument as in Rect::new_empty().

PixelFormat {
bits_per_pixel: 32,
depth: 24,
big_endian: true,
true_colour: true,
red_max: 255,
green_max: 255,
blue_max: 255,
red_shift: 0,
green_shift: 8,
blue_shift: 16,
}
}

/// Creates BGR pixel format with 4 bytes per pixel and 3 bytes of depth.
pub fn new_bgr8888() -> Self {
PixelFormat {
bits_per_pixel: 32,
depth: 24,
big_endian: true,
true_colour: true,
red_max: 255,
green_max: 255,
blue_max: 255,
red_shift: 16,
green_shift: 8,
blue_shift: 0,
}
}
}

impl Message for PixelFormat {
fn read_from<R: Read>(reader: &mut R) -> Result<PixelFormat> {
let pixel_format = PixelFormat {
Expand Down Expand Up @@ -330,7 +364,9 @@ pub enum Encoding {
Zrle,
Cursor,
DesktopSize,

// extensions
ExtendedKeyEvent,
}

impl Message for Encoding {
Expand All @@ -344,6 +380,7 @@ impl Message for Encoding {
16 => Ok(Encoding::Zrle),
-239 => Ok(Encoding::Cursor),
-223 => Ok(Encoding::DesktopSize),
-258 => Ok(Encoding::ExtendedKeyEvent),
n => Ok(Encoding::Unknown(n))
}
}
Expand All @@ -357,6 +394,7 @@ impl Message for Encoding {
&Encoding::Zrle => 16,
&Encoding::Cursor => -239,
&Encoding::DesktopSize => -223,
&Encoding::ExtendedKeyEvent => -258,
&Encoding::Unknown(n) => n
};
try!(writer.write_i32::<BigEndian>(encoding));
Expand Down Expand Up @@ -386,7 +424,13 @@ pub enum C2S {
y_position: u16
},
CutText(String),

// extensions
ExtendedKeyEvent {
down: bool,
keysym: u32,
keycode: u32,
},
}

impl Message for C2S {
Expand Down Expand Up @@ -437,6 +481,18 @@ impl Message for C2S {
try!(reader.read_exact(&mut [0u8; 3]));
Ok(C2S::CutText(try!(String::read_from(reader))))
},
255 => {
let submessage_type = try!(reader.read_u8());
match submessage_type {
0 => {
let down = try!(reader.read_u16::<BigEndian>()) != 0;
let keysym = try!(reader.read_u32::<BigEndian>());
let keycode = try!(reader.read_u32::<BigEndian>());
Ok(C2S::ExtendedKeyEvent { down: down, keysym: keysym, keycode: keycode })
}
_ => Err(Error::Unexpected("server to client QEMU submessage type"))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it QEMU specifically?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My god, what a disaster.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's worth adding a comment indicating the source for the extended message formats.

}
}
_ => Err(Error::Unexpected("client to server message type"))
}
}
Expand Down Expand Up @@ -478,23 +534,30 @@ impl Message for C2S {
&C2S::CutText(ref text) => {
try!(String::write_to(text, writer));
}
&C2S::ExtendedKeyEvent { down, keysym, keycode } => {
try!(writer.write_u8(255));
try!(writer.write_u8(0));
try!(writer.write_u16::<BigEndian>(if down { 1 } else { 0 }));
try!(writer.write_u32::<BigEndian>(keysym));
try!(writer.write_u32::<BigEndian>(keycode));
}
}
Ok(())
}
}

#[derive(Debug)]
pub struct Rectangle {
pub struct RectangleHeader {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This renaming is no longer necessary as we have an Update now.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But it really is rectangle header. That's how RFC calls this.

pub x_position: u16,
pub y_position: u16,
pub width: u16,
pub height: u16,
pub encoding: Encoding,
}

impl Message for Rectangle {
fn read_from<R: Read>(reader: &mut R) -> Result<Rectangle> {
Ok(Rectangle {
impl Message for RectangleHeader {
fn read_from<R: Read>(reader: &mut R) -> Result<RectangleHeader> {
Ok(RectangleHeader {
x_position: try!(reader.read_u16::<BigEndian>()),
y_position: try!(reader.read_u16::<BigEndian>()),
width: try!(reader.read_u16::<BigEndian>()),
Expand Down Expand Up @@ -542,7 +605,7 @@ pub enum S2C {
// core spec
FramebufferUpdate {
count: u16,
/* Vec<Rectangle> has to be read out manually */
// Vec<RectangleHeader> has to be read out manually
},
SetColourMapEntries {
first_colour: u16,
Expand Down
4 changes: 2 additions & 2 deletions src/proxy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,9 +177,9 @@ impl Proxy {
match message {
protocol::S2C::FramebufferUpdate { count } => {
for _ in 0..count {
let rectangle = try!(protocol::Rectangle::read_from(server_stream));
let rectangle = try!(protocol::RectangleHeader::read_from(server_stream));
debug!("c<-s {:?}", rectangle);
try!(protocol::Rectangle::write_to(&rectangle, &mut buffer_stream));
try!(protocol::RectangleHeader::write_to(&rectangle, &mut buffer_stream));

match rectangle.encoding {
protocol::Encoding::Raw => {
Expand Down
Loading