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

Force request encodings to be UTF-8 instead of ASCII-8BIT after a reflex #320

Merged
merged 2 commits into from
Oct 31, 2020

Conversation

marcoroth
Copy link
Member

Type of PR (feature, enhancement, bug fix, etc.)

Bug Fix

Description

This was a really hard one! Spent the last few hours debugging where that ASCII-8BIT came from.

Calling Rack::MockRequest.env_for() in lib/stimulus_reflex/reflex.rb returns for some reason a Hash with some ASCII-8BIT strings. Specifically those are the strings which are encoded as ASCII-8BIT:

mock_env

=> { 
  [...] 
  "REQUEST_METHOD"=>"GET", 
  "SERVER_NAME"=>"localhost", 
  "SERVER_PORT"=>"3000", 
  "QUERY_STRING"=>"", 
  "PATH_INFO"=>"/", 
  "HTTPS"=>"off", 
  [...]
}

This happens because Rack::MockRequest.env_for() encodes it's input as Encoding::BINARY which results in ASCII-8BIT. See below for reference in rack/rack:

https://github.com/rack/rack/blob/4db9d299e43dbf856664359b16e2e461c378ee1c/lib/rack/mock.rb#L155-L158

With this hash we are building a new ActionDispatch::Request which we then pass into Rails.application.routes.recognize_path_with_request().

This then returns the following Hash which also contains ASCII-8BIT strings as a result:

@url_params

=> {
  "action": "index",
  "controller": "todos"
}

Not quite sure if this also impacts this issue, but they are again converted to ASCII-8BIT in RouteSet#recognize_path_with_request. See rails/rails for reference:

https://github.com/rails/rails/blob/60ef07093d9023568ccbbd34e862b40395ec02f4/actionpack/lib/action_dispatch/routing/route_set.rb#L866-L868.

The strings in this @url_params hash get passed around the whole Rails stack and finally end up as controller.action_name and action_name in the views. This is why @asgerb encountered the Can not transliterate strings with ASCII-8BIT encoding error when calling action_name.parameterize.

This commit converts the values of :action and :controller in @url_params back to UTF-8 strings as they should be (this is the case if you render the view normally).

I didn't find another way in the Rack::MockRequest.env_for() or Rails.application.routes.recognize_path_with_request() call to enforce those strings to be UTF-8 even earlier.

This PR also untangles the StimulusReflex::Reflex#request method a little bit, which should make it easier to follow and easier to understand. There is a whole lot going on the env merging.

Finally Resolves #202.

/cc @asgerb could you check if this resolves your issue? Thank you!

Why should this be added

Fixes the edge case in #202 where the action_name was converted to a ASCII-8BIT string.

Checklist

  • My code follows the style guidelines of this project
  • Checks (StandardRB & Prettier-Standard) are passing

Calling `Rack::MockRequest.env_for()` returns for some reason a Hash with some ASCII-8BIT strings. This commit converts the `params[:action]` and `params[:controller]` back to UTF-8 strings since those are the only params converted to ASCII-8BIT
@ghost
Copy link

ghost commented Oct 5, 2020

Nice work, @marcoroth! I had some issues with this as well today!
I'm glad you got it sorted.

@marcoroth
Copy link
Member Author

@darkrubyist let me know if this resolves the issue for you as well 😊


I've also forked the TodoMVC repo and added the code to reproduce this issue: https://github.com/marcoroth/stimulus_reflex-issue-202

@ghost
Copy link

ghost commented Oct 5, 2020

For me it was triggering on Posts#create, because I had some validations on content and title.
I removed the validations and it worked enough to continue my work.

I will be able to test your work tomorrow when I will wake up!
Thanks a lot! I will let you know if the error will go away!

@ghost
Copy link

ghost commented Oct 6, 2020

Unfortunately, I'm still getting this. :(

16:40:14 web.1     | 2020-10-06 16:40:14 +0300: Rack app error handling request { POST /posts }
16:40:14 web.1     | #<Encoding::CompatibilityError: incompatible encoding regexp match (UTF-8 regexp with ASCII-8BIT string)>

@marcoroth
Copy link
Member Author

marcoroth commented Oct 6, 2020

@darkrubyist can you see in the logs which method call caused this error or do you know with which string you are operating?

And: are you sure this is even caused while rendering a reflex action? Just judging from the POST /posts in your logs I have the feeling there is no reflex action involved in your error.

@ghost
Copy link

ghost commented Oct 7, 2020

You are correct. The error is not coming from any reflex action.
Unfortunately, I got no other logs than what I posted.

To confirm again, it's not coming from any reflex action.

@marcoroth marcoroth marked this pull request as ready for review October 28, 2020 15:34
@leastbad leastbad merged commit f415641 into stimulusreflex:master Oct 31, 2020
@marcoroth marcoroth deleted the fix-encodings branch November 1, 2020 02:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Encoding changes from UTF-8 to ASCII-8BIT
2 participants