Project

General

Profile

Actions

Defect #40948

closed

ActiveRecord::ValueTooLong error with git author longer than 255 characters

Added by Holger Just 6 months ago. Updated 5 months ago.

Status:
Closed
Priority:
Normal
Category:
SCM
Target version:
Start date:
Due date:
% Done:

0%

Estimated time:
Resolution:
Fixed
Affected version:

Description

If a Git repository contains a commit where the author is longer than 255 characters, Redmine throws an exception when loading the commits:

ActiveRecord::ValueTooLong: Data too long for column 'committer'

The attached patch against the current trunk fixes this by truncating the author to 255 characters. Note that this is a git patch which contains an update of the test/fixtures/repositories/git_repository.tar.gz file. This is an update of the existing repository with just a new commit with a long author added:

Only in git_repository/objects: 14
Only in git_repository/objects: 17
Only in git_repository/objects: 80
diff -ru git_repository-1/packed-refs git_repository/packed-refs
--- a/git_repository/packed-refs    2019-04-05 05:04:24.000000000 +0200
+++ b/git_repository/packed-refs    2024-07-08 19:17:46.019881974 +0200
@@ -1,10 +1,10 @@
-# pack-refs with: peeled fully-peeled
+# pack-refs with: peeled fully-peeled sorted
 2a682156a3b6e77a8bf9cd4590e8db757f3c6c78 refs/heads/issue-8857
 4fc55c43bf3d3dc2efb66145365ddc17639ce81e refs/heads/latin-1-branch-�-01
 1ca7f5ed374f3cb31a93ae5215c2e25cc6ec5127 refs/heads/latin-1-branch-�-02
 1ca7f5ed374f3cb31a93ae5215c2e25cc6ec5127 refs/heads/latin-1-path-encoding
 83ca5fd546063a3c7dc2e568ba3355661a9e2b2c refs/heads/master
-83ca5fd546063a3c7dc2e568ba3355661a9e2b2c refs/heads/master-20120212
+1722811bef943a957eaf9339648df234590281d1 refs/heads/master-20120212
 67e7792ce20ccae2e4bb73eed09bb397819c8834 refs/heads/test-latin-1
 fba357b886984ee71185ad2065e65fc0417d9b92 refs/heads/test_branch
 7234cb2750b63f47bff735edc50a1c0a433c2518 refs/tags/tag00.lightweight

The rest of the patch is as follows:

diff --git a/app/models/repository/git.rb b/app/models/repository/git.rb
index 9637702abd..b6b3c8336c 100644
--- a/app/models/repository/git.rb
+++ b/app/models/repository/git.rb
@@ -219,7 +219,7 @@ def save_revision(rev)
         :repository   => self,
         :revision     => rev.identifier,
         :scmid        => rev.scmid,
-        :committer    => rev.author,
+        :committer    => rev.author.truncate(255),
         :committed_on => rev.time,
         :comments     => rev.message,
         :parents      => parents
diff --git a/test/functional/repositories_git_controller_test.rb b/test/functional/repositories_git_controller_test.rb
index fee41faac4..70d7950686 100644
--- a/test/functional/repositories_git_controller_test.rb
+++ b/test/functional/repositories_git_controller_test.rb
@@ -28,7 +28,7 @@ class RepositoriesGitControllerTest < Redmine::RepositoryControllerTest
   REPOSITORY_PATH = Rails.root.join('tmp/test/git_repository').to_s
   REPOSITORY_PATH.tr!('/', "\\") if Redmine::Platform.mswin?
   PRJ_ID     = 3
-  NUM_REV = 28
+  NUM_REV = 29

   def setup
     super
diff --git a/test/integration/repositories_git_test.rb b/test/integration/repositories_git_test.rb
index 67c61575ca..8a33c356e9 100644
--- a/test/integration/repositories_git_test.rb
+++ b/test/integration/repositories_git_test.rb
@@ -26,7 +26,7 @@ class RepositoriesGitTest < Redmine::IntegrationTest
   REPOSITORY_PATH = Rails.root.join('tmp/test/git_repository').to_s
   REPOSITORY_PATH.tr!('/', "\\") if Redmine::Platform.mswin?
   PRJ_ID     = 3
-  NUM_REV = 28
+  NUM_REV = 29

   def setup
     User.current = nil

Files

Actions #1

Updated by Marius BĂLTEANU 6 months ago

  • Status changed from New to Resolved
  • Assignee set to Marius BĂLTEANU
  • Target version set to 6.0.0
  • Resolution set to Fixed

Committed, thanks!

Actions #2

Updated by Go MAEDA 5 months ago

Due to the change in the test repository in r22910, the following tests have started to fail:

  • test/unit/lib/redmine/scm/adapters/git_adapter_test.rb
  • test/unit/repository_git_test.rb
Actions #3

Updated by Marius BĂLTEANU 5 months ago

Thanks Go MAEDA for reporting this.

I've fixed a part of the tests, now I'm looking on the remaining ones.

Actions #4

Updated by Marius BĂLTEANU 5 months ago

I think it is a problem with the archive because with the previous version the output for me is

root@ebb9961479d7:/work# bundle exec rake test:scm:setup:git 
./git_repository/
./git_repository/info/
./git_repository/info/exclude
./git_repository/info/refs
./git_repository/packed-refs
./git_repository/description
./git_repository/branches/
./git_repository/HEAD
./git_repository/hooks/
./git_repository/hooks/pre-commit
./git_repository/hooks/update
./git_repository/hooks/applypatch-msg
./git_repository/hooks/post-receive
./git_repository/hooks/commit-msg
./git_repository/hooks/post-update
./git_repository/hooks/pre-applypatch
./git_repository/hooks/pre-rebase
./git_repository/hooks/post-commit
./git_repository/gitk.cache
./git_repository/objects/
./git_repository/objects/info/
./git_repository/objects/info/packs
./git_repository/objects/pack/
./git_repository/objects/pack/pack-3b2cc08d52ff500c3a25fa92c41ec847eb1607a1.pack
./git_repository/objects/pack/pack-3b2cc08d52ff500c3a25fa92c41ec847eb1607a1.idx
./git_repository/refs/
./git_repository/refs/tags/
./git_repository/refs/heads/
./git_repository/config

and now I have:

root@ebb9961479d7:/work# bundle exec rake test:scm:setup:git 
._git_repository
tar: Ignoring unknown extended header keyword 'LIBARCHIVE.xattr.com.apple.quarantine'
git_repository/
git_repository/._config
tar: Ignoring unknown extended header keyword 'LIBARCHIVE.xattr.com.apple.quarantine'
git_repository/config
git_repository/._objects
tar: Ignoring unknown extended header keyword 'LIBARCHIVE.xattr.com.apple.quarantine'
git_repository/objects/
git_repository/._HEAD
tar: Ignoring unknown extended header keyword 'LIBARCHIVE.xattr.com.apple.quarantine'
git_repository/HEAD
git_repository/._info
tar: Ignoring unknown extended header keyword 'LIBARCHIVE.xattr.com.apple.quarantine'
git_repository/info/
git_repository/._description
tar: Ignoring unknown extended header keyword 'LIBARCHIVE.xattr.com.apple.quarantine'
git_repository/description
git_repository/._hooks
tar: Ignoring unknown extended header keyword 'LIBARCHIVE.xattr.com.apple.quarantine'
git_repository/hooks/
git_repository/._refs
tar: Ignoring unknown extended header keyword 'LIBARCHIVE.xattr.com.apple.quarantine'
git_repository/refs/
git_repository/._branches
tar: Ignoring unknown extended header keyword 'LIBARCHIVE.xattr.com.apple.quarantine'
git_repository/branches/
git_repository/packed-refs
git_repository/._gitk.cache
tar: Ignoring unknown extended header keyword 'LIBARCHIVE.xattr.com.apple.quarantine'
git_repository/gitk.cache
git_repository/refs/._heads
tar: Ignoring unknown extended header keyword 'LIBARCHIVE.xattr.com.apple.quarantine'
git_repository/refs/heads/
git_repository/refs/._tags
tar: Ignoring unknown extended header keyword 'LIBARCHIVE.xattr.com.apple.quarantine'
git_repository/refs/tags/
git_repository/hooks/._pre-rebase
tar: Ignoring unknown extended header keyword 'LIBARCHIVE.xattr.com.apple.quarantine'
git_repository/hooks/pre-rebase
git_repository/hooks/._pre-applypatch
tar: Ignoring unknown extended header keyword 'LIBARCHIVE.xattr.com.apple.quarantine'
git_repository/hooks/pre-applypatch
git_repository/hooks/._update
tar: Ignoring unknown extended header keyword 'LIBARCHIVE.xattr.com.apple.quarantine'
git_repository/hooks/update
git_repository/hooks/._post-receive
tar: Ignoring unknown extended header keyword 'LIBARCHIVE.xattr.com.apple.quarantine'
git_repository/hooks/post-receive
git_repository/hooks/._post-commit
tar: Ignoring unknown extended header keyword 'LIBARCHIVE.xattr.com.apple.quarantine'
git_repository/hooks/post-commit
git_repository/hooks/._applypatch-msg
tar: Ignoring unknown extended header keyword 'LIBARCHIVE.xattr.com.apple.quarantine'
git_repository/hooks/applypatch-msg
git_repository/hooks/._post-update
tar: Ignoring unknown extended header keyword 'LIBARCHIVE.xattr.com.apple.quarantine'
git_repository/hooks/post-update
git_repository/hooks/._commit-msg
tar: Ignoring unknown extended header keyword 'LIBARCHIVE.xattr.com.apple.quarantine'
git_repository/hooks/commit-msg
git_repository/hooks/._pre-commit
tar: Ignoring unknown extended header keyword 'LIBARCHIVE.xattr.com.apple.quarantine'
git_repository/hooks/pre-commit
git_repository/info/._exclude
tar: Ignoring unknown extended header keyword 'LIBARCHIVE.xattr.com.apple.quarantine'
git_repository/info/exclude
git_repository/info/._refs
tar: Ignoring unknown extended header keyword 'LIBARCHIVE.xattr.com.apple.quarantine'
git_repository/info/refs
git_repository/objects/._pack
tar: Ignoring unknown extended header keyword 'LIBARCHIVE.xattr.com.apple.quarantine'
git_repository/objects/pack/
git_repository/objects/80/
git_repository/objects/17/
git_repository/objects/._info
tar: Ignoring unknown extended header keyword 'LIBARCHIVE.xattr.com.apple.quarantine'
git_repository/objects/info/
git_repository/objects/14/
git_repository/objects/14/c347274972c9cf404af0f596c97afc89883409
git_repository/objects/info/._packs
tar: Ignoring unknown extended header keyword 'LIBARCHIVE.xattr.com.apple.quarantine'
git_repository/objects/info/packs
git_repository/objects/17/22811bef943a957eaf9339648df234590281d1
git_repository/objects/80/e934fc3fe47de92c42d55ea5bb386d62a70fed
git_repository/objects/pack/._pack-3b2cc08d52ff500c3a25fa92c41ec847eb1607a1.pack
tar: Ignoring unknown extended header keyword 'LIBARCHIVE.xattr.com.apple.quarantine'
git_repository/objects/pack/pack-3b2cc08d52ff500c3a25fa92c41ec847eb1607a1.pack
git_repository/objects/pack/._pack-3b2cc08d52ff500c3a25fa92c41ec847eb1607a1.idx
tar: Ignoring unknown extended header keyword 'LIBARCHIVE.xattr.com.apple.quarantine'
git_repository/objects/pack/pack-3b2cc08d52ff500c3a25fa92c41ec847eb1607a1.idx

Also, when I load the repository in the current trunk and I click Load all revisions, I receive:

Completed 500 Internal Server Error in 84ms (ActiveRecord: 4.8ms | Allocations: 18946)

ActionView::Template::Error (source sequence is illegal/malformed utf-8):
    2: function revisionGraphHandler(){
    3:   drawRevisionGraph(
    4:     document.getElementById('holder'),
    5:     <%= commits.to_json.html_safe %>,
    6:     <%= space %>
    7:   );
    8: }

app/views/repositories/_revision_graph.html.erb:5
app/views/repositories/_revision_graph.html.erb:1
app/views/repositories/_revisions.html.erb:23
app/views/repositories/_revisions.html.erb:1
app/views/repositories/revisions.html.erb:14
lib/redmine/sudo_mode.rb:78:in `sudo_mode'

so we should fix the archive first.

Actions #5

Updated by Marius BĂLTEANU 5 months ago

  • Status changed from Resolved to Reopened
Actions #6

Updated by Marius BĂLTEANU 5 months ago

I'm not reverting this because I think we can fix the issues.

Actions #7

Updated by Holger Just 5 months ago

  • Assignee changed from Marius BĂLTEANU to Holger Just

Sorry for that, I'm having a look and will provide a new patch later today.

Actions #8

Updated by Holger Just 5 months ago

It seems my tar command to pack the updated repository has some added Mac specific file attributes. Oh well, let's try again...

Attached, you will a new archive (0002-git_repository.tar.gz) which I have created on my Mac with the following command:

tar --{uname,gname}=root --{uid,gid}=0 --no-fflags --no-acls --no-xattrs --numeric-owner -czf test/fixtures/repositories/git_repository.tar.gz test/fixtures/repositories/git_repository

Hopefully, this should get rid of any ACLs or extended attributes. Could you try this archive? Here, I have added the following changes (compared to the original repository):

  • I added commit b1650eac7c505a6dab9f19858afc9ecb481eccc2 to the branches master and master-20120212. In my first attempt, I only added the new commit to master-20120212.
    • It performs an (inconsequential) single character change in new_file.txt.
    • It has a very long author name.
    • It has a date of 2010-10-01 06:00:00. This is new and is required to please GitRepositoryTest#test_next which depends on commit dates.

The commit command I have used was:

GIT_COMMITTER_DATE="2010-10-01 06:00:00" GIT_AUTHOR_DATE="2010-10-01 06:00:00" git commit --author "This is a very long and also quite pointless name of a committer which is just here so that it is very long and very pointless and also quite redundant as we have already stated that this name is very long and pointless and thus long enough to overflow our default author length <user@example.com>" -m "Edit new_file.txt by a long author" 

As for the tests, I'm sorry that I have not checked the entire suite (or rather: that I have missed changes I made). Attached, you will find an additional patch against the current trunk (0002-Fix-tests-for-updated-git-repository.patch) which should fix the remaining test failures along with the updated repository fixture. This time, I have not included the repository fixture tar.gz file in the patch itself. You still want to to include it in the final SVN commit in test/fixtures/repositories/git_repository.tar.gz.

Actions #9

Updated by Holger Just 5 months ago

Also, when I load the repository in the current trunk and I click Load all revisions, I receive: [an exception]

This happens if you chose the (default) repository path encoding of UTF-8. However, the repository in git_repository.tar.gz is encoded with ISO-8859-1. When choosing the ISO-8859-1 path encoding in the repo settings, it works for me. This path encoding is also set in the test files.

Actions #10

Updated by Marius BĂLTEANU 5 months ago

  • Target version changed from 6.0.0 to 5.0.10

I've committed the fixes, thanks Holger for fixing the issues so quickly.

Actions #11

Updated by Marius BĂLTEANU 5 months ago

  • Status changed from Reopened to Resolved
Actions #12

Updated by Marius BĂLTEANU 5 months ago

  • Status changed from Resolved to Closed

Merged to stable branches.

Actions

Also available in: Atom PDF