Add unit tests for cookies handling

[fduraffourg]

Thank you for contributing to Servo! Please replace each [ ] by [X] when the step is complete, and replace __ with appropriate data:

• ./mach build -d does not report any errors
• ./mach test-tidy does not report any errors
• These changes fix #9965

Either:

• There are tests for these changes OR
• These changes do not require tests because _____

Add unit tests for the net component about cookies. The tests are generated
with a new mach update-net-cookies command from this repo: https://github.com/abarth/http-state.

This PR also includes two trivial bug fixes about cookie handling.

From all the tests included, the following ones are currently failing:

• cookie_http_state::test_0003
• cookie_http_state::test_0006
• cookie_http_state::test_attribute0004
• cookie_http_state::test_attribute0005
• cookie_http_state::test_attribute0007
• cookie_http_state::test_attribute0008
• cookie_http_state::test_domain0017
• cookie_http_state::test_mozilla0001
• cookie_http_state::test_mozilla0002
• cookie_http_state::test_mozilla0003
• cookie_http_state::test_mozilla0005
• cookie_http_state::test_mozilla0007
• cookie_http_state::test_mozilla0009
• cookie_http_state::test_mozilla0010
• cookie_http_state::test_mozilla0013

test_000[36] and test_mozilla* are failing because there is currently no
method to clean a net::cookie_storage from expired cookies.

test_attribute000[4578] are failing because hyper does not parse the Secure
attribute correctly. I will open an issue on the upstream project.

test_domain0017 fails because the TLD .org is not on the PUB_DOMAINS list.

---

This change is

[highfive]

Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @nox (or someone else) soon.

[highfive]

Heads up! This PR modifies the following files:

• @wafflespeanut: python/servo/testing_commands.py
• @KiChjang: components/net/cookie_storage.rs

[nox]

r? @jdm

Not sure what you wanted Josh, the issue is quite vague.

[highfive]

New code was committed to pull request.

[fduraffourg]

./mach test-tidy was not OK. Sorry about that. I reformated the code.

[jdm]

This is really great! To deal with the tests that currently fail, let's add a text file that lists the expected failures, and when we're generating the appropriate test we can add #[should_panic] to the test. This will allow the unit test suite to pass, and when we change the cookie implementation to fix the failure then the test should report an error until the list of expected failures if updated.
-S-awaiting-review +S-needs-code-changes

Previously, fduraffourg wrote…

./mach test-tidy was not OK. Sorry about that. I reformated the code.

---

Reviewed 2 of 2 files at r1, 2 of 2 files at r2, 1 of 1 files at r3, 4 of 4 files at r4, 1 of 1 files at r5.
Review status: all files reviewed at latest revision, 5 unresolved discussions, some commit checks failed.

---

python/servo/testing_commands.py, line 667 [r2] (raw file):

        run_file = path.abspath(path.join(
                PROJECT_TOPLEVEL_PATH,
                "tests/unit/net/cookie_http_state_utils.py"

"tests", "unit", "net", "cookie_http-state_utils.py"

---

tests/unit/net/cookie_http_state_utils.py, line 19 [r2] (raw file):

            {set_cookies},
            "{location}");
    assert_eq!(result, "{expect}".to_string());

This could be assert_eq!(&result, "{expect}") instead.

---

tests/unit/net/cookie_http_state_utils.py, line 69 [r2] (raw file):

            line = line.decode("utf-8").rstrip()
            if line.startswith("Set-Cookie: "):
                set_cookies.append(line[12:])

Let's do

prefix = "Set-Cookie: "
if line.startswith(prefix):
    set_cookies.append(line[len(prefix):])

for all of these.

---

tests/unit/net/cookie_http_state_utils.py, line 132 [r2] (raw file):

    # Append all tests to unit test file
    tests_dir = os.path.join(repo_dir, "tests/data/parser")

os.path.join(repo_dir, "tests", "data", "parser")

---

tests/unit/net/cookie_http_state_utils.py, line 140 [r2] (raw file):

if __name__ == "__main__":
    update_test_file("/tmp")

Perhaps we can use tempfile.gettempdir() instead?

---

Comments from Reviewable

[jdm]

I filed #11216 about updating the list of TLDs.

[fduraffourg]

Thanks for the review, I'll make the changes.

I also filed SergioBenitez/cookie-rs#46 and SergioBenitez/cookie-rs#47 regarding the parsing fo the Secure attribute.

[highfive]

New code was committed to pull request.

[jdm]

@bors-servo: r+
Fantastic work! Thank you!

Previously, highfive wrote…

New code was committed to pull request.

---

Reviewed 4 of 4 files at r6.
Review status: 4 of 5 files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[bors-servo]

📌 Commit 4fb53c8 has been approved by jdm

[bors-servo]

⌛ Testing commit 4fb53c8 with merge 8bdce6f...

[bors-servo]

💔 Test failed - mac-rel-wpt

[highfive]

  ▶ CRASH [expected TIMEOUT] /workers/semantics/encodings/004.worker
  │ 
  │ thread 'WebWorker for http://web-platform.test:8000/workers/semantics/encodings/004.worker.js' panicked at 'called `Result::unwrap()` on an `Err` value: FromUtf8Error { bytes: [105, 109, 112, 111, 114, 116, 83, 99, 114, 105, 112, 116, 115, 40, 34, 47, 114, 101, 115, 111, 117, 114, 99, 101, 115, 47, 116, 101, 115, 116, 104, 97, 114, 110, 101, 115, 115, 46, 106, 115, 34, 41, 59, 10, 116, 101, 115, 116, 40, 102, 117, 110, 99, 116, 105, 111, 110, 40, 41, 32, 123, 10, 32, 32, 97, 115, 115, 101, 114, 116, 95, 101, 113, 117, 97, 108, 115, 40, 34, 255, 34, 44, 32, 34, 92, 117, 102, 102, 102, 100, 34, 41, 59, 10, 125, 44, 32, 34, 68, 101, 99, 111, 100, 105, 110, 103, 32, 105, 110, 118, 97, 108, 105, 100, 32, 117, 116, 102, 45, 56, 34, 41, 59, 10, 100, 111, 110, 101, 40, 41, 59, 10], error: Utf8Error { valid_up_to: 79 } }', ../src/libcore/result.rs:785
  │ stack backtrace:
  │    1:        0x104907728 - std::sys::backtrace::tracing::imp::write::h9fb600083204ae7f
  │    2:        0x10490dbb5 - std::panicking::default_hook::_$u7b$$u7b$closure$u7d$$u7d$::hca543c34f11229ac
  │    3:        0x10490d7ce - std::panicking::default_hook::hc2c969e7453d080c
  │    4:        0x104049502 - util::panicking::initiate_panic_hook::_$u7b$$u7b$closure$u7d$$u7d$::_$u7b$$u7b$closure$u7d$$u7d$::ha2bd86c312dc8d7a
  │    5:        0x1048f5442 - std::panicking::rust_panic_with_hook::hfe203e3083c2b544
  │    6:        0x10490e176 - std::panicking::begin_panic::h4889569716505182
  │    7:        0x1048f6d38 - std::panicking::begin_panic_fmt::h484cd47786497f03
  │    8:        0x10490ddcf - rust_begin_unwind
  │    9:        0x1049355c0 - core::panicking::panic_fmt::h257ceb0aa351d801
  │   10:        0x103a7aeb7 - core::result::unwrap_failed::hf6c09f6181fb0aed
  │   11:        0x103a79639 - script::dom::dedicatedworkerglobalscope::DedicatedWorkerGlobalScope::run_worker_scope::_$u7b$$u7b$closure$u7d$$u7d$::h15f9e1afc6a0cd00
  │   12:        0x103a7a71f - std::panicking::try::call::hacb70a37009bf82f
  │   13:        0x10491133b - __rust_try
  │   14:        0x1049112d5 - __rust_maybe_catch_panic
  │   15:        0x103a7a9d4 - _<F as std..boxed..FnBox<A>>::call_box::h4edca254635c3085
  │   16:        0x10490cbd8 - std::sys::thread::Thread::new::thread_start::h6f266e069bf4ec2b
  │   17:     0x7fff95828059 - _pthr</span><span class="stdout">ead_body
  └   18:     0x7fff95827fd6 - _pthread_start

[jdm]

@bors-servo: retry

• #11234

[bors-servo]

⌛ Testing commit 4fb53c8 with merge f8b019b...

[bors-servo]

💔 Test failed - mac-rel-wpt

[highfive]

  ▶ CRASH [expected OK] /_mozilla/mozilla/htmllabel-activation.html
  │ 
  │ ERROR:js::rust: Error at :0:0: Error: assert_unreached: disabled submit should not have been activated Reached unreachable code
  │ 
  │ ERROR:compositing::constellation: Panic: ScriptThread: received an event message for a layout channel that is not associated with this script thread.This is a bug.
  │ ERROR:compositing::constellation: Backtrace:
  │ frame #0  - 0x000000010a0ea20e - backtrace::backtrace::trace::h970e6004c9e40bad
  │ frame #1  - 0x000000010a0ea191 - backtrace::capture::Backtrace::new::hcd7fdea4950c7315
  │ frame #2  - 0x0000000109d0d333 - _<F as std..boxed..FnBox<A>>::call_box::h4db35dba00c6e637
  │ frame #3  - 0x000000010a0db84a - util::panicking::initiate_panic_hook::_$u7b$$u7b$closure$u7d$$u7d$::_$u7b$$u7b$closure$u7d$$u7d$::ha2bd86c312dc8d7a
  │ frame #4  - 0x000000010a987502 - std::panicking::rust_panic_with_hook::hfe203e3083c2b544
  │ frame #5  - 0x000000010a9a0236 - std::panicking::begin_panic::h4889569716505182
  │ frame #6  - 0x000000010a988df8 - std::panicking::begin_panic_fmt::h484cd47786497f03
  │ frame #7  - 0x000000010a99fe8f - rust_begin_unwind
  │ frame #8  - 0x000000010a9c7680 - core::panicking::panic_fmt::h257ceb0aa351d801
  │ frame #9  - 0x000000010a9cf185 - core::option::expect_failed::h2d57a5644f345e0b
  │ frame #10 - 0x0000000109d1b150 - script::script_thread::ScriptThread::handle_msg_from_script::hf27da52056b2f2ef
  │ frame #11 - 0x0000000109d6c5d1 - script::script_thread::ScriptThread::handle_msgs::_$u7b$$u7b$closure$u7d$$u7d$::hf428df46c0a22ada
  │ frame #12 - 0x0000000109d5390a - script::script_thread::ScriptThread::handle_msgs::h3b1c1773cd9c5c3d
  │ frame #13 - 0x0000000109d0b807 - std::panicking::try::call::h8d69dc69dac5d183
  │ frame #14 - 0x000000010a9a33fb - __rust_try
  │ frame #15 - 0x000000010a9a3395 - __rust_maybe_catch_panic
  │ frame #16 - 0x0000000109d0cbd4 - _<F as std..boxed..FnBox<A>>::call_box::h2e5eb7eb1a952230
  │ frame #17 - 0x000000010a99ec98 - std::sys::thread::Thread::new::thread_start::h6f266e069bf4ec2b
  │ frame #18 - 0x00007fff86bcf059 - _pthread_body
  │ frame #19 - 0x00007fff86bcefd6 - _pthread_start
  │ 
  └ ERROR:compositing::constellation: Pipeline failed in hard-fail mode.  Crashing!

[jdm]

@bors-servo: retry

• #11099

[bors-servo]

⌛ Testing commit 4fb53c8 with merge c519739...

[bors-servo]

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt, windows