Skip to content
This repository has been archived by the owner on May 12, 2018. It is now read-only.

Fix branch name encoding issue #25

Merged
merged 4 commits into from
Feb 18, 2014
Merged

Fix branch name encoding issue #25

merged 4 commits into from
Feb 18, 2014

Conversation

layzerar
Copy link
Contributor

When the branch name contains non-ascii characters, it will raise a encoding error when viewing any commit of this branch.

Here is the detail of traceback.

Encoding::CompatibilityError (incompatible character encodings: UTF-8 and ASCII-8BIT):
  app/views/projects/commit/_commit_box.html.haml:47:in `_app_views_projects_commit__commit_box_html_haml__3615852107586763187_70120827707580'
  app/views/projects/commit/show.html.haml:1:in `_app_views_projects_commit_show_html_haml___1131168359666268760_70120827597420'
  app/controllers/application_controller.rb:57:in `set_current_user_for_thread'

  Rendered errors/encoding.html.haml within layouts/errors (0.1ms)
  Rendered layouts/_head.html.haml (1.4ms)
  Rendered layouts/_search.html.haml (1.4ms)
  Rendered layouts/_head_panel.html.haml (10.0ms)
  Rendered layouts/_flash.html.haml (0.2ms)
Completed 500 Internal Server Error in 526ms (Views: 12.4ms | ActiveRecord: 14.5ms)

@layzerar
Copy link
Contributor Author

@layzerar
Copy link
Contributor Author

It's better to treat the branch name as utf-8 than ascii.

Hm, how does this build failed?

@layzerar
Copy link
Contributor Author

It's failed in ruby 1.9.3 but succeed in ruby 2.0. Does it concerned with this pull request?

@dosire
Copy link
Member

dosire commented Feb 17, 2014

@randx Lots of people are encountering problems with non-ascii branch names that this should fix. Can you review it?

@@ -295,6 +295,7 @@ def find_commits(options = {})
#
def branch_names_contains(commit)
output = grit.git.native(:branch, {contains: true}, commit)
output.force_encoding("UTF-8")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can use existing encoding helper output = EncodingHelper.encode!(output)?

@layzerar
Copy link
Contributor Author

Sorry for commit three times. I'm really a novice to ruby. I'm test it with gitlab 6.5.1 in Ubuntu12.04. It works fine.

@dosire
Copy link
Member

dosire commented Feb 17, 2014

@layzerar Can you use the code @randx suggested? output = EncodingHelper.encode!(output)

@layzerar
Copy link
Contributor Author

That's a really good solution.

@dosire
Copy link
Member

dosire commented Feb 17, 2014

@layzerar Can you adapt your PR?

@layzerar
Copy link
Contributor Author

@dosire Yes. But I don't have test environment now. I'm afraid I couldn't change it until tomorrow.

@dosire
Copy link
Member

dosire commented Feb 17, 2014

@layzerar No problem, tomorrow is fine.

@layzerar
Copy link
Contributor Author

I adapted the pull request just now. I tested it in my machine, it works fine.

@dosire
Copy link
Member

dosire commented Feb 18, 2014

@randx Looks good to me.

dzaporozhets added a commit that referenced this pull request Feb 18, 2014
@dzaporozhets dzaporozhets merged commit 3855b04 into gitlabhq:master Feb 18, 2014
@dosire
Copy link
Member

dosire commented Feb 18, 2014

Thanks @randx and @layzerar

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants