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

Remove the network.http.redirection-limit preference. #14204

Merged
merged 1 commit into from Nov 14, 2016
Merged
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

Remove the network.http.redirection-limit preference.

The Fetch standard defines this value as twenty; there is no good reason to
allow changing that at runtime.
  • Loading branch information
Ms2ger committed Nov 14, 2016
commit 56dd6417e63e8459b1c508a56044e00aeaaaabe5
@@ -53,7 +53,6 @@ use time::Tm;
#[cfg(any(target_os = "macos", target_os = "linux", target_os = "windows"))]
use tinyfiledialogs;
use url::{Position, Url, Origin};
use util::prefs::PREFS;
use util::thread::spawn_named;
use uuid;

@@ -890,7 +889,6 @@ pub fn load<A, B>(load_data: &LoadData,
cancel_listener: &CancellationListener,
swmanager_chan: Option<IpcSender<CustomResponseMediator>>)
-> Result<StreamedResponse, LoadError> where A: HttpRequest + 'static, B: UIProvider {
let max_redirects = PREFS.get("network.http.redirection-limit").as_i64().unwrap() as u32;
let mut iters = 0;
// URL of the document being loaded, as seen by all the higher-level code.
let mut doc_url = load_data.url.clone();
@@ -939,7 +937,7 @@ pub fn load<A, B>(load_data: &LoadData,
doc_url = secure_url(&doc_url);
}

if iters > max_redirects {
if iters > 20 {
return Err(LoadError::new(doc_url, LoadErrorType::MaxRedirects(iters - 1)));
}

@@ -52,7 +52,6 @@
"layout.text-orientation.enabled": false,
"layout.viewport.enabled": false,
"layout.writing-mode.enabled": false,
"network.http.redirection-limit": 20,
"network.mime.sniff": false,
"shell.builtin-key-shortcuts.enabled": false,
"shell.homepage": "https://servo.org",
@@ -53,7 +53,6 @@
"layout.text-orientation.enabled": false,
"layout.viewport.enabled": false,
"layout.writing-mode.enabled": false,
"network.http.redirection-limit": 20,
"network.mime.sniff": false,
"shell.builtin-key-shortcuts.enabled": true,
"shell.homepage": "http://servo.org",
@@ -42,7 +42,6 @@ use std::sync::atomic::{AtomicBool, Ordering};
use std::sync::mpsc::Receiver;
use std::thread;
use url::Url;
use util::prefs::{self, PREFS};

const DEFAULT_USER_AGENT: &'static str = "Test-agent";

@@ -1256,45 +1255,6 @@ fn test_load_succeeds_with_a_redirect_loop() {
ResponseBody::Done(b"Success".to_vec()));
}

#[test]
fn test_load_errors_when_there_is_too_many_redirects() {
struct Factory;

impl HttpRequestFactory for Factory {
type R = MockRequest;

fn create(&self, url: Url, _: Method, _: Headers) -> Result<MockRequest, LoadError> {
if url.domain().unwrap() == "mozilla.com" {
Ok(MockRequest::new(ResponseType::Redirect(format!("{}/1", url))))
} else {
panic!("unexpected host {:?}", url)
}
}
}

let url = Url::parse("http://mozilla.com").unwrap();
let load_data = LoadData::new(LoadContext::Browsing, url.clone(), &HttpTest);

let http_state = HttpState::new();
let ui_provider = TestProvider::new();

let redirect_limit = 13.;
PREFS.set("network.http.redirection-limit",
prefs::PrefValue::Number(redirect_limit));

match load(&load_data, &ui_provider, &http_state, None, &Factory,
DEFAULT_USER_AGENT.into(), &CancellationListener::new(None), None) {
Err(LoadError { error: LoadErrorType::MaxRedirects(num_redirects),
url, .. }) => {
assert_eq!(num_redirects, redirect_limit as u32);
assert_eq!(url.domain().unwrap(), "mozilla.com");
}
_ => panic!("expected max redirects to fail")
}

PREFS.reset("network.http.redirection-limit");
}

#[test]
fn test_load_follows_a_redirect() {
struct Factory;
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.