Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace almost all uses of `referer` with `referrer` #13286

Closed
jdm opened this issue Sep 15, 2016 · 6 comments
Closed

Replace almost all uses of `referer` with `referrer` #13286

jdm opened this issue Sep 15, 2016 · 6 comments

Comments

@jdm
Copy link
Member

@jdm jdm commented Sep 15, 2016

There is only valid misspelling of referrer that belongs in our code, and that is the historically-misspelled HTTP Referer header. Any other places in the code that misspell it should be corrected, such as the referer_url in the RequestInit structure.

@highfive
Copy link

@highfive highfive commented Sep 15, 2016

Please make a comment here if you intend to work on this issue. Thank you!

@jdm
Copy link
Member Author

@jdm jdm commented Sep 15, 2016

components/net/fetch/methods.rs
159:    // TODO this step (referer policy)
160:    // currently the clients themselves set referer policy in RequestInit
168:    if *request.referer.borrow() != Referer::NoReferer {
174:                                                      request.referer.borrow_mut().take(),
176:        *request.referer.borrow_mut() = Referer::from_url(referrer_url);
748:    match *http_request.referer.borrow() {
750:        Referer::RefererUrl(ref http_request_referer) =>
751:            http_request.headers.borrow_mut().set(RefererHeader(http_request_referer.to_string())),
753:            // it should be impossible for referer to be anything else during fetching
799:    // this can only be uncommented when the referer header is set, else it crashes
1115:    *preflight.referer.borrow_mut() = request.referer.borrow().clone();

components/net/http_loader.rs
679:    if let Some(referer_val) = referrer_url.clone() {
680:        headers.set(Referer(referer_val.into_string()));

components/net_traits/request.rs
44:/// A [referer](https://fetch.spec.whatwg.org/#concept-request-referrer)
48:    /// Default referer if nothing is specified
135:    pub referer_url: Option<Url>,
167:    pub referer: RefCell<Referer>,
208:            referer: RefCell::new(Referer::Client),
241:        *req.referer.borrow_mut() = if let Some(url) = init.referer_url {
274:            referer: RefCell::new(Referer::Client),

components/script/dom/headers.rs
320:         "referer", "te", "trailer", "transfer-encoding",

components/script/dom/request.rs
157:        request.referer = temporary_request.referer;
185:                *request.referer.borrow_mut() = NetTraitsRequestReferer::Client;
196:                *request.referer.borrow_mut() = NetTraitsRequestReferer::NoReferer;
210:                            *request.referer.borrow_mut() = NetTraitsRequestReferer::Client;
218:                            *request.referer.borrow_mut() = NetTraitsRequestReferer::RefererUrl(parsed_referrer);
541:        let referrer = r.referer.borrow();

components/script/dom/xmlhttprequest.rs
596:            referer_url: self.referrer_url.clone(),
@jdm
Copy link
Member Author

@jdm jdm commented Sep 15, 2016

components/net/fetch/methods.rs
168:    if *request.referer.borrow() != Referer::NoReferer {
176:        *request.referer.borrow_mut() = Referer::from_url(referrer_url);
749:        Referer::NoReferer => (),
750:        Referer::RefererUrl(ref http_request_referer) =>
752:        Referer::Client =>

components/net_traits/request.rs
46:pub enum Referer {
47:    NoReferer,
50:    RefererUrl(Url)
167:    pub referer: RefCell<Referer>,
208:            referer: RefCell::new(Referer::Client),
242:            Referer::RefererUrl(url)
244:            Referer::NoReferer
274:            referer: RefCell::new(Referer::Client),
328:impl Referer {
331:            Referer::NoReferer | Referer::Client => None,
332:            Referer::RefererUrl(ref url) => Some(url)
337:            Referer::RefererUrl(url)
339:            Referer::NoReferer
343:        let mut new = Referer::Client;
346:            Referer::NoReferer | Referer::Client => None,
347:            Referer::RefererUrl(url) => Some(url)

components/script/dom/request.rs
31:use net_traits::request::Referer as NetTraitsRequestReferer;
185:                *request.referer.borrow_mut() = NetTraitsRequestReferer::Client;
196:                *request.referer.borrow_mut() = NetTraitsRequestReferer::NoReferer;
210:                            *request.referer.borrow_mut() = NetTraitsRequestReferer::Client;
218:                            *request.referer.borrow_mut() = NetTraitsRequestReferer::RefererUrl(parsed_referrer);
543:            &NetTraitsRequestReferer::NoReferer => String::from("no-referrer"),
544:            &NetTraitsRequestReferer::Client => String::from("client"),
545:            &NetTraitsRequestReferer::RefererUrl(ref u) => {
@jdm
Copy link
Member Author

@jdm jdm commented Sep 15, 2016

I've excluded uses of the Referer header type that's defined in the hyper crate, because that represents the actual HTTP header which is intentionally misspelled.

@6112
Copy link
Contributor

@6112 6112 commented Sep 16, 2016

@jdm

I'd like to work on this.

After making the changes to the files mentioned, and updating some unit tests, I still found some other occurrences of "referrer" in unit tests, mostly for variable & function names. Should I correct those too?

I didn't change the string at components/script/dom/headers.rs:320, since it seems to identify the Referer HTTP header.

tests/unit/net/http_loader.rs:
1628:fn assert_referer_header_matches(origin_info: &LoadOrigin,
1638:    let mut referer_headers = Headers::new();
1639:    referer_headers.set(Referer(expected_referrer.to_owned()));
1645:                     expected_headers: referer_headers,
1651:fn assert_referer_header_not_included(origin_info: &LoadOrigin, request_url: &str) {
1664:            headers_not_expected: vec!["Referer".to_owned()],
1670:fn test_referer_set_to_origin_with_origin_policy() {
1681:    assert_referer_header_matches(&origin_info, request_url, expected_referrer);
1685:fn test_referer_set_to_ref_url_with_sameorigin_policy_same_orig() {
1696:    assert_referer_header_matches(&origin_info, request_url, expected_referrer);
1700:fn test_no_referer_set_with_sameorigin_policy_cross_orig() {
1710:    assert_referer_header_not_included(&origin_info, request_url);
1714:fn test_referer_set_to_stripped_url_with_unsafeurl_policy() {
1724:    assert_referer_header_matches(&origin_info, request_url, expected_referrer);
1728:fn test_referer_with_originwhencrossorigin_policy_cross_orig() {
1739:    assert_referer_header_matches(&origin_info, request_url, expected_referrer);
1743:fn test_referer_with_originwhencrossorigin_policy_same_orig() {
1754:    assert_referer_header_matches(&origin_info, request_url, expected_referrer);
1758:fn test_http_to_https_considered_cross_origin_for_referer_header_logic() {
1769:    assert_referer_header_matches(&origin_info, request_url, expected_referrer);
1773:fn test_referer_set_to_ref_url_with_noreferrerwhendowngrade_policy_https_to_https() {
1784:    assert_referer_header_matches(&origin_info, request_url, expected_referrer);
1788:fn test_no_referer_set_with_noreferrerwhendowngrade_policy_https_to_http() {
1798:    assert_referer_header_not_included(&origin_info, request_url)
1802:fn test_referer_set_to_ref_url_with_noreferrerwhendowngrade_policy_http_to_https() {
1813:    assert_referer_header_matches(&origin_info, request_url, expected_referrer);
1817:fn test_referer_set_to_ref_url_with_noreferrerwhendowngrade_policy_http_to_http() {
1828:    assert_referer_header_matches(&origin_info, request_url, expected_referrer);
1843:    assert_referer_header_matches(&origin_info, request_url, expected_referrer);
1857:    assert_referer_header_not_included(&origin_info, request_url);
1872:    assert_referer_header_matches(&origin_info, request_url, expected_referrer);
1887:    assert_referer_header_matches(&origin_info, request_url, expected_referrer);
1891:fn test_no_referer_set_with_noreferrer_policy() {
1901:    assert_referer_header_not_included(&origin_info, request_url)
@jdm
Copy link
Member Author

@jdm jdm commented Sep 16, 2016

Yes, updating the unit tests too would be great!

@6112 6112 mentioned this issue Sep 16, 2016
4 of 5 tasks complete
bors-servo added a commit that referenced this issue Sep 16, 2016
Fix most typoes for: "referer" -> "referrer"

Replace most uses of the word "referer" with "referrer", except for `hyper::header::Referer`. Also update the unit tests to compile & pass after those changes.

---
<!-- 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
- [X] These changes fix #13286

<!-- Either: -->
- [ ]  There are tests for these changes OR
- [X] These changes do not require tests because they're only typo fixes.

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- 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/13294)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Sep 16, 2016
Fix most typoes for: "referer" -> "referrer"

Replace most uses of the word "referer" with "referrer", except for `hyper::header::Referer`. Also update the unit tests to compile & pass after those changes.

---
<!-- 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
- [X] These changes fix #13286

<!-- Either: -->
- [ ]  There are tests for these changes OR
- [X] These changes do not require tests because they're only typo fixes.

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- 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/13294)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

4 participants
You can’t perform that action at this time.