Follow redirect location as new referrers for nested module imports (#2031)

Fixes #1742
Fixes #2021

cli/compiler.rs

@@ -104,6 +104,7 @@ impl WorkerBehavior for CompilerBehavior {
 #[derive(Debug, Clone)]
 pub struct ModuleMetaData {
   pub module_name: String,
+  pub module_redirect_source_name: Option<String>, // source of redirect
   pub filename: String,
   pub media_type: msg::MediaType,
   pub source_code: Vec<u8>,
@@ -250,6 +251,9 @@ pub fn compile_sync(
                   ) {
                     Ok(serde_json::Value::Object(map)) => ModuleMetaData {
                       module_name: module_meta_data_.module_name.clone(),
+                      module_redirect_source_name: module_meta_data_
+                        .module_redirect_source_name
+                        .clone(),
                       filename: module_meta_data_.filename.clone(),
                       media_type: module_meta_data_.media_type,
                       source_code: module_meta_data_.source_code.clone(),
@@ -342,6 +346,7 @@ mod tests {
 
       let mut out = ModuleMetaData {
         module_name: "xxx".to_owned(),
+        module_redirect_source_name: None,
         filename: "/tests/002_hello.ts".to_owned(),
         media_type: msg::MediaType::TypeScript,
         source_code: include_bytes!("../tests/002_hello.ts").to_vec(),

cli/http_util.rs

@@ -1,6 +1,7 @@
 // Copyright 2018-2019 the Deno authors. All rights reserved. MIT license.
 use crate::errors;
 use crate::errors::DenoError;
+#[cfg(test)]
 use futures::future::{loop_fn, Loop};
 use futures::{future, Future, Stream};
 use hyper;
@@ -45,8 +46,13 @@ fn resolve_uri_from_location(base_uri: &Uri, location: &str) -> Uri {
   } else {
     // assuming path-noscheme | path-empty
     let mut new_uri_parts = base_uri.clone().into_parts();
-    new_uri_parts.path_and_query =
+    let base_uri_path_str = base_uri.path().to_owned();
-      Some(format!("{}/{}", base_uri.path(), location).parse().unwrap());
+    let segs: Vec<&str> = base_uri_path_str.rsplitn(2, "/").collect();
+    new_uri_parts.path_and_query = Some(
+      format!("{}/{}", segs.last().unwrap_or(&""), location)
+        .parse()
+        .unwrap(),
+    );
     Uri::from_parts(new_uri_parts).unwrap()
   }
 }
@@ -61,6 +67,74 @@ pub fn fetch_sync_string(module_name: &str) -> DenoResult<(String, String)> {
   tokio_util::block_on(fetch_string(module_name))
 }
 
+pub enum FetchOnceResult {
+  // (code, maybe_content_type)
+  Code(String, Option<String>),
+  Redirect(http::uri::Uri),
+}
+
+/// Asynchronously fetchs the given HTTP URL one pass only.
+/// If no redirect is present and no error occurs,
+/// yields Code(code, maybe_content_type).
+/// If redirect occurs, does not follow and
+/// yields Redirect(url).
+pub fn fetch_string_once(
+  url: http::uri::Uri,
+) -> impl Future<Item = FetchOnceResult, Error = DenoError> {
+  type FetchAttempt = (Option<String>, Option<String>, Option<FetchOnceResult>);
+  let client = get_client();
+  client
+    .get(url.clone())
+    .map_err(DenoError::from)
+    .and_then(move |response| -> Box<dyn Future<Item = FetchAttempt, Error = DenoError> + Send> {
+      if response.status().is_redirection() {
+        let location_string = response
+          .headers()
+          .get("location")
+          .expect("url redirection should provide 'location' header")
+          .to_str()
+          .unwrap()
+          .to_string();
+        debug!("Redirecting to {}...", &location_string);
+        let new_url = resolve_uri_from_location(&url, &location_string);
+        // Boxed trait object turns out to be the savior for 2+ types yielding same results.
+        return Box::new(
+          future::ok(None).join3(
+            future::ok(None),
+            future::ok(Some(FetchOnceResult::Redirect(new_url))
+          ))
+        );
+      } else if response.status().is_client_error() || response.status().is_server_error() {
+        return Box::new(future::err(
+          errors::new(errors::ErrorKind::Other,
+            format!("Import '{}' failed: {}", &url, response.status()))
+          ));
+      }
+      let content_type = response
+        .headers()
+        .get(CONTENT_TYPE)
+        .map(|content_type| content_type.to_str().unwrap().to_owned());
+      let body = response
+        .into_body()
+        .concat2()
+        .map(|body| String::from_utf8(body.to_vec()).ok())
+        .map_err(DenoError::from);
+      Box::new(body.join3(
+        future::ok(content_type),
+        future::ok(None)
+      ))
+    })
+    .and_then(move |(maybe_code, maybe_content_type, maybe_redirect)| {
+      if let Some(redirect) = maybe_redirect {
+        future::ok(redirect)
+      } else {
+        // maybe_code should always contain code here!
+        future::ok(FetchOnceResult::Code(maybe_code.unwrap(), maybe_content_type))
+      }
+    })
+}
+
+#[cfg(test)]
 /// Asynchronously fetchs the given HTTP URL. Returns (content, media_type).
 pub fn fetch_string(
   module_name: &str,
@@ -182,5 +256,5 @@ fn test_resolve_uri_from_location_relative_3() {
   let url = "http://deno.land/x".parse::<Uri>().unwrap();
   let new_uri = resolve_uri_from_location(&url, "z");
   assert_eq!(new_uri.host().unwrap(), "deno.land");
-  assert_eq!(new_uri.path(), "/x/z");
+  assert_eq!(new_uri.path(), "/z");
 }

cli/isolate.rs

@@ -84,6 +84,15 @@ impl<B: DenoBehavior> Isolate<B> {
           &out.js_source(),
         )?;
 
+        // The resolved module is an alias to another module (due to redirects).
+        // Save such alias to the module map.
+        if out.module_redirect_source_name.is_some() {
+          self.mod_alias(
+            &out.module_redirect_source_name.clone().unwrap(),
+            &out.module_name,
+          );
+        }
+
         self.mod_load_deps(child_id)?;
       }
     }
@@ -117,10 +126,23 @@ impl<B: DenoBehavior> Isolate<B> {
     let out = fetch_module_meta_data_and_maybe_compile(&self.state, url, ".")
       .map_err(RustOrJsError::from)?;
 
+    // Be careful.
+    // url might not match the actual out.module_name
+    // due to the mechanism of redirection.
+
     let id = self
       .mod_new_and_register(true, &out.module_name.clone(), &out.js_source())
       .map_err(RustOrJsError::from)?;
 
+    // The resolved module is an alias to another module (due to redirects).
+    // Save such alias to the module map.
+    if out.module_redirect_source_name.is_some() {
+      self.mod_alias(
+        &out.module_redirect_source_name.clone().unwrap(),
+        &out.module_name,
+      );
+    }
+
     self.mod_load_deps(id)?;
 
     let state = self.state.clone();
@@ -153,6 +175,13 @@ impl<B: DenoBehavior> Isolate<B> {
     Ok(id)
   }
 
+  /// Create an alias for another module.
+  /// The alias could later be used to grab the module
+  /// which `target` points to.
+  fn mod_alias(&self, name: &str, target: &str) {
+    self.state.modules.lock().unwrap().alias(name, target);
+  }
+
   pub fn print_file_info(&self, module: &str) {
     let m = self.state.modules.lock().unwrap();
     m.print_file_info(&self.state.dir, module.to_string());

cli/modules.rs

@@ -12,23 +12,78 @@ pub struct ModuleInfo {
   children: Vec<deno_mod>,
 }
 
+/// A symbolic module entity.
+pub enum SymbolicModule {
+  /// This module is an alias to another module.
+  /// This is useful such that multiple names could point to
+  /// the same underlying module (particularly due to redirects).
+  Alias(String),
+  /// This module associates with a V8 module by id.
+  Mod(deno_mod),
+}
+
+#[derive(Default)]
+/// Alias-able module name map
+pub struct ModuleNameMap {
+  inner: HashMap<String, SymbolicModule>,
+}
+
+impl ModuleNameMap {
+  pub fn new() -> Self {
+    ModuleNameMap {
+      inner: HashMap::new(),
+    }
+  }
+
+  /// Get the id of a module.
+  /// If this module is internally represented as an alias,
+  /// follow the alias chain to get the final module id.
+  pub fn get(&self, name: &str) -> Option<deno_mod> {
+    let mut mod_name = name;
+    loop {
+      let cond = self.inner.get(mod_name);
+      match cond {
+        Some(SymbolicModule::Alias(target)) => {
+          mod_name = target;
+        }
+        Some(SymbolicModule::Mod(mod_id)) => {
+          return Some(*mod_id);
+        }
+        _ => {
+          return None;
+        }
+      }
+    }
+  }
+
+  /// Insert a name assocated module id.
+  pub fn insert(&mut self, name: String, id: deno_mod) {
+    self.inner.insert(name, SymbolicModule::Mod(id));
+  }
+
+  /// Create an alias to another module.
+  pub fn alias(&mut self, name: String, target: String) {
+    self.inner.insert(name, SymbolicModule::Alias(target));
+  }
+}
+
 /// A collection of JS modules.
 #[derive(Default)]
 pub struct Modules {
   pub info: HashMap<deno_mod, ModuleInfo>,
-  pub by_name: HashMap<String, deno_mod>,
+  pub by_name: ModuleNameMap,
 }
 
 impl Modules {
   pub fn new() -> Modules {
     Self {
       info: HashMap::new(),
-      by_name: HashMap::new(),
+      by_name: ModuleNameMap::new(),
     }
   }
 
   pub fn get_id(&self, name: &str) -> Option<deno_mod> {
-    self.by_name.get(name).cloned()
+    self.by_name.get(name)
   }
 
   pub fn get_children(&self, id: deno_mod) -> Option<&Vec<deno_mod>> {
@@ -56,6 +111,10 @@ impl Modules {
     );
   }
 
+  pub fn alias(&mut self, name: &str, target: &str) {
+    self.by_name.alias(name.to_owned(), target.to_owned());
+  }
+
   pub fn resolve_cb(
     &mut self,
     deno_dir: &DenoDir,
@@ -78,8 +137,7 @@ impl Modules {
     }
     let (name, _local_filename) = r.unwrap();
 
-    if let Some(id) = self.by_name.get(&name) {
+    if let Some(child_id) = self.by_name.get(&name) {
-      let child_id = *id;
       info.children.push(child_id);
       return child_id;
     } else {

tests/023_no_ext_with_headers.headers.json

@@ -0,0 +1 @@
+{ "mime_type": "application/javascript" }

tests/023_no_ext_with_headers.test

@@ -0,0 +1,2 @@
+args: tests/023_no_ext_with_headers --reload
+output: tests/023_no_ext_with_headers.out

tests/023_no_ext_with_mime.mime

@@ -1 +0,0 @@
-application/javascript

tests/023_no_ext_with_mime.test

@@ -1,2 +0,0 @@
-args: tests/023_no_ext_with_mime --reload
-output: tests/023_no_ext_with_mime.out

tests/024_import_no_ext_with_headers.test

@@ -0,0 +1,2 @@
+args: tests/024_import_no_ext_with_headers.ts --reload
+output: tests/024_import_no_ext_with_headers.ts.out

tests/024_import_no_ext_with_headers.ts

@@ -0,0 +1 @@
+import "./023_no_ext_with_headers";

tests/024_import_no_ext_with_mime.test

@@ -1,2 +0,0 @@
-args: tests/024_import_no_ext_with_mime.ts --reload
-output: tests/024_import_no_ext_with_mime.ts.out

tests/024_import_no_ext_with_mime.ts

@@ -1 +0,0 @@
-import "./023_no_ext_with_mime";

tests/026_redirect_javascript.js

@@ -0,0 +1,2 @@
+import { value } from "http://localhost:4547/redirects/redirect3.js";
+console.log(value);

tests/026_redirect_javascript.js.out

@@ -0,0 +1 @@
+3 imports 1

tests/026_redirect_javascript.js.test

@@ -0,0 +1,2 @@
+args: tests/026_redirect_javascript.js --reload
+output: tests/026_redirect_javascript.js.out

tests/027_redirect_typescript.ts

@@ -0,0 +1,2 @@
+import { value } from "http://localhost:4547/redirects/redirect4.ts";
+console.log(value);

tests/027_redirect_typescript.ts.out

@@ -0,0 +1 @@
+4 imports 1

tests/027_redirect_typescript.ts.test

@@ -0,0 +1,2 @@
+args: tests/027_redirect_typescript.ts --reload
+output: tests/027_redirect_typescript.ts.out

tests/subdir/redirects/redirect1.js

@@ -0,0 +1 @@
+export const redirect = 1;

tests/subdir/redirects/redirect1.ts

@@ -0,0 +1 @@
+export const redirect = 1;

tests/subdir/redirects/redirect2.js

@@ -0,0 +1 @@
+import "./redirect1.js";

tests/subdir/redirects/redirect3.js

@@ -0,0 +1,2 @@
+import { redirect } from "./redirect1.js";
+export const value = `3 imports ${redirect}`;

tests/subdir/redirects/redirect4.ts

@@ -0,0 +1,2 @@
+import { redirect } from "./redirect1.ts";
+export const value: string = `4 imports ${redirect}`;

tools/http_server.py

@@ -12,6 +12,8 @@ from time import sleep
 
 PORT = 4545
 REDIRECT_PORT = 4546
+ANOTHER_REDIRECT_PORT = 4547
+DOUBLE_REDIRECTS_PORT = 4548
 
 
 class ContentTypeHandler(SimpleHTTPServer.SimpleHTTPRequestHandler):
@@ -99,24 +101,42 @@ def server():
     return s
 
 
-def redirect_server():
+def base_redirect_server(host_port, target_port, extra_path_segment=""):
     os.chdir(root_path)
-    target_host = "http://localhost:%d" % PORT
+    target_host = "http://localhost:%d" % target_port
 
     class RedirectHandler(SimpleHTTPServer.SimpleHTTPRequestHandler):
         def do_GET(self):
             self.send_response(301)
-            self.send_header('Location', target_host + self.path)
+            self.send_header('Location',
+                             target_host + extra_path_segment + self.path)
             self.end_headers()
 
     Handler = RedirectHandler
     SocketServer.TCPServer.allow_reuse_address = True
-    s = SocketServer.TCPServer(("", REDIRECT_PORT), Handler)
+    s = SocketServer.TCPServer(("", host_port), Handler)
     print "redirect server http://localhost:%d/ -> http://localhost:%d/" % (
-        REDIRECT_PORT, PORT)
+        host_port, target_port)
     return s
 
 
+# redirect server
+def redirect_server():
+    return base_redirect_server(REDIRECT_PORT, PORT)
+
+
+# another redirect server pointing to the same port as the one above
+# BUT with an extra subdir path
+def another_redirect_server():
+    return base_redirect_server(
+        ANOTHER_REDIRECT_PORT, PORT, extra_path_segment="/tests/subdir")
+
+
+# redirect server that points to another redirect server
+def double_redirects_server():
+    return base_redirect_server(DOUBLE_REDIRECTS_PORT, REDIRECT_PORT)
+
+
 def spawn():
     # Main http server
     s = server()
@@ -128,6 +148,16 @@ def spawn():
     r_thread = Thread(target=rs.serve_forever)
     r_thread.daemon = True
     r_thread.start()
+    # Another redirect server
+    ars = another_redirect_server()
+    ar_thread = Thread(target=ars.serve_forever)
+    ar_thread.daemon = True
+    ar_thread.start()
+    # Double redirects server
+    drs = double_redirects_server()
+    dr_thread = Thread(target=drs.serve_forever)
+    dr_thread.daemon = True
+    dr_thread.start()
     sleep(1)  # TODO I'm too lazy to figure out how to do this properly.
     return thread