Browser API: expose SSL info (securitychange event)

[paulrouget]

mozbrowsersecuritychange properties: https://github.com/paulrouget/mozBrowserAPI/blob/master/BrowserAPI.md#the-mozbrowsersecuritychange-event

Could someone describe what needs to be done to expose SSL information (no ssl, non valid cert, valid cert, mixed content, …)? At least, we'd like to start with basic info: ssl or no ssl.

[jdm]

I would add an enum to the Metadata struct, and store this as a field in Document. This would allow it to be updated for mixed content that is added subsequently, and trigger these events. The field of the enum would be set around http://mxr.mozilla.org/servo/source/components/net/http_loader.rs#711 .

[paulrouget]

Understood.

I'd like to start by implementing event.details.state, with these 2 possible values:

• "insecure" indicates that the data corresponding to the request was received over an insecure channel.
• "secure" indicates that the data corresponding to the request was received over a secure channel.

Is it enough to just use: hsts_list.read().unwrap().is_host_secure(domain) ?

[jdm]

No, since that only corresponds to domains included in the HSTS preload list and any domains that have included the Strict-Transport-Settings header in a previous response. It should be enough to check that the scheme of the final request URL is https if there's a successful response received, I believe, since we deal with certificate errors before that.

[paulrouget]

ok. I think the event should be dispatched once the document and its resources are all loaded, not once the document page request has loaded, since we will eventually add mixed content information to the event.

So probably along the mozbrowserloadend event.

[jdm]

What about if mixed content is added after the document's load event occurs? Does a new event get dispatched in that case?

[paulrouget]

With gecko, securitychange is dispatched right after load starts, and is dispatched again as mixed content gets added to the document. Which I guess makes sense.

[paulrouget]

I'm trying to bring the load status to document.rs.

it looks like that: https://gist.github.com/paulrouget/852cd69e2830d381f3d3

Does that look right? I could use the metadata field instead, it holds a status value as well (RawStatus though), but I had trouble with moved values issues (but I believe I can work around that, I haven't try hard).

The patch above doesn't work though.
status is a Result<(),String>, and I believe it needs to be a Result<StatusCode,String> to then check if it's a 200 status.

Using Result<StatusCode,String> instead of Result<(),String> throws this error:

   Compiling net_traits v0.0.1 (file:///Users/paul/git/servo/components/servo)
/Users/paul/git/servo/components/net_traits/lib.rs:177:23: 177:32 error: the trait `serde::ser::Serialize` is not implemented for the type `hyper::status::StatusCode` [E0277]
/Users/paul/git/servo/components/net_traits/lib.rs:177 #[derive(Deserialize, Serialize)]
                                                                             ^~~~~~~~~
/Users/paul/git/servo/components/net_traits/lib.rs:177:23: 177:32 note: in this expansion of #[derive_Serialize] (defined in /Users/paul/git/servo/components/net_traits/lib.rs)
/Users/paul/git/servo/components/net_traits/lib.rs:177:23: 177:32 help: run `rustc --explain E0277` to see a detailed explanation
/Users/paul/git/servo/components/net_traits/lib.rs:177:23: 177:32 note: required by `serde::ser::Serializer::visit_newtype_variant`
/Users/paul/git/servo/components/net_traits/lib.rs:177:21: 177:21 error: the trait `serde::de::Deserialize` is not implemented for the type `hyper::status::StatusCode` [E0277]
/Users/paul/git/servo/components/net_traits/lib.rs:177 #[derive(Deserialize, Serialize)]
                                                                           ^
/Users/paul/git/servo/components/net_traits/lib.rs:177:10: 177:21 note: in this expansion of try! (defined in <std macros>)
/Users/paul/git/servo/components/net_traits/lib.rs:177:21: 177:21 help: run `rustc --explain E0277` to see a detailed explanation
error: aborting due to 2 previous errors
Could not compile `net_traits`.

If this approach makes sense, what would be the right way to solve the above issue?
Or maybe I should not switch to Result<StatusCode,String>, and somehow cast the () value to StatusCode (is that even possible)?

[jdm]

I'm not convinced that Result<StatusCode/RawStatus, String> is really the right thing to do here, since we're duplicating information between the Metadata structure that's received in headers_available, and the final result is an indication that the response was received in its entirety. I think storing the status code separately in consumers that require it makes the most sense to me.

[paulrouget]

Previous approach was wrong. I was trying to rely on the HTTP status code, which makes no sense.

Here is a simpler approach: master...paulrouget:securitychange

This is basic, but it covers enough for what we need for browser.html.

@jdm, can you take a quick look and tell me if this is what you had in mind?

You mentioned adding an enum to the Metadata struct. Afaict, metadata are not stored anywhere. I added a simple field to the document. Maybe that's enough.

[jdm]

Yep, that's more like what I was expecting. I think we should store a value in Document that reflects https://html.spec.whatwg.org/multipage/dom.html#concept-document-https-state instead of secure/broken/insecure and store the brokenness via a separate boolean field. Additionally, instead of inspecting the scheme of the final URL, we should add a field to Metadata and rely on that value.