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

Fixes #20372: there is no validation on the variable name in generic method variable_from_smth #1324

Draft
wants to merge 2 commits into
base: branches/rudder/6.1
Choose a base branch
from

Conversation

ncharles
Copy link
Member

@ncharles ncharles commented Dec 6, 2021

@ncharles ncharles requested a review from amousset December 6, 2021 15:47
@ncharles ncharles added the Trigger test Trigger jenkins build label Dec 6, 2021
# @parameter variable_name The variable to define, the full name will be variable_prefix.variable_name
# @parameter value The variable content in JSON format
# @parameter variable_prefix The prefix of the variable name
# @parameter_constraint variable_prefix "regex" : "^[A-z0-9_]+$"
Copy link
Member

Choose a reason for hiding this comment

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

A-z matches non letter chars too.

Copy link
Member

Choose a reason for hiding this comment

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

We should use something like A-Za-z instead.

Copy link
Member

Choose a reason for hiding this comment

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

plus we need to make it possible to use a variable here, I don't remember if we have a mechanism on server part to skip constraint check in this case

Copy link
Member Author

Choose a reason for hiding this comment

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

this mecanism should be server side, otherwise we would need to adapt every regex

…eneric method variable_from_smth

Fixes #20372: there is no validation on the variable name in generic method variable_from_smth
@ncharles ncharles added Trigger test Trigger jenkins build and removed Trigger test Trigger jenkins build labels Dec 9, 2021
@ncharles
Copy link
Member Author

ncharles commented Dec 9, 2021

PR updated with a new commit

@ncharles ncharles added the WIP label Mar 1, 2022
@peckpeck
Copy link
Member

@ncharles is this really a wip ?

@fanf fanf marked this pull request as draft March 28, 2024 18:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Trigger test Trigger jenkins build WIP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants