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

Encoding issue in jruby 9.0.0.0 #3267

Closed
deivid-rodriguez opened this issue Aug 19, 2015 · 11 comments
Closed

Encoding issue in jruby 9.0.0.0 #3267

deivid-rodriguez opened this issue Aug 19, 2015 · 11 comments
Labels
Milestone

Comments

@deivid-rodriguez
Copy link
Contributor

@deivid-rodriguez deivid-rodriguez commented Aug 19, 2015

Hi!

My tests pass under MRI 2.2.3 but fail under jruby 9.0.0.0. This is the error I'm getting:

  1) Error:
AdminScenarioTest#test_scenarios_are_listed_in_the_index_page:
RegexpError: invalid multibyte character: /Període\ de\ retorn/
    test/support/admin.rb:34:in `assert_table_header'
    test/support/admin.rb:25:in `assert_table_col'
    test/admin/scenario_test.rb:22:in `test_scenarios_are_listed_in_the_index_page'

Adding magic utf-8 comments where the utf-8 string is defined does not help.

I'll try to post a proper reproduction if necessary, I just wanted to get feedback about it first in case this is a known issue.

Thanks!

@enebo
Copy link
Member

@enebo enebo commented Aug 19, 2015

@deivid-rodriguez Could you try adding env var: JRUBY_OPTS='-J-Dfile.encoding=UTF-8' and see if this goes away? I am trying to fix this without requiring that but I am curious if this fixes the problem for you.

@deivid-rodriguez
Copy link
Contributor Author

@deivid-rodriguez deivid-rodriguez commented Aug 19, 2015

I forgot to mention that I tried that and didn't work either.

Thanks!

@enebo
Copy link
Member

@enebo enebo commented Aug 19, 2015

@deivid-rodriguez ok. Yeah something which reduces this to a script we can run and see the issue will allow us to fix it the quickest.

@enebo enebo added the JRuby 9000 label Aug 19, 2015
@deivid-rodriguez
Copy link
Contributor Author

@deivid-rodriguez deivid-rodriguez commented Aug 19, 2015

Here you go!

bundle
RAILS_ENV=test bundle exec rake db:migrate
bundle exec rake

should do it.

@enebo
Copy link
Member

@enebo enebo commented Aug 19, 2015

This will work but I was hoping to have something which ran as a single file and did not require loading a big env.

@deivid-rodriguez
Copy link
Contributor Author

@deivid-rodriguez deivid-rodriguez commented Aug 19, 2015

This is a default Rails app plus capybara plus an activeadmin resource. "Big" is a relative word... :)

I can further isolate the problem, but it's unlikely that I get the time to do it before October.

Thanks for your work!

@enebo
Copy link
Member

@enebo enebo commented Aug 19, 2015

@deivid-rodriguez ok I will fix the other known encoding issues and circle back to this. Having a reproduction is great in that we can at least know if it has been addressed. Thanks for that. I just get greedy and want it localized more :)

@deivid-rodriguez
Copy link
Contributor Author

@deivid-rodriguez deivid-rodriguez commented Aug 19, 2015

@enebo That sounds great, I will circle back here as well if I get the time to further localize the issue for you!

@enebo enebo added this to the JRuby 9.0.2.0 milestone Aug 26, 2015
@headius
Copy link
Member

@headius headius commented Oct 7, 2015

I believe I've narrowed this down to:

"".force_encoding('US-ASCII').match("Període\\ de\\ retorn".force_encoding('UTF-8'))

Investigating.

@headius
Copy link
Member

@headius headius commented Oct 7, 2015

I have a fix. We do not follow the correct path for constructing the regexp from the match string, and so the encoding does not get negotiated correctly.

@headius headius closed this in 0b87f44 Oct 7, 2015
@deivid-rodriguez
Copy link
Contributor Author

@deivid-rodriguez deivid-rodriguez commented Oct 7, 2015

Thanks!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.