From 3ed32f80780152806b38f3a16634f7d1922fd129 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Wed, 29 Mar 2017 15:30:12 +0200 Subject: [PATCH] Introduce NetworkError::from_hyper_error and from_ssl_error --- Cargo.lock | 1 + components/net/http_loader.rs | 60 +-------------------------- components/net_traits/Cargo.toml | 1 + components/net_traits/lib.rs | 71 +++++++++++++++++++++++++++++++- 4 files changed, 72 insertions(+), 61 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a9c9a8e6c9ab..484417fb79b9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1762,6 +1762,7 @@ dependencies = [ "log 0.3.7 (registry+https://github.com/rust-lang/crates.io-index)", "msg 0.0.1", "num-traits 0.1.37 (registry+https://github.com/rust-lang/crates.io-index)", + "openssl 0.7.14 (registry+https://github.com/rust-lang/crates.io-index)", "parse-hosts 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)", "serde 0.9.11 (registry+https://github.com/rust-lang/crates.io-index)", "serde_derive 0.9.11 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/components/net/http_loader.rs b/components/net/http_loader.rs index ec8bfed96666..3b4b9e89af19 100644 --- a/components/net/http_loader.rs +++ b/components/net/http_loader.rs @@ -35,9 +35,7 @@ use net_traits::hosts::replace_host; use net_traits::request::{CacheMode, CredentialsMode, Destination, Origin}; use net_traits::request::{RedirectMode, Referrer, Request, RequestMode, ResponseTainting}; use net_traits::response::{HttpsState, Response, ResponseBody, ResponseType}; -use openssl; use openssl::ssl::SslStream; -use openssl::ssl::error::{OpensslError, SslError}; use resource_thread::AuthCache; use servo_url::{ImmutableOrigin, ServoUrl}; use std::collections::HashSet; @@ -140,34 +138,7 @@ impl NetworkHttpRequestFactory { fn create(&self, url: ServoUrl, method: Method, headers: Headers) -> Result, NetworkError> { let connection = HyperRequest::with_connector(method, url.clone().into_url(), self); - - if let Err(HttpError::Ssl(ref error)) = connection { - let error: &(Error + Send + 'static) = &**error; - if let Some(&SslError::OpenSslErrors(ref errors)) = error.downcast_ref::() { - if errors.iter().any(is_cert_verify_error) { - let mut error_report = vec![format!("ssl error ({}):", openssl::version::version())]; - let mut suggestion = None; - for err in errors { - if is_unknown_message_digest_err(err) { - suggestion = Some("Servo recommends upgrading to a newer OpenSSL version."); - } - error_report.push(format_ssl_error(err)); - } - - if let Some(suggestion) = suggestion { - error_report.push(suggestion.to_owned()); - } - - let error_report = error_report.join("
\n"); - return Err(NetworkError::SslValidation(url, error_report)); - } - } - } - - let mut request = match connection { - Ok(req) => req, - Err(e) => return Err(NetworkError::Internal(e.description().to_owned())), - }; + let mut request = connection.map_err(|e| NetworkError::from_hyper_error(&url, e))?; *request.headers_mut() = headers; Ok(request) @@ -505,35 +476,6 @@ fn obtain_response(request_factory: &NetworkHttpRequestFactory, } } -// FIXME: This incredibly hacky. Make it more robust, and at least test it. -fn is_cert_verify_error(error: &OpensslError) -> bool { - match error { - &OpensslError::UnknownError { ref library, ref function, ref reason } => { - library == "SSL routines" && - function.to_uppercase() == "SSL3_GET_SERVER_CERTIFICATE" && - reason == "certificate verify failed" - } - } -} - -fn is_unknown_message_digest_err(error: &OpensslError) -> bool { - match error { - &OpensslError::UnknownError { ref library, ref function, ref reason } => { - library == "asn1 encoding routines" && - function == "ASN1_item_verify" && - reason == "unknown message digest algorithm" - } - } -} - -fn format_ssl_error(error: &OpensslError) -> String { - match error { - &OpensslError::UnknownError { ref library, ref function, ref reason } => { - format!("{}: {} - {}", library, function, reason) - } - } -} - /// [HTTP fetch](https://fetch.spec.whatwg.org#http-fetch) pub fn http_fetch(request: Rc, cache: &mut CorsCache, diff --git a/components/net_traits/Cargo.toml b/components/net_traits/Cargo.toml index 52fb93700799..5005d764b9a8 100644 --- a/components/net_traits/Cargo.toml +++ b/components/net_traits/Cargo.toml @@ -22,6 +22,7 @@ lazy_static = "0.2" log = "0.3.5" msg = {path = "../msg"} num-traits = "0.1.32" +openssl = "0.7.6" parse-hosts = "0.3.0" serde = "0.9" serde_derive = "0.9" diff --git a/components/net_traits/lib.rs b/components/net_traits/lib.rs index 019864041560..f29f803ef66b 100644 --- a/components/net_traits/lib.rs +++ b/components/net_traits/lib.rs @@ -22,6 +22,7 @@ extern crate lazy_static; extern crate log; extern crate msg; extern crate num_traits; +extern crate openssl; extern crate parse_hosts; extern crate serde; #[macro_use] @@ -35,16 +36,19 @@ extern crate webrender_traits; use cookie_rs::Cookie; use filemanager_thread::FileManagerThreadMsg; use heapsize::HeapSizeOf; +use hyper::Error as HyperError; use hyper::header::{ContentType, Headers, ReferrerPolicy as ReferrerPolicyHeader}; use hyper::http::RawStatus; use hyper::mime::{Attr, Mime}; use hyper_serde::Serde; -use ipc_channel::Error; +use ipc_channel::Error as IpcError; use ipc_channel::ipc::{self, IpcReceiver, IpcSender}; use ipc_channel::router::ROUTER; +use openssl::ssl::error::{OpensslError, SslError}; use request::{Request, RequestInit}; use response::{HttpsState, Response}; use servo_url::ServoUrl; +use std::error::Error; use storage_thread::StorageThreadMsg; pub mod blob_url_store; @@ -265,7 +269,7 @@ impl Action for FetchResponseMsg { /// Handle to a resource thread pub type CoreResourceThread = IpcSender; -pub type IpcSendResult = Result<(), Error>; +pub type IpcSendResult = Result<(), IpcError>; /// Abstraction of the ability to send a particular type of message, /// used by net_traits::ResourceThreads to ease the use its IpcSender sub-fields @@ -526,6 +530,69 @@ pub enum NetworkError { SslValidation(ServoUrl, String), } +impl NetworkError { + pub fn from_hyper_error(url: &ServoUrl, error: HyperError) -> Self { + if let HyperError::Ssl(ref ssl_error) = error { + if let Some(ssl_error) = ssl_error.downcast_ref::() { + return NetworkError::from_ssl_error(url, ssl_error); + } + } + NetworkError::Internal(error.description().to_owned()) + } + + fn from_ssl_error(url: &ServoUrl, error: &SslError) -> Self { + if let SslError::OpenSslErrors(ref errors) = *error { + if errors.iter().any(is_cert_verify_error) { + let mut error_report = vec![format!("ssl error ({}):", openssl::version::version())]; + let mut suggestion = None; + for err in errors { + if is_unknown_message_digest_err(err) { + suggestion = Some("Servo recommends upgrading to a newer OpenSSL version."); + } + error_report.push(format_ssl_error(err)); + } + + if let Some(suggestion) = suggestion { + error_report.push(suggestion.to_owned()); + } + + let error_report = error_report.join("
\n"); + return NetworkError::SslValidation(url.clone(), error_report); + } + } + NetworkError::Internal(error.description().to_owned()) + } +} + +fn format_ssl_error(error: &OpensslError) -> String { + match error { + &OpensslError::UnknownError { ref library, ref function, ref reason } => { + format!("{}: {} - {}", library, function, reason) + } + } +} + +// FIXME: This incredibly hacky. Make it more robust, and at least test it. +fn is_cert_verify_error(error: &OpensslError) -> bool { + match error { + &OpensslError::UnknownError { ref library, ref function, ref reason } => { + library == "SSL routines" && + function.to_uppercase() == "SSL3_GET_SERVER_CERTIFICATE" && + reason == "certificate verify failed" + } + } +} + +fn is_unknown_message_digest_err(error: &OpensslError) -> bool { + match error { + &OpensslError::UnknownError { ref library, ref function, ref reason } => { + library == "asn1 encoding routines" && + function == "ASN1_item_verify" && + reason == "unknown message digest algorithm" + } + } +} + /// Normalize `slice`, as defined by /// [the Fetch Spec](https://fetch.spec.whatwg.org/#concept-header-value-normalize). pub fn trim_http_whitespace(mut slice: &[u8]) -> &[u8] {