Auto merge of #13281 - gilbertw1:basic-auth-cache-clean, r=jdm

Update basic auth cache to key off of origin instead of url

This pull request's primary purpose is to store basic auth credentials based on the url origin instead of the entire url. This fixes an issue where servo continuously prompts the user for credentials any time a basic auth secured resource is requested even though the user has already entered auth credentials for a different resource from the same origin.

The test associated with this PR hides image redirects behind a python handler that requires basic authentication. The reference page loads two images by directly specifying the image to load, while the test page loads the two images using the basic auth redirect handler with only the first image tag providing auth credentials.

I'd like to point a few specific items for review:
* url::Origin does not derive ```Hash```, so I am using ```ascii_serialization``` as the cache key. This seems like a stable enough representation.
* I've updated the http loader to store credentials not only on Success responses, but Redirect responses as well. I stumbled on this because nginx was redirecting 'test' -> 'test/' in my testing, and other browsers were storing the credentials on the redirect response vs. prompting for credentials a second time.
* In the test I'm using a timeout to load the second image (without authentication), otherwise the order that the images were loaded was unpredictable.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors related to these changes
- [x] These changes fix #12095  (github issue number if applicable).

<!-- Either: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13281)
<!-- Reviewable:end -->

components/net/fetch/methods.rs

@@ -827,7 +827,7 @@ fn http_network_or_cache_fetch(request: Rc<Request>,
             let mut authorization_value = None;
 
             // Substep 4
-            if let Some(basic) = auth_from_cache(&context.state.auth_cache, &current_url) {
+            if let Some(basic) = auth_from_cache(&context.state.auth_cache, &current_url.origin()) {
                 if !http_request.use_url_credentials || !has_credentials(&current_url) {
                     authorization_value = Some(basic);
                 }

components/net/http_loader.rs

@@ -52,7 +52,7 @@ use time;
 use time::Tm;
 #[cfg(any(target_os = "macos", target_os = "linux", target_os = "windows"))]
 use tinyfiledialogs;
-use url::{Position, Url};
+use url::{Position, Url, Origin};
 use util::prefs::PREFS;
 use util::thread::spawn_named;
 use uuid;
@@ -688,15 +688,15 @@ fn set_auth_header(headers: &mut Headers,
         if let Some(auth) = auth_from_url(url) {
             headers.set(auth);
         } else {
-            if let Some(basic) = auth_from_cache(auth_cache, url) {
+            if let Some(basic) = auth_from_cache(auth_cache, &url.origin()) {
                 headers.set(Authorization(basic));
             }
         }
     }
 }
 
-pub fn auth_from_cache(auth_cache: &Arc<RwLock<AuthCache>>, url: &Url) -> Option<Basic> {
-    if let Some(ref auth_entry) = auth_cache.read().unwrap().entries.get(url) {
+pub fn auth_from_cache(auth_cache: &Arc<RwLock<AuthCache>>, origin: &Origin) -> Option<Basic> {
+    if let Some(ref auth_entry) = auth_cache.read().unwrap().entries.get(&origin.ascii_serialization()) {
         let user_name = auth_entry.user_name.clone();
         let password  = Some(auth_entry.password.clone());
         Some(Basic { username: user_name, password: password })
@@ -1017,13 +1017,15 @@ pub fn load<A, B>(load_data: &LoadData,
         new_auth_header = None;
 
         if let Some(auth_header) = request_headers.get::<Authorization<Basic>>() {
-            if response.status().class() == StatusClass::Success {
+            if response.status().class() == StatusClass::Success ||
+               response.status().class() == StatusClass::Redirection {
                 let auth_entry = AuthCacheEntry {
                     user_name: auth_header.username.to_owned(),
                     password: auth_header.password.to_owned().unwrap(),
                 };
 
-                http_state.auth_cache.write().unwrap().entries.insert(doc_url.clone(), auth_entry);
+                let serialized_origin = doc_url.origin().ascii_serialization();
+                http_state.auth_cache.write().unwrap().entries.insert(serialized_origin, auth_entry);
             }
         }
 

components/net/resource_thread.rs

@@ -466,7 +466,7 @@ impl AuthCache {
 #[derive(RustcDecodable, RustcEncodable, Clone)]
 pub struct AuthCache {
     pub version: u32,
-    pub entries: HashMap<Url, AuthCacheEntry>,
+    pub entries: HashMap<String, AuthCacheEntry>,
 }
 
 pub struct CoreResourceManager {

tests/unit/net/http_loader.rs

@@ -1533,7 +1533,7 @@ fn test_if_auth_creds_not_in_url_but_in_cache_it_sets_it() {
                         password: "test".to_owned(),
                      };
 
-    http_state.auth_cache.write().unwrap().entries.insert(url.clone(), auth_entry);
+    http_state.auth_cache.write().unwrap().entries.insert(url.origin().clone().ascii_serialization(), auth_entry);
 
     let mut load_data = LoadData::new(LoadContext::Browsing, url, &HttpTest);
     load_data.credentials_flag = true;

tests/wpt/metadata/MANIFEST.json

@@ -37479,6 +37479,20 @@
     "deleted": [],
     "deleted_reftests": {},
     "items": {
+      "reftest": {
+        "http/basic-auth-cache-test.html": [
+          {
+            "path": "http/basic-auth-cache-test.html",
+            "references": [
+              [
+                "/http/basic-auth-cache-test-ref.html",
+                "=="
+              ]
+            ],
+            "url": "/http/basic-auth-cache-test.html"
+          }
+        ]
+      },
       "testharness": {
         "dom/lists/DOMTokenList-Iterable.html": [
           {
@@ -37488,7 +37502,20 @@
         ]
       }
     },
-    "reftest_nodes": {}
+    "reftest_nodes": {
+      "http/basic-auth-cache-test.html": [
+        {
+          "path": "http/basic-auth-cache-test.html",
+          "references": [
+            [
+              "/http/basic-auth-cache-test-ref.html",
+              "=="
+            ]
+          ],
+          "url": "/http/basic-auth-cache-test.html"
+        }
+      ]
+    }
   },
   "reftest_nodes": {
     "2dcontext/building-paths/canvas_complexshapes_arcto_001.htm": [

tests/wpt/web-platform-tests/http/basic-auth-cache-test-ref.html

@@ -0,0 +1,9 @@
+<!doctype html>
+<html>
+  <head>
+    <meta charset="utf-8">
+  </head>
+
+  <img src="resources/image.png">
+  <img src="resources/image.png">
+</html>

tests/wpt/web-platform-tests/http/basic-auth-cache-test.html

@@ -0,0 +1,30 @@
+<!doctype html>
+<html id="doc" class="reftest-wait">
+  <head>
+    <meta charset="utf-8">
+  </head>
+
+  <link rel="match" href="basic-auth-cache-test-ref.html">
+
+  <img id="auth" onload="loadNoAuth()">
+  <img id="noauth" onload="removeWait()">
+
+
+  <script type="text/javascript">
+    function loadAuth() {
+        var authUrl = 'http://testuser:testpass@' + window.location.host + '/http/resources/securedimage.py';
+        document.getElementById('auth').src = authUrl;
+    }
+
+    function loadNoAuth() {
+        var noAuthUrl = 'http://' + window.location.host + '/http/resources/securedimage.py';
+        document.getElementById('noauth').src = noAuthUrl;
+    }
+
+    function removeWait() {
+        document.getElementById('doc').className = "";
+    }
+
+    window.onload = loadAuth;
+  </script>
+</html>

tests/wpt/web-platform-tests/http/resources/securedimage.py

@@ -0,0 +1,17 @@
+# -*- coding: utf-8 -
+
+def main(request, response):
+    image_url = str.replace(request.url, "securedimage.py", "image.png")
+
+    if "authorization" not in request.headers:
+        response.status = 401
+        response.headers.set("WWW-Authenticate", "Basic")
+        return response
+    else:
+        auth = request.headers.get("Authorization")
+        if auth != "Basic dGVzdHVzZXI6dGVzdHBhc3M=":
+            response.set_error(403, "Invalid username or password - " + auth)
+            return response
+
+    response.status = 301
+    response.headers.set("Location", image_url)