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

Core: Options - Validate keys when loading an OptionSet or OptionList from text #3274

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

Conversation

agilbert1412
Copy link
Collaborator

What is this fixing or adding?

Currently, if loading an OptionSet or OptionKey from an iterable, it validates that the keys are all valid, using the valid_keys optional attribute (if set).

image

But when loading from_text(), it does not do this validation. This leads to the problem that, depending on how the yaml is formatted (and, presumably, other sources like unit tests could have the same issue), the valid keys can be bypassed entirely

image

How was this tested?

Ran the unit tests, made sure nothing was broken.
Ran my own test case (from the original problem that made me discover this), and it now validates my keys properly

If this makes graphical changes, please attach screenshots.

image

@github-actions github-actions bot added the affects: core Issues/PRs that touch core and may need additional validation. label May 8, 2024
@agilbert1412 agilbert1412 added the is: bug/fix Issues that are reporting bugs or pull requests that are fixing bugs. label May 8, 2024
@github-actions github-actions bot added the waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. label May 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects: core Issues/PRs that touch core and may need additional validation. is: bug/fix Issues that are reporting bugs or pull requests that are fixing bugs. waiting-on: peer-review Issue/PR has not been reviewed by enough people yet.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant