-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Deny setting operations on __ENV variables from JS code #1722
Comments
Confirmed, I have the same issue. |
The breakage due to the go version seems to be a caching issue - I can't reproduce it and the k6 logo in the second screenshot looks different and we did that and was added in #1615 which was a few months after e1a5c0a. Looking at the screenshots I don't know what I did wrong, but this is good news 🤣 |
After thinking more about this, reviewing #1877, and discussing it a bit on Slack, I am of the opinion that we shouldn't "fix" this. That is, I think that My reasons for that are that there are a ton of corner cases that have to be handled here, for some dubious benefit. For example:
So, yeah, I think we should either close this issue and #1877, or "fix" it by freezing const myScriptOption = (typeof __ENV.something !== 'undefined') ? __ENV.something : 'defaultValue'; This has been broken for a while, but very few people have complained and there's an easy workaround shown above, so my vote is for "fixing" the current behavior in the sense of making it permanent and explicit by freezing |
yeah after we talked about it I also am of the opinion that "freezing" it is better. It will likely not be |
I'm removing this from the v0.32.0 milestone and marking it as not a bug then. |
Also adding here that currently we keep setting a new Line 669 in c949fd8
This currently also technically can return an error which is not checked adn the |
Update — the current agreement from the comments:
#1722 (comment)
Check the original issue description
This code should print "test" and it did up to e1a5c0a and if the golang version is go1.13.8 but not if it is 1.15.3 (or 1.14.6)
This is fairly common pattern to fix your __ENV variables in cases where you didn't provide them but you use them through the script.
The text was updated successfully, but these errors were encountered: