settings - allow input of initial settings

ThetaSinner · ThetaSinner · commit 74d8dba129ca · 2017-12-03T23:42:11.000Z
started out trying to prevent data frames which are too large, ended up with this
diff --git a/examples/http2/file_server/src/main.rs b/examples/http2/file_server/src/main.rs
@@ -22,7 +22,7 @@ extern crate chrono;
 
 use std::fs::File;
 use std::io::prelude::*;
-use osmium::http2::{self, net, header, stream as streaming};
+use osmium::http2::{self, net, header, stream as streaming, settings};
 use osmium::shared::connection_handle::ConnectionHandle;
 use osmium::shared;
 use chrono::{DateTime, TimeZone, NaiveDateTime, Utc, Local};
@@ -223,5 +223,11 @@ fn main() {
     security.set_ssl_cert_path(String::from("../../../tests/cert.pfx"));
     settings.set_security(security);
 
-    net::Server::new(MyServer {}, settings).start_server();
+    let mut http2_settings = Vec::new();
+    http2_settings.push(settings::SettingsParameter::new(settings::SettingName::SettingsHeaderTableSize, 65536));
+    http2_settings.push(settings::SettingsParameter::new(settings::SettingName::SettingsInitialWindowSize, 131072));
+    http2_settings.push(settings::SettingsParameter::new(settings::SettingName::SettingsMaxFrameSize, 16384));
+    settings.set_http2_settings(http2_settings);
+
+    net::Server::new(MyServer {}, settings).unwrap().start_server();
 }
diff --git a/src/http2/core/connection.rs b/src/http2/core/connection.rs
@@ -59,11 +59,12 @@ pub struct Connection<'a> {
 
 impl<'a> Connection<'a> {
     pub fn new(
-        hpack_send_context: hpack_context::SendContext<'a>, 
-        hpack_recv_context: hpack_context::RecvContext<'a>, 
-        initial_settings: framing::settings::SettingsFrame,
+        hpack_send_context: hpack_context::SendContext<'a>,
+        hpack_recv_context: hpack_context::RecvContext<'a>,
+        initial_local_settings: settings::Settings,
+        initial_remote_settings_frame: framing::settings::SettingsFrame,
         shutdown_signaller: shutdown_signal::ShutdownSignaller
-    ) -> Connection<'a> 
+    ) -> Connection<'a>
     {
         let mut new_con = Connection {
             send_frames: VecDeque::new(),
@@ -73,7 +74,7 @@ impl<'a> Connection<'a> {
             streams: HashMap::new(),
             stream_blocker: stream_blocker::StreamBlocker::new(),
             promised_streams_queue: VecDeque::new(),
-            connection_shared_state: Rc::new(RefCell::new(connection_shared_state::ConnectionSharedState::new())),
+            connection_shared_state: Rc::new(RefCell::new(connection_shared_state::ConnectionSharedState::new(initial_local_settings))),
             highest_remote_initiated_stream_identifier: 0,
             shutdown_initiated: false,
             shutdown_signaller: shutdown_signaller,
@@ -83,7 +84,7 @@ impl<'a> Connection<'a> {
 
         // TODO The ONLY time when ack is not required is when a 101 switching protocols is sent.
         // Switching to true for now, and need to tidy up later.
-        new_con.apply_settings(initial_settings, true);
+        new_con.apply_settings(initial_remote_settings_frame, true);
 
         new_con
     }
@@ -102,7 +103,6 @@ impl<'a> Connection<'a> {
             return;
         }
 
-        // TODO handle frame type not recognised.
         let frame_type = match frame.header.frame_type {
             Some(ref frame_type) => frame_type.clone(),
             None => {
@@ -198,6 +198,14 @@ impl<'a> Connection<'a> {
                     return;
                 }
 
+                if frame.header.length > self.connection_shared_state.borrow().local_settings.max_frame_size {
+                    self.shutdown_connection(error::HttpError::ConnectionError(
+                        error::ErrorCode::ProtocolError,
+                        error::ErrorName::DataFramePayloadLargerThanSettingsValue
+                    ));
+                    return;
+                }
+
                 self.handle_flow_control_for_recv(frame.header.length);
                 self.move_to_stream(frame_type, frame, app);
             },
diff --git a/src/http2/core/connection_shared_state.rs b/src/http2/core/connection_shared_state.rs
@@ -21,6 +21,7 @@ use http2::stream::StreamId;
 
 pub struct ConnectionSharedState {
     pub remote_settings: settings::Settings,
+    pub local_settings: settings::Settings,
     next_server_created_stream_id: StreamId,
     // If streams were ever made concurrent it would be VITAL that this is locked. It is used to communicate to
     // the client which streams have started processing, or at least the highest numbered one. That means no more
@@ -29,9 +30,10 @@ pub struct ConnectionSharedState {
 }
 
 impl ConnectionSharedState {
-    pub fn new() -> Self {
+    pub fn new(local_settings: settings::Settings) -> Self {
         ConnectionSharedState {
             remote_settings: settings::Settings::spec_default(),
+            local_settings: local_settings,
             next_server_created_stream_id: 2,
             highest_started_processing_stream_id: 0
         }
diff --git a/src/http2/error.rs b/src/http2/error.rs
@@ -114,7 +114,8 @@ pub enum ErrorName {
     InvalidInitialWindowSize,
     ConnectionFlowControlWindowNotRespected,
     SettingsAcknowledgementWithNonZeroPayloadLength,
-    SettingsFramePayloadSizeNotAMultipleOfSix
+    SettingsFramePayloadSizeNotAMultipleOfSix,
+    DataFramePayloadLargerThanSettingsValue
 }
 
 impl From<ErrorName> for Vec<u8> {
@@ -170,6 +171,9 @@ impl From<ErrorName> for Vec<u8> {
             },
             ErrorName::SettingsFramePayloadSizeNotAMultipleOfSix => {
                 "settings frame payload size is not a multiple of 6"
+            },
+            ErrorName::DataFramePayloadLargerThanSettingsValue => {
+                "data frame payload larger than settings value"
             }
         }.to_owned().as_bytes().to_vec()
     }
diff --git a/src/http2/frame/settings.rs b/src/http2/frame/settings.rs
@@ -27,7 +27,7 @@ use http2::error;
 const FLAG_ACK: u8 = 0x1;
 
 // TODO this has been copied out to the http2::settings module.
-#[derive(Debug)]
+#[derive(Debug, Clone)]
 pub struct SettingsParameter {
     name: settings::SettingName,
     value: u32
@@ -43,7 +43,7 @@ impl SettingsParameter {
     }
 }
 
-#[derive(Debug)]
+#[derive(Debug, Clone)]
 pub struct SettingsFrameCompressModel {
     flags: u8,
     parameters: Vec<SettingsParameter>
diff --git a/src/http2/net/mod.rs b/src/http2/net/mod.rs
@@ -26,6 +26,7 @@ use std::sync::Arc;
 use std::rc::Rc;
 use std::marker;
 use std::mem;
+use std::net;
 
 // tokio
 use futures::{Stream, Sink, Future, stream};
@@ -49,6 +50,11 @@ use http2::stream as streaming;
 use http2::settings;
 use shared::server_settings;
 
+#[derive(Debug)]
+pub enum ServerError {
+    InvalidSettingsConfiguration
+}
+
 // TODO this doesn't really belong in the net package.
 pub struct Server<T, R, S>
     where T: server_trait::OsmiumServer<Request=R, Response=S>, 
@@ -57,38 +63,61 @@ pub struct Server<T, R, S>
 {
     hpack: hpack::HPack,
     app: T,
-    server_settings: server_settings::ServerSettings
+    security_settings: Option<server_settings::SecuritySettings>,
+    bind_address: net::SocketAddr,
+    local_settings: settings::Settings,
+    local_settings_frame: framing::settings::SettingsFrameCompressModel
 }
 
 impl<T, R, S> Server<T, R, S> 
     where T: 'static + server_trait::OsmiumServer<Request=R, Response=S> + marker::Sync + marker::Send,
           R: 'static + convert::From<streaming::StreamRequest>,
           S: 'static + convert::Into<streaming::StreamResponse>
 {
-    pub fn new(app: T, server_settings: server_settings::ServerSettings) -> Self {
-        Server {
+    pub fn new(app: T, server_settings: server_settings::ServerSettings) -> Result<Self, ServerError> {
+        // Read the settings configuration.
+        let mut local_settings = settings::Settings::spec_default();
+        if local_settings.apply_changes(server_settings.get_http2_settings()).is_err() {
+            return Err(ServerError::InvalidSettingsConfiguration);
+        };
+
+        let mut local_settings_frame = framing::settings::SettingsFrameCompressModel::new();
+        for setting in server_settings.get_http2_settings() {
+            local_settings_frame.add_parameter(setting.get_name(), setting.get_value());
+        }
+
+        // Build the server bind address.
+        let addr = format!("{}:{}", server_settings.get_host(), server_settings.get_port()).parse().unwrap();
+
+        Ok(Server {
             hpack: hpack::HPack::new(),
             app: app,
-            server_settings: server_settings
-        }
+            security_settings: server_settings.get_security(),
+            bind_address: addr,
+            local_settings: local_settings,
+            local_settings_frame: local_settings_frame
+        })
     }
 
     // The start method consumes self so that it can ensure it can be used on the connection threads.
     // The connection threads take a closure which must have static lifetime. If server startup 
     // succeeds, then the a shared pointer to self is returned.
-    pub fn start_server(self) -> Arc<Box<Self>>
+    pub fn start_server(mut self) -> Arc<Box<Self>>
     {
         // tokio event loop
         let mut event_loop = tokio_core::reactor::Core::new().unwrap();
         let handle = event_loop.handle();
 
         // create a listener for incoming tcp connections
-        let addr = format!("{}:{}", self.server_settings.get_host(), self.server_settings.get_port()).parse().unwrap();
-        let listener = tokio_core::net::TcpListener::bind(&addr, &handle).unwrap();
+        let listener = tokio_core::net::TcpListener::bind(&self.bind_address, &handle).unwrap();
 
         let thread_pool = Rc::new(ThreadPool::new(10));
 
-        let acceptor_factory = acceptor_factory::AcceptorFactory::new(&self.server_settings.get_security());
+        // Create the acceptor factory.
+        let mut security = None;
+        mem::swap(&mut security, &mut self.security_settings);
+        // TODO might not exist.
+        let acceptor_factory = acceptor_factory::AcceptorFactory::new(&security.unwrap());
 
         // TODO this should vary depending on the startup type chosen (http/https)
         let handshake: Box<self::h2handshake::H2Handshake> = Box::new(https::HttpsH2Handshake::new(acceptor_factory));
@@ -100,13 +129,8 @@ impl<T, R, S> Server<T, R, S>
             debug!("Starting connection on {}", _remote_addr);
 
             let inner_handle = handle.clone();
-
-            let mut settings_response = framing::settings::SettingsFrameCompressModel::new();
-            settings_response.add_parameter(settings::SettingName::SettingsHeaderTableSize, 65536);
-            settings_response.add_parameter(settings::SettingName::SettingsInitialWindowSize, 131072);
-            settings_response.add_parameter(settings::SettingName::SettingsMaxFrameSize, 16384);
-
-            let handshake_future = handshake.attempt_handshake(socket, Box::new(settings_response))
+            
+            let handshake_future = handshake.attempt_handshake(socket, Box::new(server_instance.local_settings_frame.clone()))
             .map_err(|e| {
                 error!("I/O error while attempting connection handshake {}", e);
             })
@@ -137,6 +161,7 @@ impl<T, R, S> Server<T, R, S>
                             let mut connection = connection::Connection::new(
                                 server_instance.hpack.new_send_context(),
                                 server_instance.hpack.new_recv_context(),
+                                server_instance.local_settings.clone(),
                                 temp_frame,
                                 shutdown_signal::ShutdownSignaller::new(shutdown_read_tx)
                             );
diff --git a/src/http2/settings.rs b/src/http2/settings.rs
@@ -15,13 +15,16 @@
 // You should have received a copy of the GNU General Public License
 // along with Osmium. If not, see <http://www.gnu.org/licenses/>.
 
+// osmium
+use http2::error;
+
 pub const INITIAL_MAX_FRAME_SIZE: u32 = 0x4000;
 pub const MAXIMUM_MAX_FRAME_SIZE: u32 = 0xFFFFFF;
 
 pub const INITIAL_FLOW_CONTROL_WINDOW_SIZE: u32 = 0xFFFF;
 pub const MAXIMUM_FLOW_CONTROL_WINDOW_SIZE: u32 = 0x7FFFFFFF;
 
-#[derive(Debug)]
+#[derive(Debug, Clone)]
 pub enum SettingName {
     SettingsHeaderTableSize,
     SettingsEnablePush,
@@ -37,7 +40,7 @@ pub struct SettingsParameter {
     value: u32
 }
 
-#[derive(Debug)]
+#[derive(Debug, Clone)]
 pub struct Settings {
     pub header_table_size: u32,
     pub enable_push: bool,
@@ -47,6 +50,20 @@ pub struct Settings {
     pub max_header_list_size: Option<u32>
 }
 
+impl SettingsParameter {
+    pub fn new(name: SettingName, value: u32) -> Self {
+        SettingsParameter { name, value }
+    }
+
+    pub fn get_name(&self) -> SettingName {
+        self.name.clone()
+    }
+
+    pub fn get_value(&self) -> u32 {
+        self.value
+    }
+}
+
 impl Settings {
     pub fn spec_default() -> Self {
         Settings {
@@ -58,6 +75,93 @@ impl Settings {
             max_header_list_size: None
         }
     }
+
+    pub fn apply_changes(&mut self, changes: &[SettingsParameter]) -> Result<Vec<SettingName>, error::HttpError> {
+        let mut changes_applied = Vec::with_capacity(changes.len());
+
+        for setting in changes {
+            match setting.get_name() {
+                SettingName::SettingsHeaderTableSize => {
+                    self.header_table_size = setting.get_value();
+
+                    changes_applied.push(setting.get_name());
+                },
+                SettingName::SettingsEnablePush => {
+                    match setting.get_value() {
+                        0 => {
+                            self.enable_push = false;
+                        },
+                        1 => {
+                            self.enable_push = true;
+                        },
+                        _ => {
+                            // (6.5.2) Any value other than 0 or 1 MUST be treated as a connection 
+                            // error (Section 5.4.1) of type PROTOCOL_ERROR.
+                            return Err(
+                                error::HttpError::ConnectionError(
+                                    error::ErrorCode::ProtocolError,
+                                    error::ErrorName::EnablePushSettingInvalidValue
+                                )
+                            );
+                        }
+                    }
+
+                    changes_applied.push(setting.get_name())
+                },
+                SettingName::SettingsMaxConcurrentStreams => {
+                    self.max_concurrent_streams = Some(setting.get_value());
+
+                    changes_applied.push(setting.get_name());
+                },
+                SettingName::SettingsInitialWindowSize => {
+                    let val = setting.get_value();
+
+                    if val <= MAXIMUM_FLOW_CONTROL_WINDOW_SIZE {
+                        self.initial_window_size = val;
+                    }
+                    else {
+                        // (6.5.2) Values above the maximum flow-control window size of 231-1 MUST be treated as a 
+                        // connection error (Section 5.4.1) of type FLOW_CONTROL_ERROR.
+                        return Err(
+                            error::HttpError::ConnectionError(
+                                error::ErrorCode::ProtocolError,
+                                error::ErrorName::InvalidInitialWindowSize
+                            )
+                        )
+                    }
+
+                    changes_applied.push(setting.get_name());
+                },
+                SettingName::SettingsMaxFrameSize => {
+                    let val = setting.get_value();
+
+                    if INITIAL_MAX_FRAME_SIZE <= val && val <= MAXIMUM_MAX_FRAME_SIZE {
+                        self.max_frame_size = val;
+                    }
+                    else {
+                        // (6.5.2) The initial value is 214 (16,384) octets. The value advertised by an endpoint MUST be between this initial 
+                        // value and the maximum allowed frame size (224-1 or 16,777,215 octets), inclusive. Values outside this range MUST 
+                        // be treated as a connection error (Section 5.4.1) of type PROTOCOL_ERROR.
+                        return Err(
+                            error::HttpError::ConnectionError(
+                                error::ErrorCode::ProtocolError,
+                                error::ErrorName::InvalidMaxFrameSize
+                            )
+                        );
+                    }
+
+                    changes_applied.push(setting.get_name());
+                },
+                SettingName::SettingsMaxHeaderListSize => {
+                    self.max_header_list_size = Some(setting.get_value());
+
+                    changes_applied.push(setting.get_name());
+                }
+            }
+        }
+
+        Ok(changes_applied)
+    }
 }
 
 impl From<SettingName> for u16 {
diff --git a/src/shared/server_settings.rs b/src/shared/server_settings.rs

Original file line number	Diff line number	Diff line change
`@@ -114,7 +114,8 @@ pub enum ErrorName {`
`114`	`114`	`InvalidInitialWindowSize,`
`115`	`115`	`ConnectionFlowControlWindowNotRespected,`
`116`	`116`	`SettingsAcknowledgementWithNonZeroPayloadLength,`
`117`		`- SettingsFramePayloadSizeNotAMultipleOfSix`
	`117`	`+ SettingsFramePayloadSizeNotAMultipleOfSix,`
	`118`	`+ DataFramePayloadLargerThanSettingsValue`
`118`	`119`	`}`
`119`	`120`
`120`	`121`	`impl From<ErrorName> for Vec<u8> {`
`@@ -170,6 +171,9 @@ impl From<ErrorName> for Vec<u8> {`
`170`	`171`	`},`
`171`	`172`	`ErrorName::SettingsFramePayloadSizeNotAMultipleOfSix => {`
`172`	`173`	`"settings frame payload size is not a multiple of 6"`
	`174`	`+ },`
	`175`	`+ ErrorName::DataFramePayloadLargerThanSettingsValue => {`
	`176`	`+ "data frame payload larger than settings value"`
`173`	`177`	`}`
`174`	`178`	`}.to_owned().as_bytes().to_vec()`
`175`	`179`	`}`