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.

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

Allow transforms that match Object.prototype methods #83

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

peterdemartini
Copy link

For example, this allows specifying foo|toString or foo|valueOf. Previously this would error due to a syntax error.

…otype

For example, this allows specifying foo|toString. Previously this would error due to a syntax error.
@TomFrost
Copy link
Owner

TomFrost commented May 2, 2020

I like what you're going for here, but I'm hesitant to do anything that would expose executable code that neither Jexl nor the programmer explicitly supplied. One of the design goals of this library is absolute execution safety, in that an expression cannot execute any code outside of a tightly controlled sandbox. By exposing the native Object prototype, expressions gain access to a large library of tools that will change over time, and potentially be unsafe for certain applications. It also makes ports of Jexl to other languages nearly impossible to keep parity with the standard set by this library.

Providing a library of predefined transforms, though, is something I've been chewing on for some time, and would likely solve your use case. I'm not ready to take the leap there yet (it would be incredibly easy to make Jexl as large/heavy/complex as lodash practically overnight) but I'm definitely sensitive to any needs for it!

@TomFrost TomFrost closed this May 2, 2020
@peterdemartini
Copy link
Author

peterdemartini commented May 2, 2020

I think you have it backwards. This PR does not expose the toString on the Object.prototype, it allows a programmer to define a transformation named toString since the current version of Jexl won’t allow it because it exists on Obect.prototype.

@peterdemartini
Copy link
Author

@TomFrost ^^

@peterdemartini
Copy link
Author

@TomFrost I am sorry if I wasn't clear in original description but I'd definitely consider this a bug that is preventing us from creating using a transformer named toString

@TomFrost
Copy link
Owner

Hey @peterdemartini! I'm so sorry, I could have sworn I responded to your reply but I am clearly losing my mind in quarantine. This makes 100% sense and I apologize for my misunderstanding. I'm prepping a release for the near term (top-level functions support in expressions!) so I'll plan on getting this in for it as well!

@TomFrost TomFrost reopened this May 14, 2020
@peterdemartini
Copy link
Author

No worries! Thanks for your help @TomFrost

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants