stimulusreflex / stimulus_reflex

Build reactive applications with the Rails tooling you already know and love.
https://docs.stimulusreflex.com
MIT License
2.28k stars 172 forks source link

Form serialisation does not escape input values #430

Closed ukdave closed 3 years ago

ukdave commented 3 years ago

Bug Report

Describe the bug

Form serialisation does not escape input field values.

Form input fields containing & and % characters can cause values to be truncated when accessing the params in the reflex, and can also trigger Rack::QueryParser::InvalidParameterError - invalid %-encoding errors.

To Reproduce

Create a simple form that submits to a reflex and enter a & or % character in one of the input fields, e.g:

<%= form_with(model: @comment, data: {reflex: "submit->CommentsReflex#add_comment"}) do |form| %>
  <div class="field">
    <%= form.label :name %>
    <%= form.text_field :name %>
  </div>

  <div class="field">
    <%= form.label :message %>
    <%= form.text_field :message %>
  </div>

  <div class="actions">
    <%= form.submit %>
  </div>
<% end %>
class CommentsReflex < ApplicationReflex
  def add_comment
    @comment = Comment.create(comment_params)
  end

  private

  def comment_params
    params.require(:comment).permit(:name, :message)
  end
end

Expected behavior

Form values should be escaped and parameter values received correctly in the reflex.

Screenshots or reproduction

Using the example form above and filling in the form fields like so:

I can see in the WebSocket messages in Firefox Dev Tools that the formData attribute is being sent like this:

formData: authenticity_token=xxx&comment[name]=David&comment[message]=foo & bar&commit=Create Comment

I believe it should be escaped and sent like this:

formData: authenticity_token=xxx&comment[name]=David&comment[message]=foo %26 &commit=Create Comment

In fact entering foo %26 bar into the message field results in the parameter being received in the reflex as foo & bar.

I have created a simple Rails app demonstrating the bug here: https://github.com/ukdave/stimulus_reflex_form_serialization/

From a poke around the stimulus_reflex source code I believe the issue is in the serialiseForm method of javascript/utils.js where it joins the form field names and values together with an = and then joins those together with an & without any escaping.

Versions

StimulusReflex

External tools

Browser

marcoroth commented 3 years ago

Hey @ukdave. Thanks for reporting this issue with providing a working app to reproduce the issue! ❤️

After looking into this, I think this issue might have been solved with #418.

Can you try to point your StimulusReflex gem and npm package to master and to see if the issue persists? Thank you!

https://docs.stimulusreflex.com/hello-world/setup#running-edge

ukdave commented 3 years ago

Thanks @marcoroth. I can confirm that the issue is fixed when I switch the gem and npm package to master 🙂

Apologies, I did check the closed issues but didn't see #418.

leastbad commented 3 years ago

@ukdave I very much wish that all folks reporting issues would provide as much information and explanation as you do. Kudos!