Issue 128: Convert frame::Reason to struct (#142)
Alter frame::Reason to a struct with a single u32 member. Introduce Constants to the impl for existing Reasons. Change all usage in the library and its tests to adopt this change, using the new constants.
This commit is contained in:
		
				
					committed by
					
						 Carl Lerche
						Carl Lerche
					
				
			
			
				
	
			
			
			
						parent
						
							1c179f7bf2
						
					
				
				
					commit
					2aee78c7d7
				
			| @@ -103,11 +103,11 @@ impl FlowControl { | ||||
|         let (val, overflow) = self.window_size.overflowing_add(sz as i32); | ||||
|  | ||||
|         if overflow { | ||||
|             return Err(Reason::FlowControlError); | ||||
|             return Err(Reason::FLOW_CONTROL_ERROR); | ||||
|         } | ||||
|  | ||||
|         if val > MAX_WINDOW_SIZE as i32 { | ||||
|             return Err(Reason::FlowControlError); | ||||
|             return Err(Reason::FLOW_CONTROL_ERROR); | ||||
|         } | ||||
|  | ||||
|         trace!( | ||||
| @@ -126,7 +126,12 @@ impl FlowControl { | ||||
|     /// This is called after receiving a SETTINGS frame with a lower | ||||
|     /// INITIAL_WINDOW_SIZE value. | ||||
|     pub fn dec_window(&mut self, sz: WindowSize) { | ||||
|         trace!("dec_window; sz={}; window={}, available={}", sz, self.window_size, self.available); | ||||
|         trace!( | ||||
|             "dec_window; sz={}; window={}, available={}", | ||||
|             sz, | ||||
|             self.window_size, | ||||
|             self.available | ||||
|         ); | ||||
|         // This should not be able to overflow `window_size` from the bottom. | ||||
|         self.window_size -= sz as i32; | ||||
|         self.available = self.available.saturating_sub(sz); | ||||
|   | ||||
| @@ -23,7 +23,6 @@ use self::state::State; | ||||
| use self::store::{Entry, Store}; | ||||
| use self::stream::Stream; | ||||
|  | ||||
| use error::Reason::*; | ||||
| use frame::{StreamId, StreamIdOverflow}; | ||||
| use proto::*; | ||||
|  | ||||
|   | ||||
| @@ -110,7 +110,7 @@ where | ||||
|  | ||||
|         let next_id = self.next_stream_id()?; | ||||
|         if id < next_id { | ||||
|             return Err(RecvError::Connection(ProtocolError)); | ||||
|             return Err(RecvError::Connection(Reason::PROTOCOL_ERROR)); | ||||
|         } | ||||
|  | ||||
|         self.next_stream_id = id.next_id(); | ||||
| @@ -152,10 +152,12 @@ where | ||||
|             if let Some(content_length) = frame.fields().get(header::CONTENT_LENGTH) { | ||||
|                 let content_length = match parse_u64(content_length.as_bytes()) { | ||||
|                     Ok(v) => v, | ||||
|                     Err(_) => return Err(RecvError::Stream { | ||||
|                         id: stream.id, | ||||
|                         reason: Reason::ProtocolError, | ||||
|                     }), | ||||
|                     Err(_) => { | ||||
|                         return Err(RecvError::Stream { | ||||
|                             id: stream.id, | ||||
|                             reason: Reason::PROTOCOL_ERROR, | ||||
|                         }) | ||||
|                     }, | ||||
|                 }; | ||||
|  | ||||
|                 stream.content_length = ContentLength::Remaining(content_length); | ||||
| @@ -191,7 +193,7 @@ where | ||||
|         if stream.ensure_content_length_zero().is_err() { | ||||
|             return Err(RecvError::Stream { | ||||
|                 id: stream.id, | ||||
|                 reason: ProtocolError, | ||||
|                 reason: Reason::PROTOCOL_ERROR, | ||||
|             }); | ||||
|         } | ||||
|  | ||||
| @@ -272,7 +274,7 @@ where | ||||
|         if !stream.state.is_recv_streaming() { | ||||
|             // Receiving a DATA frame when not expecting one is a protocol | ||||
|             // error. | ||||
|             return Err(RecvError::Connection(ProtocolError)); | ||||
|             return Err(RecvError::Connection(Reason::PROTOCOL_ERROR)); | ||||
|         } | ||||
|  | ||||
|         trace!( | ||||
| @@ -285,7 +287,7 @@ where | ||||
|         // Ensure that there is enough capacity on the connection before acting | ||||
|         // on the stream. | ||||
|         if self.flow.window_size() < sz || stream.recv_flow.window_size() < sz { | ||||
|             return Err(RecvError::Connection(FlowControlError)); | ||||
|             return Err(RecvError::Connection(Reason::FLOW_CONTROL_ERROR)); | ||||
|         } | ||||
|  | ||||
|         // Update connection level flow control | ||||
| @@ -300,7 +302,7 @@ where | ||||
|         if stream.dec_content_length(frame.payload().len()).is_err() { | ||||
|             return Err(RecvError::Stream { | ||||
|                 id: stream.id, | ||||
|                 reason: ProtocolError, | ||||
|                 reason: Reason::PROTOCOL_ERROR, | ||||
|             }); | ||||
|         } | ||||
|  | ||||
| @@ -308,12 +310,12 @@ where | ||||
|             if stream.ensure_content_length_zero().is_err() { | ||||
|                 return Err(RecvError::Stream { | ||||
|                     id: stream.id, | ||||
|                     reason: ProtocolError, | ||||
|                     reason: Reason::PROTOCOL_ERROR, | ||||
|                 }); | ||||
|             } | ||||
|  | ||||
|             if stream.state.recv_close().is_err() { | ||||
|                 return Err(RecvError::Connection(ProtocolError)); | ||||
|                 return Err(RecvError::Connection(Reason::PROTOCOL_ERROR)); | ||||
|             } | ||||
|         } | ||||
|  | ||||
| @@ -382,7 +384,7 @@ where | ||||
|     pub fn ensure_not_idle(&self, id: StreamId) -> Result<(), Reason> { | ||||
|         if let Ok(next) = self.next_stream_id { | ||||
|             if id >= next { | ||||
|                 return Err(ProtocolError); | ||||
|                 return Err(Reason::PROTOCOL_ERROR); | ||||
|             } | ||||
|         } | ||||
|         // if next_stream_id is overflowed, that's ok. | ||||
| @@ -417,12 +419,12 @@ where | ||||
|         if !P::is_server() { | ||||
|             // Remote is a server and cannot open streams. PushPromise is | ||||
|             // registered by reserving, so does not go through this path. | ||||
|             return Err(RecvError::Connection(ProtocolError)); | ||||
|             return Err(RecvError::Connection(Reason::PROTOCOL_ERROR)); | ||||
|         } | ||||
|  | ||||
|         // Ensure that the ID is a valid server initiated ID | ||||
|         if !id.is_client_initiated() { | ||||
|             return Err(RecvError::Connection(ProtocolError)); | ||||
|             return Err(RecvError::Connection(Reason::PROTOCOL_ERROR)); | ||||
|         } | ||||
|  | ||||
|         Ok(()) | ||||
| @@ -432,7 +434,7 @@ where | ||||
|         if let Ok(id) = self.next_stream_id { | ||||
|             Ok(id) | ||||
|         } else { | ||||
|             Err(RecvError::Connection(ProtocolError)) | ||||
|             Err(RecvError::Connection(Reason::PROTOCOL_ERROR)) | ||||
|         } | ||||
|     } | ||||
|  | ||||
| @@ -442,7 +444,7 @@ where | ||||
|         if P::is_server() { | ||||
|             // The remote is a client and cannot reserve | ||||
|             trace!("recv_push_promise; error remote is client"); | ||||
|             return Err(RecvError::Connection(ProtocolError)); | ||||
|             return Err(RecvError::Connection(Reason::PROTOCOL_ERROR)); | ||||
|         } | ||||
|  | ||||
|         if !promised_id.is_server_initiated() { | ||||
| @@ -450,12 +452,12 @@ where | ||||
|                 "recv_push_promise; error promised id is invalid {:?}", | ||||
|                 promised_id | ||||
|             ); | ||||
|             return Err(RecvError::Connection(ProtocolError)); | ||||
|             return Err(RecvError::Connection(Reason::PROTOCOL_ERROR)); | ||||
|         } | ||||
|  | ||||
|         if !self.is_push_enabled { | ||||
|             trace!("recv_push_promise; error push is disabled"); | ||||
|             return Err(RecvError::Connection(ProtocolError)); | ||||
|             return Err(RecvError::Connection(Reason::PROTOCOL_ERROR)); | ||||
|         } | ||||
|  | ||||
|         Ok(()) | ||||
| @@ -474,7 +476,7 @@ where | ||||
|             try_ready!(dst.poll_ready()); | ||||
|  | ||||
|             // Create the RST_STREAM frame | ||||
|             let frame = frame::Reset::new(stream_id, RefusedStream); | ||||
|             let frame = frame::Reset::new(stream_id, Reason::REFUSED_STREAM); | ||||
|  | ||||
|             // Buffer the frame | ||||
|             dst.buffer(frame.into()) | ||||
|   | ||||
| @@ -151,8 +151,6 @@ where | ||||
|  | ||||
|         trace!("send_reset -- queueing; frame={:?}", frame); | ||||
|         self.prioritize.queue_frame(frame.into(), stream, task); | ||||
|  | ||||
|  | ||||
|     } | ||||
|  | ||||
|     pub fn send_data( | ||||
| @@ -253,7 +251,7 @@ where | ||||
|     ) -> Result<(), Reason> { | ||||
|         if let Err(e) = self.prioritize.recv_stream_window_update(sz, stream) { | ||||
|             debug!("recv_stream_window_update !!; err={:?}", e); | ||||
|             self.send_reset(FlowControlError.into(), stream, task, true); | ||||
|             self.send_reset(Reason::FLOW_CONTROL_ERROR.into(), stream, task, true); | ||||
|  | ||||
|             return Err(e); | ||||
|         } | ||||
| @@ -342,7 +340,7 @@ where | ||||
|     pub fn ensure_not_idle(&self, id: StreamId) -> Result<(), Reason> { | ||||
|         if let Ok(next) = self.next_stream_id { | ||||
|             if id >= next { | ||||
|                 return Err(ProtocolError); | ||||
|                 return Err(Reason::PROTOCOL_ERROR); | ||||
|             } | ||||
|         } | ||||
|         // if next_stream_id is overflowed, that's ok. | ||||
|   | ||||
| @@ -1,7 +1,6 @@ | ||||
| use codec::{RecvError, UserError}; | ||||
| use codec::UserError::*; | ||||
| use frame::Reason; | ||||
| use frame::Reason::*; | ||||
| use proto; | ||||
|  | ||||
| use self::Inner::*; | ||||
| @@ -166,7 +165,7 @@ impl State { | ||||
|             }, | ||||
|             _ => { | ||||
|                 // All other transitions result in a protocol error | ||||
|                 return Err(RecvError::Connection(ProtocolError)); | ||||
|                 return Err(RecvError::Connection(Reason::PROTOCOL_ERROR)); | ||||
|             }, | ||||
|         }; | ||||
|  | ||||
| @@ -180,7 +179,7 @@ impl State { | ||||
|                 self.inner = ReservedRemote; | ||||
|                 Ok(()) | ||||
|             }, | ||||
|             _ => Err(RecvError::Connection(ProtocolError)), | ||||
|             _ => Err(RecvError::Connection(Reason::PROTOCOL_ERROR)), | ||||
|         } | ||||
|     } | ||||
|  | ||||
| @@ -200,7 +199,7 @@ impl State { | ||||
|                 self.inner = Closed(None); | ||||
|                 Ok(()) | ||||
|             }, | ||||
|             _ => Err(RecvError::Connection(ProtocolError)), | ||||
|             _ => Err(RecvError::Connection(Reason::PROTOCOL_ERROR)), | ||||
|         } | ||||
|     } | ||||
|  | ||||
|   | ||||
| @@ -117,7 +117,7 @@ where | ||||
|             } else { | ||||
|                 if !frame.is_end_stream() { | ||||
|                     // TODO: Is this the right error | ||||
|                     return Err(RecvError::Connection(ProtocolError)); | ||||
|                     return Err(RecvError::Connection(Reason::PROTOCOL_ERROR)); | ||||
|                 } | ||||
|  | ||||
|                 actions.recv.recv_trailers(frame, stream) | ||||
| @@ -135,7 +135,7 @@ where | ||||
|  | ||||
|         let stream = match me.store.find_mut(&id) { | ||||
|             Some(stream) => stream, | ||||
|             None => return Err(RecvError::Connection(ProtocolError)), | ||||
|             None => return Err(RecvError::Connection(Reason::PROTOCOL_ERROR)), | ||||
|         }; | ||||
|  | ||||
|         let actions = &mut me.actions; | ||||
| @@ -153,7 +153,7 @@ where | ||||
|         let id = frame.stream_id(); | ||||
|  | ||||
|         if id.is_zero() { | ||||
|             return Err(RecvError::Connection(ProtocolError)); | ||||
|             return Err(RecvError::Connection(Reason::PROTOCOL_ERROR)); | ||||
|         } | ||||
|  | ||||
|         let stream = match me.store.find_mut(&id) { | ||||
| @@ -213,16 +213,14 @@ where | ||||
|         let err = frame.reason().into(); | ||||
|  | ||||
|         me.store | ||||
|             .for_each(|stream| { | ||||
|                 if stream.id > last_stream_id { | ||||
|                     counts.transition(stream, |_, stream| { | ||||
|                         actions.recv.recv_err(&err, &mut *stream); | ||||
|                         actions.send.recv_err(stream); | ||||
|                         Ok::<_, ()>(()) | ||||
|                     }) | ||||
|                 } else { | ||||
|             .for_each(|stream| if stream.id > last_stream_id { | ||||
|                 counts.transition(stream, |_, stream| { | ||||
|                     actions.recv.recv_err(&err, &mut *stream); | ||||
|                     actions.send.recv_err(stream); | ||||
|                     Ok::<_, ()>(()) | ||||
|                 } | ||||
|                 }) | ||||
|             } else { | ||||
|                 Ok::<_, ()>(()) | ||||
|             }) | ||||
|             .unwrap(); | ||||
|  | ||||
| @@ -274,7 +272,7 @@ where | ||||
|  | ||||
|         let stream = match me.store.find_mut(&id) { | ||||
|             Some(stream) => stream.key(), | ||||
|             None => return Err(RecvError::Connection(ProtocolError)), | ||||
|             None => return Err(RecvError::Connection(Reason::PROTOCOL_ERROR)), | ||||
|         }; | ||||
|  | ||||
|         me.actions | ||||
| @@ -513,8 +511,7 @@ impl<B, P> StreamRef<B, P> | ||||
| where | ||||
|     P: Peer, | ||||
| { | ||||
|     pub fn send_data(&mut self, data: B, end_stream: bool) | ||||
|         -> Result<(), UserError> | ||||
|     pub fn send_data(&mut self, data: B, end_stream: bool) -> Result<(), UserError> | ||||
|     where | ||||
|         B: Buf, | ||||
|     { | ||||
| @@ -622,10 +619,7 @@ where | ||||
|  | ||||
|     /// Releases recv capacity back to the peer. This may result in sending | ||||
|     /// WINDOW_UPDATE frames on both the stream and connection. | ||||
|     pub fn release_capacity( | ||||
|             &mut self, | ||||
|             capacity: WindowSize | ||||
|     ) -> Result<(), UserError> | ||||
|     pub fn release_capacity(&mut self, capacity: WindowSize) -> Result<(), UserError> | ||||
|     where | ||||
|         B: Buf, | ||||
|     { | ||||
| @@ -742,11 +736,9 @@ where | ||||
|                     .field("ref_count", &stream.ref_count) | ||||
|                     .finish() | ||||
|             }, | ||||
|             Err(_poisoned) => { | ||||
|                 fmt.debug_struct("StreamRef") | ||||
|                     .field("inner", &"<Poisoned>") | ||||
|                     .finish() | ||||
|             } | ||||
|             Err(_poisoned) => fmt.debug_struct("StreamRef") | ||||
|                 .field("inner", &"<Poisoned>") | ||||
|                 .finish(), | ||||
|         } | ||||
|     } | ||||
| } | ||||
| @@ -786,7 +778,7 @@ where | ||||
|                     stream.id, | ||||
|                 ); | ||||
|                 actions.send.send_reset( | ||||
|                     Reason::Cancel, | ||||
|                     Reason::CANCEL, | ||||
|                     stream, | ||||
|                     &mut actions.task, | ||||
|                     false | ||||
|   | ||||
		Reference in New Issue
	
	Block a user