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

constellation: crash to a new “sad tab” error page #30290

Merged
merged 10 commits into from Sep 6, 2023
39 changes: 22 additions & 17 deletions components/constellation/constellation.rs
Expand Up @@ -2782,7 +2782,7 @@ where

self.embedder_proxy.send((
Some(top_level_browsing_context_id),
EmbedderMsg::Panic(reason, backtrace),
EmbedderMsg::Panic(reason.clone(), backtrace.clone()),
));

let browsing_context = match self.browsing_contexts.get(&browsing_context_id) {
Expand All @@ -2797,7 +2797,6 @@ where
Some(p) => p,
None => return warn!("failed pipeline is missing"),
};
let pipeline_url = pipeline.url.clone();
let opener = pipeline.opener;

self.close_browsing_context_children(
Expand All @@ -2806,23 +2805,27 @@ where
ExitPipelineMode::Force,
);

let failure_url = ServoUrl::parse("about:failure").expect("infallible");

if pipeline_url == failure_url {
return error!("about:failure failed");
let old_pipeline_id = pipeline_id;
let old_load_data = match self.pipelines.get(&pipeline_id) {
Some(pipeline) => pipeline.load_data.clone(),
None => return warn!("failed pipeline is missing"),
};
if old_load_data.crash.is_some() {
return error!("crash page crashed");
}

warn!("creating replacement pipeline for about:failure");
warn!("creating replacement pipeline for crash page");

let new_pipeline_id = PipelineId::new();
let load_data = LoadData::new(
LoadOrigin::Constellation,
failure_url,
None,
Referrer::NoReferrer,
None,
None,
);
let new_load_data = LoadData {
crash: Some(
backtrace
.map(|b| format!("{}\n{}", reason, b))
.unwrap_or(reason),
),
..old_load_data.clone()
};

let sandbox = IFrameSandboxState::IFrameSandboxed;
let is_private = false;
self.new_pipeline(
Expand All @@ -2832,7 +2835,7 @@ where
None,
opener,
window_size,
load_data,
new_load_data,
sandbox,
is_private,
is_visible,
Expand All @@ -2841,7 +2844,9 @@ where
top_level_browsing_context_id,
browsing_context_id,
new_pipeline_id,
replace: None,
// Pipeline already closed by close_browsing_context_children, so we can pass Yes here
// to avoid closing again in handle_activate_document_msg (though it would be harmless)
replace: Some(NeedsToReload::Yes(old_pipeline_id, old_load_data)),
new_browsing_context_info: None,
window_size,
});
Expand Down
2 changes: 2 additions & 0 deletions components/embedder_traits/resources.rs
Expand Up @@ -61,6 +61,7 @@ pub enum Resource {
RippyPNG,
MediaControlsCSS,
MediaControlsJS,
CrashHTML,
}

pub trait ResourceReaderMethods {
Expand Down Expand Up @@ -96,6 +97,7 @@ fn resources_for_tests() -> Box<dyn ResourceReaderMethods + Sync + Send> {
Resource::RippyPNG => "rippy.png",
Resource::MediaControlsCSS => "media-controls.css",
Resource::MediaControlsJS => "media-controls.js",
Resource::CrashHTML => "crash.html",
};
let mut path = env::current_exe().unwrap();
path = path.canonicalize().unwrap();
Expand Down
7 changes: 7 additions & 0 deletions components/net/fetch/methods.rs
Expand Up @@ -195,6 +195,13 @@ pub async fn main_fetch(
// Step 1.
let mut response = None;

// Servo internal: return a crash error when a crash error page is needed
if let Some(ref details) = request.crash {
response = Some(Response::network_error(NetworkError::Crash(
details.clone(),
)));
}

// Step 2.
if request.local_urls_only {
if !matches!(
Expand Down
4 changes: 3 additions & 1 deletion components/net_traits/lib.rs
Expand Up @@ -758,8 +758,10 @@ pub enum NetworkError {
/// Could be any of the internal errors, like unsupported scheme, connection errors, etc.
Internal(String),
LoadCancelled,
/// SSL validation error that has to be handled in the HTML parser
/// SSL validation error, to be converted to Resource::BadCertHTML in the HTML parser.
SslValidation(String, Vec<u8>),
/// Crash error, to be converted to Resource::Crash in the HTML parser.
Crash(String),
}

impl NetworkError {
Expand Down
12 changes: 12 additions & 0 deletions components/net_traits/request.rs
Expand Up @@ -254,6 +254,8 @@ pub struct RequestBuilder {
pub initiator: Initiator,
pub https_state: HttpsState,
pub response_tainting: ResponseTainting,
/// Servo internal: if crash details are present, trigger a crash error page with these details.
pub crash: Option<String>,
delan marked this conversation as resolved.
Show resolved Hide resolved
}

impl RequestBuilder {
Expand Down Expand Up @@ -284,6 +286,7 @@ impl RequestBuilder {
csp_list: None,
https_state: HttpsState::None,
response_tainting: ResponseTainting::Basic,
crash: None,
}
}

Expand Down Expand Up @@ -382,6 +385,11 @@ impl RequestBuilder {
self
}

pub fn crash(mut self, crash: Option<String>) -> Self {
self.crash = crash;
self
}

pub fn build(self) -> Request {
let mut request = Request::new(
self.url.clone(),
Expand Down Expand Up @@ -415,6 +423,7 @@ impl RequestBuilder {
request.parser_metadata = self.parser_metadata;
request.csp_list = self.csp_list;
request.response_tainting = self.response_tainting;
request.crash = self.crash;
request
}
}
Expand Down Expand Up @@ -488,6 +497,8 @@ pub struct Request {
#[ignore_malloc_size_of = "Defined in rust-content-security-policy"]
pub csp_list: Option<CspList>,
pub https_state: HttpsState,
/// Servo internal: if crash details are present, trigger a crash error page with these details.
pub crash: Option<String>,
}

impl Request {
Expand Down Expand Up @@ -528,6 +539,7 @@ impl Request {
response_tainting: ResponseTainting::Basic,
csp_list: None,
https_state: https_state,
crash: None,
}
}

Expand Down
64 changes: 37 additions & 27 deletions components/script/dom/servoparser/mod.rs
Expand Up @@ -774,28 +774,29 @@ impl FetchResponseListener for ParserContext {
fn process_request_eof(&mut self) {}

fn process_response(&mut self, meta_result: Result<FetchMetadata, NetworkError>) {
let mut ssl_error = None;
let mut network_error = None;
let metadata = match meta_result {
Ok(meta) => Some(match meta {
FetchMetadata::Unfiltered(m) => m,
FetchMetadata::Filtered { unsafe_, .. } => unsafe_,
}),
Err(NetworkError::SslValidation(reason, cert_bytes)) => {
ssl_error = Some((reason, cert_bytes));
let mut meta = Metadata::default(self.url.clone());
let mime: Option<Mime> = "text/html".parse().ok();
meta.set_content_type(mime.as_ref());
Some(meta)
},
Err(NetworkError::Internal(reason)) => {
network_error = Some(reason);
let mut meta = Metadata::default(self.url.clone());
let mime: Option<Mime> = "text/html".parse().ok();
meta.set_content_type(mime.as_ref());
Some(meta)
},
Err(_) => None,
let (metadata, error) = match meta_result {
Ok(meta) => (
Some(match meta {
FetchMetadata::Unfiltered(m) => m,
FetchMetadata::Filtered { unsafe_, .. } => unsafe_,
}),
None,
),
Err(error) => (
// Check variant without moving
match &error {
NetworkError::SslValidation(..) |
NetworkError::Internal(..) |
NetworkError::Crash(..) => {
let mut meta = Metadata::default(self.url.clone());
let mime: Option<Mime> = "text/html".parse().ok();
meta.set_content_type(mime.as_ref());
Some(meta)
},
_ => None,
},
Some(error),
),
};
let content_type: Option<Mime> = metadata
.clone()
Expand Down Expand Up @@ -876,8 +877,8 @@ impl FetchResponseListener for ParserContext {
parser.parse_sync();
parser.tokenizer.borrow_mut().set_plaintext_state();
},
(mime::TEXT, mime::HTML, _) => {
if let Some((reason, bytes)) = ssl_error {
(mime::TEXT, mime::HTML, _) => match error {
Some(NetworkError::SslValidation(reason, bytes)) => {
self.is_synthesized_document = true;
let page = resources::read_string(Resource::BadCertHTML);
let page = page.replace("${reason}", &reason);
Expand All @@ -887,14 +888,23 @@ impl FetchResponseListener for ParserContext {
page.replace("${secret}", &net_traits::PRIVILEGED_SECRET.to_string());
parser.push_string_input_chunk(page);
parser.parse_sync();
}
if let Some(reason) = network_error {
},
Some(NetworkError::Internal(reason)) => {
self.is_synthesized_document = true;
let page = resources::read_string(Resource::NetErrorHTML);
let page = page.replace("${reason}", &reason);
parser.push_string_input_chunk(page);
parser.parse_sync();
}
},
Some(NetworkError::Crash(details)) => {
self.is_synthesized_document = true;
let page = resources::read_string(Resource::CrashHTML);
let page = page.replace("${details}", &details);
parser.push_string_input_chunk(page);
parser.parse_sync();
},
Some(_) => {},
None => {},
},
(mime::TEXT, mime::XML, _) |
(mime::APPLICATION, mime::XML, _) |
Expand Down
1 change: 1 addition & 0 deletions components/script/fetch.rs
Expand Up @@ -129,6 +129,7 @@ fn request_init_from_request(request: NetTraitsRequest) -> RequestBuilder {
csp_list: None,
https_state: request.https_state,
response_tainting: request.response_tainting,
crash: None,
}
}

Expand Down
4 changes: 3 additions & 1 deletion components/script/script_thread.rs
Expand Up @@ -3843,7 +3843,8 @@ impl ScriptThread {
.headers(load_data.headers)
.body(load_data.data)
.redirect_mode(RedirectMode::Manual)
.origin(incomplete.origin.immutable().clone());
.origin(incomplete.origin.immutable().clone())
.crash(load_data.crash);

let context = ParserContext::new(id, load_data.url);
self.incomplete_parser_contexts
Expand All @@ -3869,6 +3870,7 @@ impl ScriptThread {
) {
match fetch_metadata {
Ok(_) => (),
Err(NetworkError::Crash(..)) => (),
Err(ref e) => {
warn!("Network error: {:?}", e);
},
Expand Down
4 changes: 4 additions & 0 deletions components/script_traits/lib.rs
Expand Up @@ -184,6 +184,9 @@ pub struct LoadData {
pub srcdoc: String,
/// The inherited context is Secure, None if not inherited
pub inherited_secure_context: Option<bool>,

/// Servo internal: if crash details are present, trigger a crash error page with these details.
pub crash: Option<String>,
}

/// The result of evaluating a javascript scheme url.
Expand Down Expand Up @@ -218,6 +221,7 @@ impl LoadData {
referrer_policy: referrer_policy,
srcdoc: "".to_string(),
inherited_secure_context,
crash: None,
}
}
}
Expand Down
1 change: 1 addition & 0 deletions ports/gstplugin/resources.rs
Expand Up @@ -35,6 +35,7 @@ fn filename(file: Resource) -> &'static str {
Resource::RippyPNG => "rippy.png",
Resource::MediaControlsCSS => "media-controls.css",
Resource::MediaControlsJS => "media-controls.js",
Resource::CrashHTML => "crash.html",
}
}

Expand Down
1 change: 1 addition & 0 deletions ports/libsimpleservo/api/src/lib.rs
Expand Up @@ -920,6 +920,7 @@ impl ResourceReaderMethods for ResourceReaderInstance {
Resource::MediaControlsJS => {
&include_bytes!("../../../../resources/media-controls.js")[..]
},
Resource::CrashHTML => &include_bytes!("../../../../resources/crash.html")[..],
})
}

Expand Down
1 change: 1 addition & 0 deletions ports/servoshell/resources.rs
Expand Up @@ -30,6 +30,7 @@ fn filename(file: Resource) -> &'static str {
Resource::RippyPNG => "rippy.png",
Resource::MediaControlsCSS => "media-controls.css",
Resource::MediaControlsJS => "media-controls.js",
Resource::CrashHTML => "crash.html",
}
}

Expand Down
8 changes: 8 additions & 0 deletions resources/crash.html
@@ -0,0 +1,8 @@
<p>Servo crashed!</p>

<!-- NOTE: unlike in Firefox and Chrome, this reloads POST as GET -->
<!-- see whatwg/html#6600 + whatwg/html#3215 -->
<button onclick="location.reload()">Reload</button>

<pre><plaintext>
${details}
delan marked this conversation as resolved.
Show resolved Hide resolved