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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Document and update API for `skip_parameter_encoding` #27427

Merged
merged 2 commits into from Dec 21, 2016

Conversation

Projects
None yet
4 participants
@tenderlove
Member

tenderlove commented Dec 21, 2016

This commit changes parameter_encoding to skip_parameter_encoding.
skip_parameter_encoding will set encoding on all parameters to
ASCII-8BIT for a given action on a particular controller. This allows
the controller to handle data when the encoding of that data is unknown,
for example file systems or truly binary parameters.

This should work for our case on GitHub. I think with the change to Utils, we could even go so far as to remove the class level config method and just call out to the controller to do parameter encoding. Either way will work for me! 馃槃

/cc @rafaelfranca @dhh @fxn @matthewd

@dhh

This comment has been minimized.

Show comment
Hide comment
@dhh

dhh Dec 21, 2016

Member

I like the concept of being able to turn off the default encoding. If you guys hit this, surely someone else did too. 馃憤

Member

dhh commented Dec 21, 2016

I like the concept of being able to turn off the default encoding. If you guys hit this, surely someone else did too. 馃憤

tenderlove added some commits Dec 21, 2016

Document and update API for `skip_parameter_encoding`
This commit changes `parameter_encoding` to `skip_parameter_encoding`.
`skip_parameter_encoding` will set encoding on all parameters to
ASCII-8BIT for a given action on a particular controller.  This allows
the controller to handle data when the encoding of that data is unknown,
for example file systems or truly binary parameters.

@tenderlove tenderlove merged commit 7336d0a into master Dec 21, 2016

1 of 3 checks passed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
continuous-integration/travis-ci/push The Travis CI build failed
Details
codeclimate no new or fixed issues
Details

@tenderlove tenderlove deleted the binary-params branch Dec 21, 2016

@bf4 bf4 referenced this pull request Dec 27, 2016

Closed

configurable forced encoding #37

@arthurnn

This comment has been minimized.

Show comment
Hide comment
@arthurnn

arthurnn Jul 12, 2017

Member

So this allows us to set an action that should force the encoding to be binary.
However, I wonder why Rack sets all the parameters as UTF8 in the first place?
Should?:

  • Rack sets all parameters to ASCII-8BIT (binary)
  • Rails force everything to UTF8, as that's a sane default for most applications.
  • Rails would allow an action to skip the UTF8 set
Member

arthurnn commented Jul 12, 2017

So this allows us to set an action that should force the encoding to be binary.
However, I wonder why Rack sets all the parameters as UTF8 in the first place?
Should?:

  • Rack sets all parameters to ASCII-8BIT (binary)
  • Rails force everything to UTF8, as that's a sane default for most applications.
  • Rails would allow an action to skip the UTF8 set
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment