Skip to content

Commit

Permalink
fix: avoid global declaration collisions in cjs (denoland#15608)
Browse files Browse the repository at this point in the history
* Use a default stack size * 2 in debug for Windows because swc using so much stack size. We should look into this more later though.
  • Loading branch information
dsherret committed Aug 26, 2022
1 parent 0fe590b commit 376665d
Show file tree
Hide file tree
Showing 15 changed files with 267 additions and 16 deletions.
2 changes: 1 addition & 1 deletion .cargo/config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ rustflags = [
"target-feature=+crt-static",
"-C",
# increase the stack size to prevent swc overflowing the stack in debug
"link-arg=/STACK:1572864",
"link-arg=/STACK:2097152",
]

[target.aarch64-apple-darwin]
Expand Down
3 changes: 3 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 10 additions & 1 deletion cli/npm/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,16 @@ impl NpmCache {
if response.status() == 404 {
bail!("Could not find npm package tarball at: {}", dist.tarball);
} else if !response.status().is_success() {
bail!("Bad response: {:?}", response.status());
let status = response.status();
let maybe_response_text = response.text().await.ok();
bail!(
"Bad response: {:?}{}",
status,
match maybe_response_text {
Some(text) => format!("\n\n{}", text),
None => String::new(),
}
);
} else {
let bytes = response.bytes().await?;

Expand Down
11 changes: 10 additions & 1 deletion cli/npm/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,16 @@ impl NpmRegistryApi {
if response.status() == 404 {
Ok(None)
} else if !response.status().is_success() {
bail!("Bad response: {:?}", response.status());
let status = response.status();
let maybe_response_text = response.text().await.ok();
bail!(
"Bad response: {:?}{}",
status,
match maybe_response_text {
Some(text) => format!("\n\n{}", text),
None => String::new(),
}
);
} else {
let bytes = response.bytes().await?;
let package_info = serde_json::from_slice(&bytes)?;
Expand Down
24 changes: 16 additions & 8 deletions cli/tests/integration/npm_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@ use test_util as util;
use util::assert_contains;
use util::http_server;

// NOTE: It's possible to automatically update the npm registry data in the test server
// by setting the DENO_TEST_UTIL_UPDATE_NPM=1 environment variable.
// NOTE: See how to make test npm packages at ../testdata/npm/README.md

itest!(esm_module {
args: "run --allow-read --unstable npm/esm/main.js",
Expand Down Expand Up @@ -48,6 +47,13 @@ itest!(cjs_sub_path {
http_server: true,
});

itest!(cjs_local_global_decls {
args: "run --allow-read --unstable npm/cjs_local_global_decls/main.ts",
output: "npm/cjs_local_global_decls/main.out",
envs: env_vars(),
http_server: true,
});

itest!(dynamic_import {
args: "run --allow-read --unstable npm/dynamic_import/main.ts",
output: "npm/dynamic_import/main.out",
Expand Down Expand Up @@ -238,12 +244,14 @@ fn ensure_registry_files_local() {
let registry_json_path = registry_dir_path
.join(entry.file_name())
.join("registry.json");
let file_text = std::fs::read_to_string(&registry_json_path).unwrap();
if file_text.contains("https://registry.npmjs.org/") {
panic!(
"file {} contained a reference to the npm registry",
registry_json_path.display(),
);
if registry_json_path.exists() {
let file_text = std::fs::read_to_string(&registry_json_path).unwrap();
if file_text.contains("https://registry.npmjs.org/") {
panic!(
"file {} contained a reference to the npm registry",
registry_json_path.display(),
);
}
}
}
}
Expand Down
18 changes: 18 additions & 0 deletions cli/tests/testdata/npm/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# npm test data

This folder contains test data for npm specifiers.

## Registry

The registry is served by the test server (server in test_util) at
https://localhost:4545/npm/registry/ via the `./registry` folder.

### Updating with real npm packages

1. Set the `DENO_TEST_UTIL_UPDATE_NPM=1` environment variable
2. Run the test and it should download the packages.

### Using a custom npm package

1. Add the custom package to `./registry/@denotest`
2. Reference `npm:@denotest/<your-package-name>` in the tests.
3 changes: 3 additions & 0 deletions cli/tests/testdata/npm/cjs_local_global_decls/main.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Download https://localhost:4545/npm/registry/@denotest/cjs-local-global-decls
Download https://localhost:4545/npm/registry/@denotest/cjs-local-global-decls/1.0.0.tgz
Loaded.
1 change: 1 addition & 0 deletions cli/tests/testdata/npm/cjs_local_global_decls/main.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import "npm:@denotest/[email protected]";
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// package that has all the locals defined
const Buffer = 1, clearImmediate = 1, clearInterval = 1, clearTimeout = 1, global = 1, process = 1, setImmediate = 1, setInterval = 1, setTimeout = 1, globalThis = 1;
const exports = 2;
console.log("Loaded.");
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"name": "@deno/cjs-local-global-decls",
"version": "1.0.0"
}
7 changes: 3 additions & 4 deletions ext/node/02_require.js
Original file line number Diff line number Diff line change
Expand Up @@ -656,11 +656,10 @@
};

Module.wrapper = [
// TODO:
// We provide non standard timer APIs in the CommonJS wrapper
// We provide the non-standard APIs in the CommonJS wrapper
// to avoid exposing them in global namespace.
"(function (exports, require, module, __filename, __dirname, globalThis) { (function (exports, require, module, __filename, __dirname, globalThis, Buffer, clearImmediate, clearInterval, clearTimeout, global, process, setImmediate, setInterval, setTimeout) {",
"\n}).call(this, exports, require, module, __filename, __dirname, globalThis, globalThis.Buffer, globalThis.clearImmediate, globalThis.clearInterval, globalThis.clearTimeout, globalThis.global, globalThis.process, globalThis.setImmediate, globalThis.setInterval, globalThis.setTimeout); })",
"(function (exports, require, module, __filename, __dirname, globalThis) { const { Buffer, clearImmediate, clearInterval, clearTimeout, global, process, setImmediate, setInterval, setTimeout} = globalThis; (function () {",
"\n}).call(this); })",
];
Module.wrap = function (script) {
script = script.replace(/^#!.*?\n/, "");
Expand Down
3 changes: 3 additions & 0 deletions test_util/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ anyhow = "1.0.57"
async-stream = "0.3.3"
atty = "0.2.14"
base64 = "0.13.0"
flate2 = "1.0.24"
futures = "0.3.21"
hyper = { version = "0.14.18", features = ["server", "http1", "http2", "runtime"] }
lazy_static = "1.4.0"
Expand All @@ -25,9 +26,11 @@ parking_lot = "0.12.0"
pretty_assertions = "=1.2.1"
regex = "1.6.0"
reqwest = { version = "0.11.11", default-features = false, features = ["rustls-tls", "stream", "gzip", "brotli", "socks"] }
ring = "0.16.20"
rustls-pemfile = "1.0.0"
serde = { version = "1.0.136", features = ["derive"] }
serde_json = "1.0.79"
tar = "0.4.38"
tokio = { version = "1.19", features = ["full"] }
tokio-rustls = "0.23"
tokio-tungstenite = "0.16"
Expand Down
48 changes: 47 additions & 1 deletion test_util/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use hyper::Request;
use hyper::Response;
use hyper::StatusCode;
use lazy_static::lazy_static;
use npm::custom_npm_cache;
use os_pipe::pipe;
use pretty_assertions::assert_eq;
use regex::Regex;
Expand Down Expand Up @@ -51,6 +52,7 @@ use tokio_tungstenite::accept_async;

pub mod assertions;
pub mod lsp;
mod npm;
pub mod pty;
mod temp_dir;

Expand Down Expand Up @@ -953,7 +955,22 @@ async fn main_server(
}

// serve npm registry files
if req.uri().path().starts_with("/npm/registry/") {
if let Some(suffix) =
req.uri().path().strip_prefix("/npm/registry/@denotest/")
{
// serve all requests to /npm/registry/@deno using the file system
// at that path
match handle_custom_npm_registry_path(suffix) {
Ok(Some(response)) => return Ok(response),
Ok(None) => {} // ignore, not found
Err(err) => {
return Response::builder()
.status(StatusCode::INTERNAL_SERVER_ERROR)
.body(format!("{:#}", err).into());
}
}
} else if req.uri().path().starts_with("/npm/registry/") {
// otherwise, serve based on registry.json and tgz files
let is_tarball = req.uri().path().ends_with(".tgz");
if !is_tarball {
file_path.push("registry.json");
Expand Down Expand Up @@ -985,6 +1002,33 @@ async fn main_server(
};
}

fn handle_custom_npm_registry_path(
path: &str,
) -> Result<Option<Response<Body>>, anyhow::Error> {
let parts = path
.split('/')
.filter(|p| !p.is_empty())
.collect::<Vec<_>>();
let cache = custom_npm_cache()?;
let package_name = format!("@denotest/{}", parts[0]);
if parts.len() == 2 {
if let Some(file_bytes) =
cache.tarball_bytes(&package_name, parts[1].trim_end_matches(".tgz"))
{
let file_resp = custom_headers("file.tgz", file_bytes.to_owned());
return Ok(Some(file_resp));
}
} else if parts.len() == 1 {
if let Some(registry_file) = cache.registry_file(&package_name) {
let file_resp =
custom_headers("registry.json", registry_file.as_bytes().to_vec());
return Ok(Some(file_resp));
}
}

Ok(None)
}

fn should_download_npm_packages() -> bool {
// when this env var is set, it will download and save npm packages
// to the testdata/npm/registry directory
Expand Down Expand Up @@ -1489,6 +1533,8 @@ fn custom_headers(p: &str, body: Vec<u8>) -> Response<Body> {
Some("application/json")
} else if p.ends_with(".wasm") {
Some("application/wasm")
} else if p.ends_with(".tgz") {
Some("application/gzip")
} else {
None
};
Expand Down
Loading

0 comments on commit 376665d

Please sign in to comment.