Skip to content

Conversation

@kbrock
Copy link
Member

@kbrock kbrock commented Nov 8, 2024

Put limits on models so API will not allow very large input

These limits were taken from the React forms found in the react schema.js files.

ag -B3 maxLength app/javascript/components/

@kbrock kbrock requested review from Fryguy and agrare as code owners November 8, 2024 20:41
@kbrock kbrock force-pushed the model_field_length branch 2 times, most recently from d59848f to 0c7e540 Compare November 11, 2024 15:32
@kbrock
Copy link
Member Author

kbrock commented Nov 11, 2024

update:

  • fix syntax error

update:

  • no longer require vm#description
  • rubocop spaces

@kbrock kbrock force-pushed the model_field_length branch from 0c7e540 to d879123 Compare November 11, 2024 16:10
@kbrock
Copy link
Member Author

kbrock commented Nov 11, 2024

update:

  • authentication no longer requires name

@kbrock kbrock force-pushed the model_field_length branch 2 times, most recently from fde4e05 to 6a295e4 Compare November 19, 2024 14:44
@kbrock kbrock added the security fix Security fix generated by WhiteSource label Nov 19, 2024
@kbrock
Copy link
Member Author

kbrock commented Nov 20, 2024

update:

  • Rebase
  • Remove Authentication#name presence check
  • Zone#name, description length from 50 to 128

update:

  • Fix error MiqAlert#name (mistyped to notes)

@kbrock kbrock force-pushed the model_field_length branch from 6a295e4 to e4dca87 Compare November 20, 2024 16:34
@kbrock
Copy link
Member Author

kbrock commented Nov 20, 2024

update:

  • fix merge conflict with hash serialization in Zone.

@Fryguy Fryguy self-assigned this Jan 15, 2025
@Fryguy Fryguy added the bug label Jan 15, 2025
@kbrock kbrock force-pushed the model_field_length branch from 1952d23 to fe8046a Compare March 10, 2025 16:27
@kbrock
Copy link
Member Author

kbrock commented Mar 10, 2025

update:

  • rebased
  • removed vm limits
    • were via app/javascript/components/vm-common-rename-form/vm-common-rename-form.schema.js

has_many :flavors, :through => :cloud_tenant_flavors
has_many :cloud_volume_types, :through => :ext_management_system

validates :name, :length => {:maximum => 128}
Copy link
Member

@Fryguy Fryguy Mar 10, 2025

Choose a reason for hiding this comment

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

I think I mentioned this elsewhere, but any data that can come from providers I don't think we should be as conservative with, otherwise we run the risk of failing during provider collection. On the backend, I think we can be much looser with these values. For example, here I'd be ok with 1k or even higher. The frontend is only for new objects, and those forms (I think) are provider specific, so the value could be different on different forms depending on the provider. I understand the need to limit it to some value, but I'd prefer if it were a really large, but reasonable value.

@agrare Thoughts?


virtual_total :total_based_volumes, :based_volumes

validates :description, :length => {:maximum => 50}
Copy link
Member

Choose a reason for hiding this comment

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

This is a great example, where the description (if it's not a name or identifier), could actually be really large coming from a provider.


validates :name, :presence => true
validates :name, :presence => true, :length => {:maximum => 256}
validates :description, :length => {:maximum => 1024}
Copy link
Member

Choose a reason for hiding this comment

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

1kb for a service description feels really small. I'm surprised the UI only allows that much.

@miq-bot
Copy link
Member

miq-bot commented Mar 10, 2025

Checked commits kbrock/manageiq@a4abf7a~...fe8046a with ruby 3.1.5, rubocop 1.56.3, haml-lint 0.59.0, and yamllint
9 files checked, 0 offenses detected
Everything looks fine. 🍪

@miq-bot miq-bot added the stale label Jun 16, 2025
@miq-bot
Copy link
Member

miq-bot commented Jun 16, 2025

This pull request has been automatically marked as stale because it has not been updated for at least 3 months.

If these changes are still valid, please remove the stale label, make any changes requested by reviewers (if any), and ensure that this issue is being looked at by the assigned/reviewer(s).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug security fix Security fix generated by WhiteSource stale

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants