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

Fixing #5743 - Automation Expressions SQL Handling #5744

Closed
wants to merge 4 commits into from

Conversation

TheWitness
Copy link
Member

Automation Expressions do not properly handle blank match strings

Automation Expressions do not properly handle blank match strings
@gvde
Copy link
Contributor

gvde commented May 7, 2024

As mentioned in my comment TheWitness@79799f3 this doesn't fix the issue.

Properly handle the regular expression.
Copy link
Contributor

@gvde gvde left a comment

Choose a reason for hiding this comment

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

This still has a lot of issues:

  1. You have to use db_qstr on the expression instead of preq_quote. I would recommend a prepared statement, anyway, because it saves you all that trouble.
  2. You still first check if the qsysDescr, qsysObject or qsysName values from the device are empty or not. If it's empty, you omit the condition meaning match anything. This means, if I have a device rule matching for "Cisco" in sysDescr, and leaving the matches for sysObject and sysName empty, any device which has an empty sysDescr set will match the rule in addition to all devices containing "Cisco". I am pretty sure that this is not what the average user would expect. IMHO devices with an empty sysdesc, sysoid or sysname should not match just like any device which woul have an other, non-empty string.

After discussion with user, going to use prepared statement.
* After a sample run, found that the previous change was not correct.  This new changes operates correctly.
@TheWitness TheWitness closed this May 10, 2024
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.

None yet

2 participants