add auth cache to resources + set auth header from it if url does not have creds

[ddefisher]

initial attempt of

• in resource_thread.rs, define an HTTP authorization cache storage (username, password, URL) and instantiate it like the cookie_storage member (inside an Arc<Rwlock<>> value, to enable sharing it between threads)

• in modify_request_headers in http_loader.rs, implement the remaining pieces of step 12(13?) of the appropriate specification using this new authorization cache.

  for the NCSU student project Implement HTTP authorization UI and persistent sessions.

---

This change is

[highfive]

Warning

• These commits modify net code, but no tests are modified. Please consider adding a test!

[jdm]

This is a great start! Please be sure to:

• run ./mach test-tidy and fix the stylistic errors that it reports
• run ./mach test-unit -p net and fix the build errors that it reports

The biggest issue to address here is that the cache needs to support more than a single entry. We actually want a hashmap of serialized URLs to username/password credentials, so we can easily determine if we have stored credentials for a given URL or not. Additionally, we should write a unit test that demonstrates that this new code works as expected - the ones in tests/unit/net/http_loader.rs can be used as a model here.

[ddefisher]

Still need to fix the http_loader unit test stuff(it's failing because a lot of functions argument number increased by 1) as well as add a test. Hoping I could get some comments if I am on the right track on the other stuff though. Thanks!

[jdm]

Definitely on the right track!
-S-awaiting-review +S-needs-code-changes

---

Reviewed 2 of 2 files at r2.
Review status: all files reviewed at latest revision, 8 unresolved discussions, some commit checks failed.

---

components/net/http_loader.rs, line 558 [r2] (raw file):
nit: please remove this newline.

---

components/net/http_loader.rs, line 560 [r2] (raw file):
nit: please remove this newline.

---

components/net/http_loader.rs, line 563 [r2] (raw file):
nit: } else {

---

components/net/http_loader.rs, line 564 [r2] (raw file):
nit: please remove this newline.

---

components/net/http_loader.rs, line 565 [r2] (raw file):
I think this is clearer as:

if let Some(ref auth_entry) = auth_cache.read().unwrap().get(url) {
    auth_from_entry(&auth_entry, headers);
}

---

components/net/http_loader.rs, line 573 [r2] (raw file):
nit: remove the space before (

---

components/net/http_loader.rs, line 577 [r2] (raw file):
No need for if let here.

headers.set(Authorization(Basic { username: user_name, password: password }));

---

components/net/resource_thread.rs, line 277 [r2] (raw file):
Let's remove it if there aren't any methods.

---

Comments from the review on Reviewable.io

[jdm]

Great work! Still looking for a unit test that shows the changes working, and ./mach test-tidy is failing again.
-S-awaiting-review

---

Reviewed 3 of 3 files at r3.
Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.

---

Comments from the review on Reviewable.io

[ddefisher]

added a unit test and tidied up

[jdm]

Great! There are a couple small changes left, but you can squash everything together once they're complete.
-S-fails-tidy -S-needs-tests -S-awaiting-review +S-needs-code-changes +S-needs-squash

---

Reviewed 2 of 2 files at r4.
Review status: all files reviewed at latest revision, 4 unresolved discussions.

---

components/net/resource_thread.rs, line 277 [r4] (raw file):
I don't think this constructor is particularly useful. Let's remove it.

---

tests/unit/net/http_loader.rs, line 1486 [r4] (raw file):
Let's just use this instead:

AuthCacheEntry {
    user_name: "username".to_owned(),
    password: "test".to_owned(),
}

---

tests/unit/net/http_loader.rs, line 1492 [r4] (raw file):
No need for this clone.

---

tests/unit/net/http_loader.rs, line 1494 [r4] (raw file):
These are the default values of data and method so we can remove those lines.

---

Comments from the review on Reviewable.io

[jdm]

@bors-servo: r+
Thanks!

---

Reviewed 2 of 2 files at r5.
Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from the review on Reviewable.io

[bors-servo]

📌 Commit d49d3b0 has been approved by jdm

[bors-servo]

⌛ Testing commit d49d3b0 with merge 7f944af...

[bors-servo]

☀️ Test successful - android, arm32, gonk, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt, status-appveyor