-
-
Notifications
You must be signed in to change notification settings - Fork 407
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
Restrict Direct Construction of JsValue
to Ensure Invariants
#3761
Comments
Is this issue still available? |
@MartiPresa Sure! Let me assign it to you. |
As a small guidance, the enum you need to work on is here: boa/core/engine/src/value/mod.rs Lines 62 to 81 in 848103a
Ideally, the new repr of #[derive(Trace, Finalize, Debug, Clone)]
struct JsValue {
inner: ValueRepr
}
#[derive(Finalize, Debug, Clone)]
enum ValueRepr {
Null,
Undefined,
/* ... rest of the variants ... */
} Then, impl JsValue {
pub variant(&self) -> JsValueVariant<'_> {
/// Convert from &ValueRepr to JsValueVariant<'_>
}
} The rest is fixing all the compilation errors related to the new API, like replacing direct matches of |
I will start checking on this.
Thanx!
El lun, 8 abr 2024 a la(s) 4:40 p.m., José Julián Espina (
***@***.***) escribió:
… As a small guidance, the enum you need to work on is here:
https://github.com/boa-dev/boa/blob/848103ae3fbf47b4224679adcb14350344920140/core/engine/src/value/mod.rs#L62-L81
Ideally, the new repr of JsValue would be:
#[derive(Trace, Finalize, Debug, Clone)]struct JsValue {
inner: ValueRepr}
#[derive(Finalize, Debug, Clone)]enum ValueRepr {
Null,
Undefined,
/* ... rest of the variants ... */}
Then, JsValue would need to expose a method to get an enum of the inner
variant:
impl JsValue {
pub variant(&self) -> JsValueVariant<'_>{
/// Convert from ValueRepr to JsValueVariant<'_>
}}
The rest is fixing all the compilation errors related to the new API, like
replacing direct matches of JsValue variables to matches on JsValueVariant
and calls to variant().
—
Reply to this email directly, view it on GitHub
<#3761 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AYQMU3JRGTZX4IGZKCEBCH3Y4LXCBAVCNFSM6AAAAABFFCKW2WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANBTGUYTMNJXGM>
.
You are receiving this because you were assigned.Message ID:
***@***.***>
|
The problem lies in the ability for users to manually construct a
JsValue::Rational
from af64
value that fits within ani32
, asJsValue
is a public enum.A proposed solution is to introduce a struct that encapsulates the
JsValue
enum. By doing so, construction ofJsValue::Rational
would be limited to its respective constructor functions.This would allow us to remove many checks that are done to see if a
JsValue::Rational
f64 value can fit in ai32
.Additional Information
Related to #1373 , something similar was done in #1830 here
The text was updated successfully, but these errors were encountered: