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

[processor] Prevent accidental calls to global functions #5

Conversation

Marrikulus
Copy link

if any of the values is the string 'header' or 'die' then that global function gets called.
This should be done explicitly if indended.

Issue: MOYA-1244

Copy link
Member

@bix0r bix0r left a comment

Choose a reason for hiding this comment

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

LGTM

if any of the values is the string 'header' or 'die'
then that global function gets called.
This should be done explicitly if indended.

Issue: MOYA-1244
@sunkan sunkan requested a review from bix0r December 20, 2023 09:47
@Marrikulus Marrikulus force-pushed the MOYA-1244/restrict-CallableContextProcessor-from-calling-string-global-functions branch from 6014389 to a97aff2 Compare December 20, 2023 09:47
@sunkan
Copy link
Member

sunkan commented Dec 20, 2023

Can you add a test case

@bix0r
Copy link
Member

bix0r commented Dec 20, 2023

@Marrikulus The Processor takes in a LogRecord, not an array. Did you try to run the tests locally?
(Note that the 1.x branch actually takes in an array, but not the develop branch (2.x))

Also, would it be better to try to add this test to the CallableContextTest test instead? You could then use the same "format" for testing this.

Assert that string functions don't get called only closures

Issue: MOYA-1244
@Marrikulus Marrikulus force-pushed the MOYA-1244/restrict-CallableContextProcessor-from-calling-string-global-functions branch from 4c6cf69 to 2c25750 Compare December 20, 2023 12:41
@sunkan sunkan merged commit aafd45e into stefna:develop Dec 20, 2023
6 checks passed
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.

3 participants