Merge branch 'next' of github.com:blockstack/stacks-blockchain into next

src/net/connection.rs

@@ -578,7 +578,7 @@ impl<P: ProtocolFamily> ConnectionInbox<P> {
                     Some(message)
                 },
                 Err(e) => {
-                    // will never be valid, even if underflowed, since the premable ought to have
+                    // will never be valid, even if underflowed, since the preamble ought to have
                     // told us the message length
                     debug!("Invalid message payload: {:?}.  Preamble was {:?}", &e, &preamble);
                     return Err(net_error::InvalidMessage);

src/net/http.rs

@@ -54,6 +54,7 @@ use net::HTTP_PREAMBLE_MAX_NUM_HEADERS;
 use net::MAX_MESSAGE_LEN;
 use net::MAX_MICROBLOCKS_UNCONFIRMED;
 use net::HTTP_REQUEST_ID_RESERVED;
+use net::ClientError;
 
 use burnchains::{ Txid, Address };
 use chainstate::burn::BlockHeaderHash;
@@ -1200,9 +1201,9 @@ impl HttpRequestType {
             }
         }
 
-        let path = preamble.path.clone();
-        test_debug!("Failed to parse '{}'", &path);
-        return Ok(HttpRequestType::Unmatched(HttpRequestMetadata::from_preamble(preamble), path));
+        let _path = preamble.path.clone();
+        test_debug!("Failed to parse '{}'", &_path);
+        Err(net_error::ClientError(ClientError::NotFound(preamble.path.clone())))
     }
 
     fn parse_getinfo<R: Read>(_protocol: &mut StacksHttp, preamble: &HttpRequestPreamble, _regex: &Captures, _query: Option<&str>, _fd: &mut R) -> Result<HttpRequestType, net_error> {
@@ -1459,7 +1460,15 @@ impl HttpRequestType {
             }
         };
 
-        let tx = StacksTransaction::consensus_deserialize(fd)?;
+        let tx = StacksTransaction::consensus_deserialize(fd)
+            .map_err(|e| {
+                if let net_error::DeserializeError(msg) = e {
+                    net_error::ClientError(ClientError::Message(
+                        format!("Failed to deserialize posted transaction: {}", msg)))
+                } else {
+                    e
+                }
+            })?;
         Ok(HttpRequestType::PostTransaction(HttpRequestMetadata::from_preamble(preamble), tx))
     }
     
@@ -1507,7 +1516,7 @@ impl HttpRequestType {
             HttpRequestType::GetContractSrc(ref md, ..) => md,
             HttpRequestType::CallReadOnlyFunction(ref md, ..) => md,
             HttpRequestType::OptionsPreflight(ref md, ..) => md,
-            HttpRequestType::Unmatched(ref md, ..) => md,
+            HttpRequestType::ClientError(ref md, ..) => md,
         }
     }
     
@@ -1528,7 +1537,7 @@ impl HttpRequestType {
             HttpRequestType::GetContractSrc(ref mut md, ..) => md,
             HttpRequestType::CallReadOnlyFunction(ref mut md, ..) => md,
             HttpRequestType::OptionsPreflight(ref mut md, ..) => md,
-            HttpRequestType::Unmatched(ref mut md, ..) => md,
+            HttpRequestType::ClientError(ref mut md, ..) => md,
         }
     }
 
@@ -1569,7 +1578,12 @@ impl HttpRequestType {
                 format!("/v2/contracts/call-read/{}/{}/{}{}", contract_addr, contract_name.as_str(), func_name.as_str(), HttpRequestType::make_query_string(tip_opt.as_ref(), true))
             },
             HttpRequestType::OptionsPreflight(_md, path) => path.to_string(),
-            HttpRequestType::Unmatched(_md, path) => path.to_string(),
+            HttpRequestType::ClientError(_md, e) => {
+                match e {
+                    ClientError::NotFound(path) => path.to_string(),
+                    _ => "error path unknown".into()
+                }
+            },
         }
     }
 
@@ -2165,7 +2179,7 @@ impl MessageSequence for StacksHttpMessage {
                 HttpRequestType::GetContractSrc(..) => "HTTP(GetContractSrc)",
                 HttpRequestType::CallReadOnlyFunction(..) => "HTTP(CallReadOnlyFunction)",
                 HttpRequestType::OptionsPreflight(..) => "HTTP(OptionsPreflight)",
-                HttpRequestType::Unmatched(..) => "HTTP(Unmatched)",
+                HttpRequestType::ClientError(..) => "HTTP(ClientError)",
             },
             StacksHttpMessage::Response(ref res) => match res {
                 HttpResponseType::TokenTransferCost(_, _) => "HTTP(TokenTransferCost)",
@@ -2549,7 +2563,13 @@ impl ProtocolFamily for StacksHttp {
                     Ok(data_request) => Ok((StacksHttpMessage::Request(data_request), cursor.position() as usize)),
                     Err(e) => {
                         info!("Failed to parse HTTP request: {:?}", &e);
-                        Err(e)
+                        if let net_error::ClientError(client_err) = e {
+                            let req = HttpRequestType::ClientError(HttpRequestMetadata::from_preamble(http_request_preamble), client_err);
+                            // consume any remaining HTTP request content by returning bytes read = len
+                            Ok((StacksHttpMessage::Request(req), len))
+                        } else {
+                            Err(e)
+                        }
                     }
                 }
             },

src/net/mod.rs

@@ -126,7 +126,7 @@ pub enum Error {
     ReadError(io::Error),
     /// Failed to decode 
     DeserializeError(String),
-    /// Filaed to write
+    /// Failed to write
     WriteError(io::Error),
     /// Underflow -- not enough bytes to form the message
     UnderflowError(String),
@@ -210,6 +210,32 @@ pub enum Error {
     ClarityError(clarity_error),
     /// Catch-all for chainstate errors that don't map cleanly into network errors
     ChainstateError(String),
+    /// Catch-all for errors that a client should receive more information about
+    ClientError(ClientError),
+}
+
+/// Enum for passing data for ClientErrors
+#[derive(Debug, Clone, PartialEq)]
+pub enum ClientError {
+    /// Catch-all
+    Message(String),
+    /// 404
+    NotFound(String),
+}
+
+impl error::Error for ClientError {
+    fn cause(&self) -> Option<&dyn error::Error> {
+        None
+    }
+}
+
+impl fmt::Display for ClientError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        match self {
+            ClientError::Message(s) => write!(f, "{}", s),
+            ClientError::NotFound(s) => write!(f, "HTTP path not matched: {}", s),
+        }
+    }
 }
 
 impl fmt::Display for Error {
@@ -260,6 +286,7 @@ impl fmt::Display for Error {
             Error::ChainstateError(ref s) => fmt::Display::fmt(s, f),
             Error::ClarityError(ref e) => fmt::Display::fmt(e, f),
             Error::MARFError(ref e) => fmt::Display::fmt(e, f),
+            Error::ClientError(ref e) => write!(f, "ClientError: {}", e),
         }
     }
 }
@@ -310,6 +337,7 @@ impl error::Error for Error {
             Error::PeerThrottled => None,
             Error::LookupError(ref _s) => None,
             Error::ChainstateError(ref _s) => None,
+            Error::ClientError(ref e) => Some(e),
             Error::ClarityError(ref e) => Some(e),
             Error::MARFError(ref e) => Some(e),
         }
@@ -1027,7 +1055,8 @@ pub enum HttpRequestType {
     GetContractSrc(HttpRequestMetadata, StacksAddress, ContractName, Option<StacksBlockId>, bool),
     GetContractABI(HttpRequestMetadata, StacksAddress, ContractName, Option<StacksBlockId>),
     OptionsPreflight(HttpRequestMetadata, String),
-    Unmatched(HttpRequestMetadata, String),     // catch-all if we can't parse the request
+    /// catch-all for any errors we should surface from parsing
+    ClientError(HttpRequestMetadata, ClientError),
 }
 
 /// The fields that Actually Matter to http responses

src/net/rpc.rs

@@ -36,6 +36,7 @@ use net::HttpResponseType;
 use net::HttpRequestMetadata;
 use net::HttpResponseMetadata;
 use net::PeerAddress;
+use net::ClientError;
 use net::RPCPeerInfoData;
 use net::NeighborAddress;
 use net::NeighborsData;
@@ -859,9 +860,15 @@ impl ConversationHttp {
                 response.send(&mut self.connection.protocol, &mut reply).map(|_| ())?;
                 None
             },
-            HttpRequestType::Unmatched(ref _md, ref path) => {
+            HttpRequestType::ClientError(ref _md, ref err) => {
                 let response_metadata = HttpResponseMetadata::from(&req);
-                let response = HttpResponseType::NotFound(response_metadata, path.clone());
+                let response = match err {
+                    ClientError::Message(s) => HttpResponseType::BadRequestJSON(
+                        response_metadata, serde_json::Value::String(s.to_string())),
+                    ClientError::NotFound(path) => HttpResponseType::NotFound(
+                        response_metadata, path.clone())
+                };
+
                 response.send(&mut self.connection.protocol, &mut reply).map(|_| ())?;
                 None
             }

testnet/stacks-node/src/tests/integrations.rs

@@ -15,6 +15,7 @@ use stacks::net::{AccountEntryResponse, ContractSrcResponse, CallReadOnlyRequest
 use stacks::net::StacksMessageCodec;
 use stacks::vm::clarity::ClarityConnection;
 use stacks::core::mempool::MAXIMUM_MEMPOOL_TX_CHAINING;
+use stacks::util::hash::hex_bytes;
 
 use crate::config::InitialBalance;
 use crate::helium::RunLoop;
@@ -556,6 +557,21 @@ fn integration_test_get_info() {
                     .unwrap();
 
                 assert_eq!(res, format!("{}", StacksTransaction::consensus_deserialize(&mut &tx_xfer[..]).unwrap().txid()));
+
+                // let's test a posttransaction call that fails to deserialize,
+                //   making sure we get a nicer error message
+                let tx_hex = "80800000000400f942874ce525e87f21bbe8c121b12fac831d02f4000000000000000000000000000003e80001031734446f0870af42bb0cafad27f405e5d9eba441375eada8607a802b875fbb7ba7c4da3474f2bfd76851fb6314a48fe98b57440b8ccec6c9b8362c843a89f303020000000001047465737400000007282b2031203129";
+                let tx_xfer = hex_bytes(tx_hex).unwrap();
+
+                let res: String = client.post(&path)
+                    .header("Content-Type", "application/octet-stream")
+                    .body(tx_xfer.clone())
+                    .send()
+                    .unwrap()
+                    .json()
+                    .unwrap();
+
+                assert!(res.contains("contract name: too short"));
                 
                 // let's submit an invalid transaction!
                 let path = format!("{}/v2/transactions", &http_origin);