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.

Sign up for GitHub

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

Jump to bottom

fix(ext/web): webstorage has trap for symbol #21090

Merged
merged 3 commits into from
Nov 14, 2023
Merged

fix(ext/web): webstorage has trap for symbol #21090

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fe4366f
fix webstorage has trap
petamoriken Nov 5, 2023
7ec61f9
rafactor
petamoriken Nov 5, 2023
719d7db
Merge branch 'main' into fix/webstorage-has-trap
petamoriken Nov 5, 2023
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
Prev Previous commit
Next Next commit
rafactor
  • Loading branch information
@petamoriken
petamoriken committed Nov 5, 2023
commit 7ec61f9c7951cc3ee9b99bd104faf6fcce5524e4
45 changes: 23 additions & 22 deletions ext/webstorage/01_webstorage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@ const ops = core.ops;
import * as webidl from "ext:deno_webidl/00_webidl.js";
const primordials = globalThis.__bootstrap.primordials;
const {
SafeArrayIterator,
Symbol,
SymbolFor,
ObjectDefineProperty,
ObjectFromEntries,
ObjectEntries,
ReflectDefineProperty,
ReflectDeleteProperty,
ReflectGet,
ReflectHas,
Proxy,
Expand Down Expand Up @@ -83,53 +83,54 @@ function createStorage(persistent) {

const proxy = new Proxy(storage, {
deleteProperty(target, key) {
if (typeof key == "symbol") {
delete target[key];
} else {
target.removeItem(key);
if (typeof key === "symbol") {
return ReflectDeleteProperty(target, key);
}
target.removeItem(key);
return true;
},

defineProperty(target, key, descriptor) {
if (typeof key == "symbol") {
ObjectDefineProperty(target, key, descriptor);
} else {
target.setItem(key, descriptor.value);
if (typeof key === "symbol") {
return ReflectDefineProperty(target, key, descriptor);
}
target.setItem(key, descriptor.value);
return true;
},
get(target, key) {
if (typeof key == "symbol") return target[key];

get(target, key, receiver) {
if (typeof key === "symbol") {
return target[key];
}
if (ReflectHas(target, key)) {
return ReflectGet(...new SafeArrayIterator(arguments));
} else {
return target.getItem(key) ?? undefined;
return ReflectGet(target, key, receiver);
}
return target.getItem(key) ?? undefined;
},

set(target, key, value) {
if (typeof key == "symbol") {
ObjectDefineProperty(target, key, {
if (typeof key === "symbol") {
return ReflectDefineProperty(target, key, {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

defineProperty is used here, should it be Reflect.set?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if my memory serves me right, there were some very weird behavioural differences that caused issues, but I might be wrong.

value,
configurable: true,
});
} else {
target.setItem(key, value);
}
target.setItem(key, value);
return true;
},

has(target, key) {
if (ReflectHas(target, key)) {
return true;
}
return typeof key === "string" && typeof target.getItem(key) === "string";
},

ownKeys() {
return ops.op_webstorage_iterate_keys(persistent);
},

getOwnPropertyDescriptor(target, key) {
if (arguments.length === 1) {
return undefined;
}
if (ReflectHas(target, key)) {
return undefined;
}
Expand Down
Loading