josefarias / hotwire_combobox

An accessible autocomplete for Ruby on Rails.
https://hotwirecombobox.com
MIT License
436 stars 26 forks source link

Incorrectly manipulating form values #172

Closed excid3 closed 2 months ago

excid3 commented 2 months ago

We discovered a bug in using Hotwire Combobox in production this morning where a value with repeated characters gets modified in the customization functionality.

An example of this is a cron expression of "* * * * *". When Hotwire Combobox applies customizations, it applies them to the hidden input value which results in calling token_list and setting the value to "*" instead of the original value.

I tracked this down to: https://github.com/josefarias/hotwire_combobox/blob/eae73f0c8b0bb453b3c0f3dfae4084b6f7c7f01f/app/presenters/hotwire_combobox/component/customizable.rb#L50

irb(#<HotwireCombobox::Component:...):001> key
=> :value
irb(#<HotwireCombobox::Component:...):002> value
=> "* * * * *"
irb(#<HotwireCombobox::Component:...):003> view.token_list(value, custom.delete(key))
=> "*"

I'm not familiar enough with how these customizations work so I haven't made a PR to fix this, but it seems like the customizations should not be applied to :value?

josefarias commented 2 months ago

Thanks @excid3 this is a serious bug. I'll get it fixed.

Could you share the code where you're customizing or otherwise initializing the combobox please?

value is a protected attr (see top of that file) so it shouldn't be customized. I'm guessing we're either processing it without cause, or we're misinterpreting the customization for a different part of the combobox.

excid3 commented 2 months ago

We're not customizing anything.

    <%#= form.combobox :run_at, [form.object.run_at, "@hourly", "@daily", "@weekly", "@monthly", "@every 4h", "0 12 * * *"].compact.uniq, free_text: true, required: true %>

It doesn't seem to be filtering out those protected attributes in the apply customizations.

josefarias commented 2 months ago

Got it. Taking a look now, thanks!

josefarias commented 2 months ago

@excid3 I just released v0.3.1 with a fix. Could I get you to verify it's working in that version, please?

Thanks a bunch. Sorry we missed this.

excid3 commented 2 months ago

Confirmed that fixes it. Thanks for the quick fix! 👍