tmp draft
authorKershaw Chang <kershaw@mozilla.com>
Wed, 02 Dec 2020 23:55:28 +0100
changeset 3400968 1f7daf6531d828010eda5159bcb6aedae5433ec6
parent 3400967 8572f69ef03a33be6cf1158e2787d8661712ed6f
child 3400969 822243e152d432a038d941f6033fb8578f7c4efc
push id629649
push userkjang@mozilla.com
push dateThu, 03 Dec 2020 10:21:43 +0000
treeherdertry@822243e152d4 [default view] [failures only]
milestone85.0a1
tmp
netwerk/protocol/http/nsHttpChannel.cpp
netwerk/test/unit/test_use_httpssvc.js
testing/xpcshell/moz-http2/moz-http2.js
--- a/netwerk/protocol/http/nsHttpChannel.cpp
+++ b/netwerk/protocol/http/nsHttpChannel.cpp
@@ -598,24 +598,31 @@ nsresult nsHttpChannel::OnBeforeConnect(
     if (!shouldUpgrade) {
       // Make sure http channel is released on main thread.
       // See bug 1539148 for details.
       nsMainThreadPtrHandle<nsHttpChannel> self(
           new nsMainThreadPtrHolder<nsHttpChannel>(
               "nsHttpChannel::OnBeforeConnect::self", this));
       auto resultCallback = [self(self)](bool aResult, nsresult aStatus) {
         MOZ_ASSERT(NS_IsMainThread());
-
+        printf_stderr("xxx [%p] aResult=%d aStatus=%x, mUseHTTPSSVC=%d\n",
+                      self.get(), aResult, aStatus, self->mUseHTTPSSVC);
         // We need to wait for HTTPSSVC record if there is no AltSvc or HSTS
         // upgrade for this request.
         if (!aResult && NS_SUCCEEDED(aStatus) && self->mUseHTTPSSVC) {
-          LOG(("nsHttpChannel Wait for HTTPSSVC record [this=%p]\n",
-               self.get()));
-          self->mWaitHTTPSSVCRecord = true;
-          return;
+          if (self->mHTTPSSVCRecord) {
+            LOG(("nsHttpChannel already got HTTPSSVC record [this=%p]\n",
+                 self.get()));
+            aResult = true;
+          } else {
+            LOG(("nsHttpChannel Wait for HTTPSSVC record [this=%p]\n",
+                 self.get()));
+            self->mWaitHTTPSSVCRecord = true;
+            return;
+          }
         }
 
         nsresult rv = self->ContinueOnBeforeConnect(aResult, aStatus);
         if (NS_FAILED(rv)) {
           self->CloseCacheEntry(false);
           Unused << self->AsyncAbort(rv);
         }
       };
@@ -6888,16 +6895,17 @@ nsresult nsHttpChannel::MaybeStartDNSPre
       mDNSBlockingThenable = mDNSBlockingPromise.Ensure(__func__);
     }
 
     if (mUseHTTPSSVC || gHttpHandler->UseHTTPSRRForSpeculativeConnection()) {
       rv = mDNSPrefetch->FetchHTTPSSVC(mCaps & NS_HTTP_REFRESH_DNS);
       if (NS_FAILED(rv)) {
         LOG(("  FetchHTTPSSVC failed with 0x%08" PRIx32,
              static_cast<uint32_t>(rv)));
+        mUseHTTPSSVC = false;
       }
     }
   }
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
@@ -9021,25 +9029,26 @@ void nsHttpChannel::PopRedirectAsyncFunc
 // nsIDNSListener functions
 //-----------------------------------------------------------------------------
 
 NS_IMETHODIMP
 nsHttpChannel::OnLookupComplete(nsICancelable* request, nsIDNSRecord* rec,
                                 nsresult status) {
   MOZ_ASSERT(NS_IsMainThread(), "Expecting DNS callback on main thread.");
 
+  nsCOMPtr<nsIDNSAddrRecord> addrRecord = do_QueryInterface(rec);
   nsCOMPtr<nsIDNSHTTPSSVCRecord> httpSSVCRecord = do_QueryInterface(rec);
   LOG(
       ("nsHttpChannel::OnLookupComplete [this=%p] prefetch complete%s: "
        "%s status[0x%" PRIx32 "], isHTTPSSVC=%d\n",
        this, mCaps & NS_HTTP_REFRESH_DNS ? ", refresh requested" : "",
        NS_SUCCEEDED(status) ? "success" : "failure",
        static_cast<uint32_t>(status), !!httpSSVCRecord));
 
-  if (!httpSSVCRecord) {
+  if (addrRecord) {
     // Unset DNS cache refresh if it was requested,
     if (mCaps & NS_HTTP_REFRESH_DNS) {
       mCaps &= ~NS_HTTP_REFRESH_DNS;
       if (mTransaction) {
         mTransaction->SetDNSWasRefreshed();
       }
     }
 
@@ -9050,30 +9059,32 @@ nsHttpChannel::OnLookupComplete(nsICance
       } else {
         mDNSBlockingPromise.Reject(status, __func__);
       }
     }
 
     return NS_OK;
   }
 
+  // This record will be used in the new redirect channel.
+  MOZ_ASSERT(!mHTTPSSVCRecord);
+  mHTTPSSVCRecord = httpSSVCRecord;
+
   if (mWaitHTTPSSVCRecord) {
     MOZ_ASSERT(mURI->SchemeIs("http"));
-    MOZ_ASSERT(!mHTTPSSVCRecord);
-
-    // This record will be used in the new redirect channel.
-    mHTTPSSVCRecord = httpSSVCRecord;
-    nsresult rv = ContinueOnBeforeConnect(true, status);
+
+    nsresult rv = ContinueOnBeforeConnect(
+        NS_SUCCEEDED(status) && mHTTPSSVCRecord, mStatus);
     if (NS_FAILED(rv)) {
       CloseCacheEntry(false);
       Unused << AsyncAbort(rv);
     }
   } else {
     // This channel is not canceled and the transaction is not created.
-    if (NS_SUCCEEDED(mStatus) && !mTransaction &&
+    if (httpSSVCRecord && NS_SUCCEEDED(mStatus) && !mTransaction &&
         (mFirstResponseSource != RESPONSE_FROM_CACHE)) {
       bool hasIPAddress = false;
       Unused << httpSSVCRecord->GetHasIPAddresses(&hasIPAddress);
       Telemetry::Accumulate(Telemetry::DNS_HTTPSSVC_RECORD_RECEIVING_STAGE,
                             hasIPAddress
                                 ? HTTPSSVC_WITH_IPHINT_RECEIVED_STAGE_0
                                 : HTTPSSVC_WITHOUT_IPHINT_RECEIVED_STAGE_0);
       mHTTPSSVCTelemetryReported = true;
--- a/netwerk/test/unit/test_use_httpssvc.js
+++ b/netwerk/test/unit/test_use_httpssvc.js
@@ -112,22 +112,22 @@ function makeChan(url) {
   return chan;
 }
 
 function channelOpenPromise(chan) {
   return new Promise(resolve => {
     function finish(req, buffer) {
       resolve([req, buffer]);
     }
-    let internal = chan.QueryInterface(Ci.nsIHttpChannelInternal);
-    internal.setWaitForHTTPSSVCRecord();
+    //let internal = chan.QueryInterface(Ci.nsIHttpChannelInternal);
+    //internal.setWaitForHTTPSSVCRecord();
     chan.asyncOpen(new ChannelListener(finish, null, CL_ALLOW_UNKNOWN_CL));
   });
 }
-
+/*
 // This is for testing when the HTTPSSVC record is already available before
 // the transaction is added in connection manager.
 add_task(async function testUseHTTPSSVC() {
   // use the h2 server as DOH provider
   prefs.setCharPref(
     "network.trr.uri",
     "https://foo.example.com:" + h2Port + "/httpssvc_as_altsvc"
   );
@@ -177,17 +177,17 @@ add_task(async function testUseHTTPSSVC1
 
   let chan = makeChan(`https://test.httpssvc.com:8080/`);
   let [req, resp] = await channelOpenPromise(chan);
   Assert.equal(req.getResponseHeader("x-connection-http2"), "yes");
 
   certOverrideService.setDisableAllSecurityChecksAndLetAttackersInterceptMyData(
     false
   );
-});
+});*/
 
 class EventSinkListener {
   getInterface(iid) {
     if (iid.equals(Ci.nsIChannelEventSink)) {
       return this;
     }
   }
   asyncOnChannelRedirect(oldChan, newChan, flags, callback) {
@@ -201,35 +201,55 @@ class EventSinkListener {
 EventSinkListener.prototype.QueryInterface = ChromeUtils.generateQI([
   "nsIInterfaceRequestor",
   "nsIChannelEventSink",
 ]);
 
 // Test if the request is upgraded to https with a HTTPSSVC record.
 add_task(async function testUseHTTPSSVCAsHSTS() {
   dns.clearCache(true);
+  // use the h2 server as DOH provider
+  prefs.setCharPref(
+    "network.trr.uri",
+    "https://foo.example.com:" + h2Port + "/httpssvc_as_altsvc"
+  );
+
+  let dnsListener = new DNSListener();
+  let request = dns.asyncResolve(
+    "test.httpssvc.com",
+    dns.RESOLVE_TYPE_HTTPSSVC,
+    0,
+    null, // resolverInfo
+    dnsListener,
+    mainThread,
+    defaultOriginAttributes
+  );
+
+  let [inRequest, inRecord, inStatus] = await dnsListener;
+  Assert.equal(inRequest, request, "correct request was used");
+  Assert.equal(inStatus, Cr.NS_OK, "status OK");
 
   certOverrideService.setDisableAllSecurityChecksAndLetAttackersInterceptMyData(
     true
   );
 
-  let chan = makeChan(`http://test.httpssvc.com:80/`);
+  let chan = makeChan(`http://foo.notexisted.com:80/`);
   let listener = new EventSinkListener();
   chan.notificationCallbacks = listener;
 
   let [req, resp] = await channelOpenPromise(chan);
 
   req.QueryInterface(Ci.nsIHttpChannel);
   Assert.equal(req.getResponseHeader("x-connection-http2"), "yes");
 
   certOverrideService.setDisableAllSecurityChecksAndLetAttackersInterceptMyData(
     false
   );
 });
-
+/*
 // Test if we can successfully fallback to the original host and port.
 add_task(async function testFallback() {
   let trrServer = new TRRServer();
   registerCleanupFunction(async () => trrServer.stop());
   await trrServer.start();
 
   Services.prefs.setIntPref("network.trr.mode", 3);
   Services.prefs.setCharPref(
@@ -292,9 +312,9 @@ add_task(async function testFallback() {
   let chan = makeChan(`https://test.fallback.com:${h2Port}`);
   let [req, resp] = await channelOpenPromise(chan);
   // Test if this request is done by h2.
   Assert.equal(req.getResponseHeader("x-connection-http2"), "yes");
 
   certOverrideService.setDisableAllSecurityChecksAndLetAttackersInterceptMyData(
     false
   );
-});
+});*/
--- a/testing/xpcshell/moz-http2/moz-http2.js
+++ b/testing/xpcshell/moz-http2/moz-http2.js
@@ -907,24 +907,28 @@ function handleRequest(req, res) {
     let payload = Buffer.from("");
     req.on("data", function receiveData(chunk) {
       payload = Buffer.concat([payload, chunk]);
     });
     req.on("end", function finishedData() {
       let packet = dnsPacket.decode(payload);
       let answers = [];
       if (packet.questions[0].type == "HTTPS") {
+        let priority = 1;
+        if (packet.questions[0].name === "foo.notexisted.com") {
+          priority = 0;
+        }
         answers.push({
           name: packet.questions[0].name,
           type: packet.questions[0].type,
           ttl: 55,
           class: "IN",
           flush: false,
           data: {
-            priority: 1,
+            priority: priority,
             name: "foo.example.com",
             values: [
               { key: "alpn", value: "h2" },
               { key: "port", value: serverPort },
               { key: 30, value: "somelargestring" },
             ],
           },
         });