fix(Network): fulfill security details for response redirects (#2025)

This patch:
- starts fulfilling security details for redirect responses
- changes `response.securityDetails()` to return null if the response
  is served over non-secure connection

docs/api.md

@@ -2434,7 +2434,7 @@ Contains a boolean stating whether the response was successful (status in the ra
 - returns: <[Request]> A matching [Request] object.
 
 #### response.securityDetails()
-- returns: <[SecurityDetails]> An object with security details associated with the response. From the original object only the fields `subjectName`, `"issuer"`, `"validFrom"`, `"validTo"`, `"protocol"` are extracted.
+- returns: <?[SecurityDetails]> Security details if the response was received over the secure connection, or `null` otherwise.
 
 #### response.status()
 - returns: <[number]>
@@ -2463,10 +2463,10 @@ Contains the URL of the response.
 - returns: <[string]> Name of the subject to which the certificate was issued to.
 
 #### securityDetails.validFrom()
-- returns: <[number]> Timestamp stating the start of validity of the certificate.
+- returns: <[number]> [UnixTime] stating the start of validity of the certificate.
 
 #### securityDetails.validTo()
-- returns: <[number]> Timestamp stating the end of validity of the certificate.
+- returns: <[number]> [UnixTime] stating the end of validity of the certificate.
 
 ### class: Target
 
@@ -2620,3 +2620,4 @@ reported.
 [Target]: #class-target "Target"
 [USKeyboardLayout]: ../lib/USKeyboardLayout.js "USKeyboardLayout"
 [xpath]: https://developer.mozilla.org/en-US/docs/Web/XPath "xpath"
+[UnixTime]: https://en.wikipedia.org/wiki/Unix_time "Unix Time"

lib/NetworkManager.js

@@ -153,7 +153,7 @@ class NetworkManager extends EventEmitter {
     if (event.redirectUrl) {
       const request = this._interceptionIdToRequest.get(event.interceptionId);
       if (request) {
-        this._handleRequestRedirect(request, event.responseStatusCode, event.responseHeaders, false /* fromDiskCache */, false /* fromServiceWorker */);
+        this._handleRequestRedirect(request, event.responseStatusCode, event.responseHeaders, false /* fromDiskCache */, false /* fromServiceWorker */, null /* securityDetails */);
         this._handleRequestStart(request._requestId, event.interceptionId, event.redirectUrl, event.resourceType, event.request, event.frameId);
       }
       return;
@@ -184,9 +184,10 @@ class NetworkManager extends EventEmitter {
    * @param {!Object} redirectHeaders
    * @param {boolean} fromDiskCache
    * @param {boolean} fromServiceWorker
+   * @param {?Object} securityDetails
    */
-  _handleRequestRedirect(request, redirectStatus, redirectHeaders, fromDiskCache, fromServiceWorker) {
-    const response = new Response(this._client, request, redirectStatus, redirectHeaders, fromDiskCache, fromServiceWorker);
+  _handleRequestRedirect(request, redirectStatus, redirectHeaders, fromDiskCache, fromServiceWorker, securityDetails) {
+    const response = new Response(this._client, request, redirectStatus, redirectHeaders, fromDiskCache, fromServiceWorker, securityDetails);
     request._response = response;
     this._requestIdToRequest.delete(request._requestId);
     this._interceptionIdToRequest.delete(request._interceptionId);
@@ -239,7 +240,7 @@ class NetworkManager extends EventEmitter {
       const request = this._requestIdToRequest.get(event.requestId);
       // If we connect late to the target, we could have missed the requestWillBeSent event.
       if (request)
-        this._handleRequestRedirect(request, event.redirectResponse.status, event.redirectResponse.headers, event.redirectResponse.fromDiskCache, event.redirectResponse.fromServiceWorker);
+        this._handleRequestRedirect(request, event.redirectResponse.status, event.redirectResponse.headers, event.redirectResponse.fromDiskCache, event.redirectResponse.fromServiceWorker, event.redirectResponse.securityDetails);
     }
     this._handleRequestStart(event.requestId, null, event.request.url, event.type, event.request, event.frameId);
   }
@@ -501,8 +502,9 @@ class Response {
    * @param {!Object} securityDetails
    * @param {boolean} fromDiskCache
    * @param {boolean} fromServiceWorker
+   * @param {?Object} securityDetails
    */
-  constructor(client, request, status, headers, fromDiskCache, fromServiceWorker, securityDetails = null) {
+  constructor(client, request, status, headers, fromDiskCache, fromServiceWorker, securityDetails) {
     this._client = client;
     this._request = request;
     this._contentPromise = null;
@@ -514,7 +516,7 @@ class Response {
     this._headers = {};
     for (const key of Object.keys(headers))
       this._headers[key.toLowerCase()] = headers[key];
-    this._securityDetails = {};
+    this._securityDetails = null;
     if (securityDetails) {
       this._securityDetails = new SecurityDetails(
           securityDetails['subjectName'],
@@ -554,7 +556,7 @@ class Response {
   }
 
   /**
-   * @return {!SecurityDetails|Object}
+   * @return {?SecurityDetails}
    */
   securityDetails() {
     return this._securityDetails;

test/test.js

@@ -155,6 +155,21 @@ describe('Puppeteer', function() {
       await page.close();
       await browser.close();
     });
+    it('Network redirects should report SecurityDetails', async({httpsServer}) => {
+      const options = Object.assign({ignoreHTTPSErrors: true}, defaultBrowserOptions);
+      const browser = await puppeteer.launch(options);
+      const page = await browser.newPage();
+      httpsServer.setRedirect('/plzredirect', '/empty.html');
+      const responses =  [];
+      page.on('response', response => responses.push(response));
+      await page.goto(httpsServer.PREFIX + '/plzredirect');
+      expect(responses.length).toBe(2);
+      expect(responses[0].status()).toBe(302);
+      const securityDetails = responses[0].securityDetails();
+      expect(securityDetails.protocol()).toBe('TLS 1.2');
+      await page.close();
+      await browser.close();
+    });
     it('should reject all promises when browser is closed', async() => {
       const browser = await puppeteer.launch(defaultBrowserOptions);
       const page = await browser.newPage();
@@ -1153,6 +1168,7 @@ describe('Page', function() {
     it('should navigate to empty page with domcontentloaded', async({page, server}) => {
       const response = await page.goto(server.EMPTY_PAGE, {waitUntil: 'domcontentloaded'});
       expect(response.status()).toBe(200);
+      expect(response.securityDetails()).toBe(null);
     });
     it('should navigate to empty page with networkidle0', async({page, server}) => {
       const response = await page.goto(server.EMPTY_PAGE, {waitUntil: 'networkidle0'});