join May Cause Unwanted HTML Escapes

[lcreid]

Array#join always produces a String, so some uses of join in our code may be causing HTML-safe strings (e.g. error messages) to become "unsafe" and then they'll get escaped.

One place to investigate:

bootstrap_form/lib/bootstrap_form/components/validation.rb

Line 74 in 57a5be7

messages.join(", ")

May be related to #653?

[donv]

@lcreid Any more concrete example for this?

[lcreid]

If I search for join in the code excluding test/, and eliminating the instances that are joining file paths, I find these:

lib/bootstrap_form/form_builder.rb:
  69          ([*options[:html][:class]&.split(/\s+/)] + %w[row row-cols-auto g-3 align-items-center])
  70:         .compact.uniq.join(" ")
  71      end

lib/bootstrap_form/form_group_builder.rb:
  93        control_classes = css_options.delete(:control_class) { control_class }
  94:       css_options[:class] = [control_classes, css_options[:class]].compact.join(" ")
  95        css_options[:class] << " is-invalid" if error?(method)

lib/bootstrap_form/components/validation.rb:
  83  
  84:         object.errors[name].join(", ")
  85        end

lib/bootstrap_form/helpers/bootstrap.rb:
  34            if hide_attribute_name
  35:             object.errors[name].join(", ")
  36            else
  37:             object.errors.full_messages_for(name).join(", ")
  38            end

  95          end
  96:         ActiveSupport::SafeBuffer.new(tags.join)
  97        end

lib/bootstrap_form/inputs/rich_text_area.rb:
  12              prepend_and_append_input(name, options) do
  13:               options[:class] = ["trix-content", options[:class]].compact.join(" ")
  14                rich_text_area_without_bootstrap(name, options)

The code has been like this for a long time and we have no complaints. And if it's "broken", I think it's broken in the safe direction, meaning it will err on the side of escaping HTML. It's just something that I've always wanted to look at. Maybe when I retire, if I can ever afford to. Ha ha.

[donv]

I propose we either define what needs to change or close this issue and #653 .

[lcreid]

I took a run at this last week when my COVID wasn't so bad and ran into one case where the right solution isn't obvious. Still working on it.

[lcreid]

Fixed by #704