Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 15 additions & 4 deletions src/js/builtins/ProcessObjectInternals.ts
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,9 @@ export function windowsEnv(
set(_, p, value) {
const k = String(p).toUpperCase();
$assert(typeof p === "string"); // proxy is only string and symbol. the symbol would have thrown by now
value = String(value); // If toString() throws, we want to avoid it existing in the envMapList
// Spec ToString, not String(): Symbol values must throw TypeError (Node.js
// behavior), and a throwing toString() must fire before envMapList is touched.
value = `${value}`;
// Track the key for enumeration if it isn't already there. Don't gate on
// `k in internalEnv`: the proxy-related env-var accessors (HTTP_PROXY,
// HTTPS_PROXY, NO_PROXY and lowercase variants) always exist on
Expand Down Expand Up @@ -530,11 +532,20 @@ export function windowsEnv(
defineProperty(_, p, attributes) {
const k = String(p).toUpperCase();
$assert(typeof p === "string"); // proxy is only string and symbol. the symbol would have thrown by now
if (!(k in internalEnv) && !envMapList.includes(p)) {
// Node coerces env values to strings on defineProperty too, same as assignment.
let coerced: string | undefined;
if ("value" in attributes) {
coerced = `${attributes.value}`;
attributes = { ...attributes, value: coerced };
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Comment thread
robobun marked this conversation as resolved.
$Object.$defineProperty(internalEnv, k, attributes);
// Sync the new value and record the key for enumeration only after a
// successful define, so a failed define leaves no phantom state.
if (!envMapList.includes(p) && !envMapList.some(x => x.toUpperCase() === k)) {
envMapList.push(p);
}
editWindowsEnvVar(k, internalEnv[k]);
return $Object.$defineProperty(internalEnv, k, attributes);
if (coerced !== undefined) editWindowsEnvVar(k, coerced);
return true;
},
getOwnPropertyDescriptor(target, p) {
if (typeof p === "string") {
Expand Down
110 changes: 108 additions & 2 deletions src/jsc/bindings/JSEnvironmentVariableMap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -744,23 +744,129 @@ RefPtr<SharedEnvStore> ensureSharedEnvStoreForWorker(Zig::GlobalObject* globalOb
return store;
}

// Default (non-SHARE_ENV) process.env: plain object for reads, with put/putByIndex/
// defineOwnProperty overridden to toString() the value first so `= 42` reads back
// "42" and `= undefined` reads back "undefined", matching Node.js.
class JSProcessEnvMap final : public JSC::JSNonFinalObject {
public:
using Base = JSC::JSNonFinalObject;

static constexpr unsigned StructureFlags = Base::StructureFlags | JSC::OverridesPut;

template<typename CellType, JSC::SubspaceAccess>
static JSC::GCClient::IsoSubspace* subspaceFor(JSC::VM& vm)
{
STATIC_ASSERT_ISO_SUBSPACE_SHARABLE(JSProcessEnvMap, Base);
return &vm.plainObjectSpace();
}

DECLARE_INFO;

static JSC::Structure* createStructure(JSC::VM& vm, JSC::JSGlobalObject* globalObject, JSC::JSValue prototype)
{
return JSC::Structure::create(vm, globalObject, prototype, JSC::TypeInfo(JSC::ObjectType, StructureFlags), info());
}

static JSProcessEnvMap* create(JSC::VM& vm, JSC::Structure* structure)
{
JSProcessEnvMap* ptr = new (NotNull, JSC::allocateCell<JSProcessEnvMap>(vm)) JSProcessEnvMap(vm, structure);
ptr->finishCreation(vm);
return ptr;
}

static bool put(JSCell*, JSGlobalObject*, JSC::PropertyName, JSC::JSValue, JSC::PutPropertySlot&);
static bool putByIndex(JSCell*, JSGlobalObject*, unsigned, JSC::JSValue, bool shouldThrow);
static bool defineOwnProperty(JSObject*, JSGlobalObject*, JSC::PropertyName, const JSC::PropertyDescriptor&, bool shouldThrow);

private:
JSProcessEnvMap(JSC::VM& vm, JSC::Structure* structure)
: Base(vm, structure)
{
}

void finishCreation(JSC::VM& vm)
{
Base::finishCreation(vm);
}
};

const JSC::ClassInfo JSProcessEnvMap::s_info = { "ProcessEnv"_s, &Base::s_info, nullptr, nullptr, CREATE_METHOD_TABLE(JSProcessEnvMap) };

bool JSProcessEnvMap::put(JSCell* cell, JSGlobalObject* globalObject, PropertyName propertyName, JSValue value, PutPropertySlot& slot)
{
VM& vm = JSC::getVM(globalObject);
auto scope = DECLARE_THROW_SCOPE(vm);

if (propertyName.isSymbol()) {
RELEASE_AND_RETURN(scope, Base::put(cell, globalObject, propertyName, value, slot));
}

JSString* string = value.toString(globalObject);
RETURN_IF_EXCEPTION(scope, false);

RELEASE_AND_RETURN(scope, Base::put(cell, globalObject, propertyName, string, slot));
}

bool JSProcessEnvMap::putByIndex(JSCell* cell, JSGlobalObject* globalObject, unsigned index, JSValue value, bool shouldThrow)
{
VM& vm = JSC::getVM(globalObject);
auto scope = DECLARE_THROW_SCOPE(vm);

JSString* string = value.toString(globalObject);
RETURN_IF_EXCEPTION(scope, false);

RELEASE_AND_RETURN(scope, Base::putByIndex(cell, globalObject, index, string, shouldThrow));
}

bool JSProcessEnvMap::defineOwnProperty(JSObject* object, JSGlobalObject* globalObject, PropertyName propertyName, const PropertyDescriptor& descriptor, bool shouldThrow)
{
VM& vm = JSC::getVM(globalObject);
auto scope = DECLARE_THROW_SCOPE(vm);

if (propertyName.isSymbol() || !descriptor.isDataDescriptor() || !descriptor.value()) {
RELEASE_AND_RETURN(scope, Base::defineOwnProperty(object, globalObject, propertyName, descriptor, shouldThrow));
}

JSString* string = descriptor.value().toString(globalObject);
RETURN_IF_EXCEPTION(scope, false);

PropertyDescriptor coerced(descriptor);
coerced.setValue(string);
RELEASE_AND_RETURN(scope, Base::defineOwnProperty(object, globalObject, propertyName, coerced, shouldThrow));
}

JSObject* createProcessEnvMapObject(Zig::GlobalObject* globalObject)
{
VM& vm = globalObject->vm();
auto* structure = JSProcessEnvMap::createStructure(vm, globalObject, globalObject->objectPrototype());
return JSProcessEnvMap::create(vm, structure);
}
Comment thread
robobun marked this conversation as resolved.

bool isEnvironmentVariablesMapObject(const JSC::ClassInfo* info)
{
return info == JSProcessEnvMap::info() || info == JSSharedEnvMap::info();
}

JSValue createEnvironmentVariablesMap(Zig::GlobalObject* globalObject)
{
VM& vm = globalObject->vm();
auto scope = DECLARE_THROW_SCOPE(vm);

void* list;
size_t count = Bun__getEnvCount(globalObject, &list);
#if OS(WINDOWS)
// The windowsEnv Proxy's set/defineProperty traps coerce, and windowsEnv()
// stores a toJSON function on this object directly: keep it a plain object.
JSC::JSObject* object = nullptr;
if (count < 63) {
object = constructEmptyObject(globalObject, globalObject->objectPrototype(), count);
} else {
object = constructEmptyObject(globalObject, globalObject->objectPrototype());
}

#if OS(WINDOWS)
JSArray* keyArray = constructEmptyArray(globalObject, nullptr, count);
RETURN_IF_EXCEPTION(scope, {});
#else
JSC::JSObject* object = createProcessEnvMapObject(globalObject);
#endif

static NeverDestroyed<String> TZ = MAKE_STATIC_STRING_IMPL("TZ");
Expand Down
8 changes: 8 additions & 0 deletions src/jsc/bindings/JSEnvironmentVariableMap.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,14 @@ namespace Bun {

JSC::JSValue createEnvironmentVariablesMap(Zig::GlobalObject* globalObject);

// Bare JSProcessEnvMap (no OS-env getters, no Windows Proxy wrap): coerces
// assigned values to strings like Node.js. For worker env: {...} snapshots.
JSC::JSObject* createProcessEnvMapObject(Zig::GlobalObject* globalObject);

// True for JSProcessEnvMap/JSSharedEnvMap classInfo; both structured-clone like
// a plain object (Node.js structuredClone(process.env) succeeds).
bool isEnvironmentVariablesMapObject(const JSC::ClassInfo*);

// worker_threads SHARE_ENV: a `process.env` whose reads/writes/enumeration go
// through the SharedEnvStore of the tree its global belongs to.
JSC::JSValue createSharedEnvironmentVariablesMap(Zig::GlobalObject* globalObject);
Expand Down
7 changes: 6 additions & 1 deletion src/jsc/bindings/ZigGlobalObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -566,12 +566,17 @@ extern "C" JSC::JSGlobalObject* Zig__GlobalObject__create(void* console_client,
strings.append(jsString(vm, value));
}

auto env = JSC::constructEmptyObject(globalObject, globalObject->objectPrototype(), size >= JSFinalObject::maxInlineCapacity ? JSFinalObject::maxInlineCapacity : size);
// JSProcessEnvMap is a JSNonFinalObject, so indexed putDirectMayBeIndex
// routes through the method-table defineOwnProperty with a throw scope;
// a top scope keeps exception-check validation balanced.
auto scope = DECLARE_TOP_EXCEPTION_SCOPE(vm);
auto env = Bun::createProcessEnvMapObject(globalObject);
Comment thread
claude[bot] marked this conversation as resolved.
size_t i = 0;
for (auto k : map) {
// They can have environment variables with numbers as keys.
// So we must use putDirectMayBeIndex to handle that.
env->putDirectMayBeIndex(globalObject, JSC::Identifier::fromString(vm, WTF::move(k.key)), strings.at(i++));
scope.assertNoException();
}
globalObject->m_processEnvObject.set(vm, globalObject, env);
} else if (options.sharedEnvStore) {
Expand Down
5 changes: 4 additions & 1 deletion src/jsc/bindings/webcore/SerializedScriptValue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@
#include "CryptoKeyType.h"
#include "JSNodePerformanceHooksHistogram.h"
#include "../napi.h"
#include "../JSEnvironmentVariableMap.h"
#include <limits>
#include <algorithm>

Expand Down Expand Up @@ -2812,7 +2813,9 @@ SerializationReturnCode CloneSerializer::serialize(JSValue in)
// like a plain object from JS's perspective (matches Node.js).
// ObjectPrototype is allowed because %Object.prototype% is an immutable
// prototype exotic object that the spec carves out of this rejection.
if (inObject->classInfo() != JSFinalObject::info() && inObject->classInfo() != Zig::NapiPrototype::info() && inObject->classInfo() != JSC::ObjectPrototype::info())
// process.env (JSProcessEnvMap / JSSharedEnvMap) is allowed because
// Node.js structured-clones it as a plain object of its string entries.
if (inObject->classInfo() != JSFinalObject::info() && inObject->classInfo() != Zig::NapiPrototype::info() && inObject->classInfo() != JSC::ObjectPrototype::info() && !Bun::isEnvironmentVariablesMapObject(inObject->classInfo()))
return SerializationReturnCode::DataCloneError;
inputObjectStack.append(inObject);
indexStack.append(0);
Expand Down
34 changes: 19 additions & 15 deletions test/cli/run/env.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -901,21 +901,25 @@ for (const shell of ["system", "bun"]) {
});
}

const todoOnPosix = process.platform !== "win32" ? test.todo : test;
todoOnPosix("setting process.env coerces the value to a string", () => {
// @ts-expect-error
process.env.SET_TO_TRUE = true;
let did_call = 0;
// @ts-expect-error
process.env.SET_TO_BUN = {
toString() {
did_call++;
return "bun!";
},
};
expect(process.env.SET_TO_TRUE).toBe("true");
expect(process.env.SET_TO_BUN).toBe("bun!");
expect(did_call).toBe(1);
test("setting process.env coerces the value to a string", () => {
Comment thread
robobun marked this conversation as resolved.
try {
// @ts-expect-error
process.env.SET_TO_TRUE = true;
let did_call = 0;
// @ts-expect-error
process.env.SET_TO_BUN = {
toString() {
did_call++;
return "bun!";
},
};
expect(process.env.SET_TO_TRUE).toBe("true");
expect(process.env.SET_TO_BUN).toBe("bun!");
expect(did_call).toBe(1);
} finally {
delete process.env.SET_TO_TRUE;
delete process.env.SET_TO_BUN;
}
});

test("NODE_ENV=test loads .env.test even when .env.production exists", () => {
Expand Down
Loading
Loading