Changeset - 5395bb85a991
[Not reviewed]
default
0 2 0
Mads Kiilerich - 8 years ago 2017-10-23 01:15:06
mads@kiilerich.com
diff: fix crash when displaying diff on a single file

File diff on an image file would fail on
%if op in 'DM':
because op was None:
TypeError: 'in <string>' requires string as left operand, not NoneType

But really, if op is None, we also don't want to show invalid "Show images"
links. Thus, guard the whole image display section with having an actual op.

_parse_gitdiff will never return op None, but wrapped_diff is more lazy and
might do that. It could be considered a bug in wrapped_diff, and this change is
just a bad workaround.
2 files changed with 17 insertions and 1 deletions:
0 comments (0 inline, 0 general)
kallithea/templates/changeset/diff_block.html
Show inline comments
 
## -*- coding: utf-8 -*-
 

	
 
<%def name="diff_block(a_repo_name, a_ref_type, a_ref_name, a_rev,
 
                       cs_repo_name, cs_ref_name, cs_ref_type, cs_rev,
 
                       file_diff_data)">
 
<div class="diff-collapse">
 
    <button data-target="${'diff-container-%s' % (id(file_diff_data))}" class="diff-collapse-button btn btn-link btn-sm">&uarr; ${_('Collapse Diff')} &uarr;</button>
 
</div>
 
%for id_fid, url_fid, op, a_filename, cs_filename, diff, stats in file_diff_data:
 
    ${diff_block_diffblock(id_fid, url_fid, op, diff,
 
        a_repo_name, a_rev, a_ref_type, a_ref_name, a_filename,
 
        cs_repo_name, cs_rev, cs_ref_type, cs_ref_name, cs_filename,
 
        'diff-container-%s' % id(file_diff_data))}
 
%endfor
 
</%def>
 

	
 
<%def name="diff_block_diffblock(id_fid, url_fid, op, diff,
 
    a_repo_name, a_rev, a_ref_type, a_ref_name, a_filename,
 
    cs_repo_name, cs_rev, cs_ref_type, cs_ref_name, cs_filename, cls)"
 
>
 
    <div id="${id_fid}_target"></div>
 
    <div id="${id_fid}" class="panel panel-default ${cls}">
 
        <div class="panel-heading clearfix">
 
                <div class="pull-left">
 
                    ${h.safe_unicode(cs_filename)}
 
                </div>
 
                <div class="pull-left diff-actions">
 
                  <span>
 
                    %if op == 'A':
 
                      <span class="no-file" data-toggle="tooltip" title="${_('No file before')}">
 
                        <i class="icon-minus-circled"></i></span>
 
                    %else:
 
                      <a href="${h.url('files_home', repo_name=a_repo_name, f_path=a_filename, revision=a_rev)}" data-toggle="tooltip" title="${_('File before')}">
 
                        <i class="icon-doc"></i></a>
 
                    %endif
 

	
 
                    %if op == 'A':
 
                      <span class="arrow" data-toggle="tooltip" title="${_('Added')}">&#10142;</span>
 
                    %elif op == 'M':
 
                      <span class="arrow" data-toggle="tooltip" title="${_('Modified')}">&#10142;</span>
 
                    %elif op == 'D':
 
                      <span class="arrow" data-toggle="tooltip" title="${_('Deleted')}">&#10142;</span>
 
                    %elif op == 'R':
 
                      <span class="arrow" data-toggle="tooltip" title="${_('Renamed')}">&#10142;</span>
 
                    %elif op is None:
 
                      <span class="arrow" data-toggle="tooltip" title="${_('No change')}">&#10142;</span>
 
                    %else:
 
                      <span class="arrow" data-toggle="tooltip" title="${_('Unknown operation: %r') % op}">&#10142;</span>
 
                    %endif
 

	
 
                    %if op == 'D':
 
                      <span class="no-file" data-toggle="tooltip" title="${_('No file after')}">
 
                        <i class="icon-minus-circled"></i></span>
 
                    %else:
 
                      <a href="${h.url('files_home', repo_name=cs_repo_name, f_path=cs_filename, revision=cs_rev)}" data-toggle="tooltip" title="${_('File after')}">
 
                        <i class="icon-doc"></i></a>
 
                    %endif
 
                  </span>
 

	
 
                  <a href="${h.url('files_diff_home',repo_name=cs_repo_name,f_path=h.safe_unicode(cs_filename),diff2=cs_rev,diff1=a_rev,diff='diff',fulldiff=1)}" data-toggle="tooltip" title="${_('Show full diff for this file')}">
 
                      <i class="icon-file-code"></i></a>
 
                  <a href="${h.url('files_diff_2way_home',repo_name=cs_repo_name,f_path=h.safe_unicode(cs_filename),diff2=cs_rev,diff1=a_rev,diff='diff',fulldiff=1)}" data-toggle="tooltip" title="${_('Show full side-by-side diff for this file')}">
 
                      <i class="icon-docs"></i></a>
 
                  <a href="${h.url('files_diff_home',repo_name=cs_repo_name,f_path=h.safe_unicode(cs_filename),diff2=cs_rev,diff1=a_rev,diff='raw')}" data-toggle="tooltip" title="${_('Raw diff')}">
 
                      <i class="icon-diff"></i></a>
 
                  <a href="${h.url('files_diff_home',repo_name=cs_repo_name,f_path=h.safe_unicode(cs_filename),diff2=cs_rev,diff1=a_rev,diff='download')}" data-toggle="tooltip" title="${_('Download diff')}">
 
                      <i class="icon-floppy"></i></a>
 
                  ${c.ignorews_url(request.GET, url_fid)}
 
                  ${c.context_url(request.GET, url_fid)}
 
                </div>
 
                <div class="pull-right">
 
                    ${_('Show inline comments')}
 
                    ${h.checkbox('checkbox-show-inline-' + id_fid, checked="checked",class_="show-inline-comments",**{'data-id_for':id_fid})}
 
                </div>
 
        </div>
 
        <div class="no-padding panel-body" data-f_path="${h.safe_unicode(cs_filename)}">
 
            ${diff|n}
 
            %if cs_filename.rsplit('.')[-1] in ['png', 'gif', 'jpg', 'bmp']:
 
            %if op and cs_filename.rsplit('.')[-1] in ['png', 'gif', 'jpg', 'bmp']:
 
              <div class="btn btn-image-diff-show">Show images</div>
 
              %if op == 'M':
 
                <div id="${id_fid}_image-diff" class="btn btn-image-diff-swap" style="display:none">Press to swap images</div>
 
              %endif
 
              <div>
 
                %if op in 'DM':
 
                  <img id="${id_fid}_image-diff-img-a" class="img-diff img-diff-swapable" style="display:none"
 
                      realsrc="${h.url('files_raw_home',repo_name=a_repo_name,revision=a_rev,f_path=a_filename)}" />
 
                %endif
 
                %if op in 'AM':
 
                  <img id="${id_fid}_image-diff-img-b" class="img-diff img-diff-swapable" style="display:none"
 
                      realsrc="${h.url('files_raw_home',repo_name=cs_repo_name,revision=cs_rev,f_path=cs_filename)}" />
 
                %endif
 
              </div>
 
            %endif
 
        </div>
 
    </div>
 
</%def>
 

	
 
<%def name="diff_block_js()">
 
<script type="text/javascript">
 
$(document).ready(function(){
 
    $('.btn-image-diff-show').click(function(e){
 
        $('.btn-image-diff-show').hide();
 
        $('.btn-image-diff-swap').show();
 
        $('.img-diff-swapable')
 
            .each(function(i,e){
 
                    $(e).prop('src', $(e).attr('realsrc'));
 
                })
 
            .show();
 
        });
 

	
 
    $('.btn-image-diff-swap').mousedown(function(e){
 
        $('#'+e.currentTarget.id+'-img-a.img-diff-swapable')
 
          .before($('#'+e.currentTarget.id+'-img-b.img-diff-swapable'));
 
    });
 
    var reset = function(e){
 
        $('#'+e.currentTarget.id+'-img-a.img-diff-swapable')
 
          .after($('#'+e.currentTarget.id+'-img-b.img-diff-swapable'));
 
    };
 
    $('.btn-image-diff-swap').mouseup(reset);
 
    $('.btn-image-diff-swap').mouseleave(reset);
 

	
 
    $('.diff-collapse-button').click(function(e) {
 
        $('.diff_block').toggleClass('hidden');
 
        var $button = $(e.currentTarget);
 
        var $target = $('.' + $button.data('target'));
 
        if($target.hasClass('hidden')){
 
            $target.removeClass('hidden');
 
            $button.html("&uarr; {0} &uarr;".format(_TM['Collapse Diff']));
 
        }
 
        else if(!$target.hasClass('hidden')){
 
            $target.addClass('hidden');
 
            $button.html("&darr; {0} &darr;".format(_TM['Expand Diff']));
 
        }
 
    });
 
    $('.show-inline-comments').change(function(e){
 
        var target = e.currentTarget;
 
        if(target == null){
 
            target = this;
 
        }
 
        var boxid = $(target).data('id_for');
 
        if(target.checked){
 
            $('#{0} .inline-comments'.format(boxid)).show();
 
        }else{
 
            $('#{0} .inline-comments'.format(boxid)).hide();
 
        }
 
    });
 
});
 
</script>
 
</%def>
kallithea/tests/functional/test_files.py
Show inline comments
 
@@ -384,384 +384,400 @@ class TestFilesController(TestController
 
                                      repo_name=repo.repo_name,
 
                                      revision='tip', f_path='/'),
 
                                 params={
 
                                    'content': "foo",
 
                                    'filename': filename,
 
                                    'location': location,
 
                                    '_authentication_token': self.authentication_token(),
 
                                 },
 
                                 status=302)
 
        try:
 
            self.checkSessionFlash(response, 'Successfully committed to %s'
 
                                   % posixpath.join(location, filename))
 
        finally:
 
            fixture.destroy_repo(repo.repo_name)
 

	
 
    # Git - add file
 
    def test_add_file_view_git(self):
 
        self.log_user()
 
        response = self.app.get(url('files_add_home',
 
                                      repo_name=GIT_REPO,
 
                                      revision='tip', f_path='/'))
 

	
 
    def test_add_file_into_git_missing_content(self):
 
        self.log_user()
 
        response = self.app.post(url('files_add_home',
 
                                      repo_name=GIT_REPO,
 
                                      revision='tip', f_path='/'),
 
                                 params={
 
                                     'content': '',
 
                                     '_authentication_token': self.authentication_token(),
 
                                 },
 
                                 status=302)
 
        self.checkSessionFlash(response, 'No content')
 

	
 
    def test_add_file_into_git_missing_filename(self):
 
        self.log_user()
 
        response = self.app.post(url('files_add_home',
 
                                      repo_name=GIT_REPO,
 
                                      revision='tip', f_path='/'),
 
                                 params={
 
                                    'content': "foo",
 
                                    '_authentication_token': self.authentication_token(),
 
                                 },
 
                                 status=302)
 

	
 
        self.checkSessionFlash(response, 'No filename')
 

	
 
    @parametrize('location,filename', [
 
        ('/abs', 'foo'),
 
        ('../rel', 'foo'),
 
        ('file/../foo', 'foo'),
 
    ])
 
    def test_add_file_into_git_bad_filenames(self, location, filename):
 
        self.log_user()
 
        response = self.app.post(url('files_add_home',
 
                                      repo_name=GIT_REPO,
 
                                      revision='tip', f_path='/'),
 
                                 params={
 
                                    'content': "foo",
 
                                    'filename': filename,
 
                                    'location': location,
 
                                    '_authentication_token': self.authentication_token(),
 
                                 },
 
                                 status=302)
 

	
 
        self.checkSessionFlash(response, 'Location must be relative path and must not contain .. in path')
 

	
 
    @parametrize('cnt,location,filename', [
 
        (1, '', 'foo.txt'),
 
        (2, 'dir', 'foo.rst'),
 
        (3, 'rel/dir', 'foo.bar'),
 
    ])
 
    def test_add_file_into_git(self, cnt, location, filename):
 
        self.log_user()
 
        repo = fixture.create_repo(u'commit-test-%s' % cnt, repo_type='git')
 
        response = self.app.post(url('files_add_home',
 
                                      repo_name=repo.repo_name,
 
                                      revision='tip', f_path='/'),
 
                                 params={
 
                                    'content': "foo",
 
                                    'filename': filename,
 
                                    'location': location,
 
                                    '_authentication_token': self.authentication_token(),
 
                                 },
 
                                 status=302)
 
        try:
 
            self.checkSessionFlash(response, 'Successfully committed to %s'
 
                                   % posixpath.join(location, filename))
 
        finally:
 
            fixture.destroy_repo(repo.repo_name)
 

	
 
    # Hg - EDIT
 
    def test_edit_file_view_hg(self):
 
        self.log_user()
 
        response = self.app.get(url('files_edit_home',
 
                                      repo_name=HG_REPO,
 
                                      revision='tip', f_path='vcs/nodes.py'))
 

	
 
    def test_edit_file_view_not_on_branch_hg(self):
 
        self.log_user()
 
        repo = fixture.create_repo(u'test-edit-repo', repo_type='hg')
 

	
 
        ## add file
 
        location = 'vcs'
 
        filename = 'nodes.py'
 
        response = self.app.post(url('files_add_home',
 
                                      repo_name=repo.repo_name,
 
                                      revision='tip', f_path='/'),
 
                                 params={
 
                                    'content': "def py():\n print 'hello'\n",
 
                                    'filename': filename,
 
                                    'location': location,
 
                                    '_authentication_token': self.authentication_token(),
 
                                 },
 
                                 status=302)
 
        response.follow()
 
        try:
 
            self.checkSessionFlash(response, 'Successfully committed to %s'
 
                                   % posixpath.join(location, filename))
 
            response = self.app.get(url('files_edit_home',
 
                                          repo_name=repo.repo_name,
 
                                          revision='tip', f_path=posixpath.join(location, filename)),
 
                                    status=302)
 
            self.checkSessionFlash(response,
 
                'You can only edit files with revision being a valid branch')
 
        finally:
 
            fixture.destroy_repo(repo.repo_name)
 

	
 
    def test_edit_file_view_commit_changes_hg(self):
 
        self.log_user()
 
        repo = fixture.create_repo(u'test-edit-repo', repo_type='hg')
 

	
 
        ## add file
 
        location = 'vcs'
 
        filename = 'nodes.py'
 
        response = self.app.post(url('files_add_home',
 
                                      repo_name=repo.repo_name,
 
                                      revision='tip',
 
                                      f_path='/'),
 
                                 params={
 
                                    'content': "def py():\n print 'hello'\n",
 
                                    'filename': filename,
 
                                    'location': location,
 
                                    '_authentication_token': self.authentication_token(),
 
                                 },
 
                                 status=302)
 
        response.follow()
 
        try:
 
            self.checkSessionFlash(response, 'Successfully committed to %s'
 
                                   % posixpath.join(location, filename))
 
            response = self.app.post(url('files_edit_home',
 
                                          repo_name=repo.repo_name,
 
                                          revision=repo.scm_instance.DEFAULT_BRANCH_NAME,
 
                                          f_path=posixpath.join(location, filename)),
 
                                     params={
 
                                        'content': "def py():\n print 'hello world'\n",
 
                                        'message': 'i committed',
 
                                        '_authentication_token': self.authentication_token(),
 
                                     },
 
                                    status=302)
 
            self.checkSessionFlash(response, 'Successfully committed to %s'
 
                                   % posixpath.join(location, filename))
 
        finally:
 
            fixture.destroy_repo(repo.repo_name)
 

	
 
    # Git - edit
 
    def test_edit_file_view_git(self):
 
        self.log_user()
 
        response = self.app.get(url('files_edit_home',
 
                                      repo_name=GIT_REPO,
 
                                      revision='tip', f_path='vcs/nodes.py'))
 

	
 
    def test_edit_file_view_not_on_branch_git(self):
 
        self.log_user()
 
        repo = fixture.create_repo(u'test-edit-repo', repo_type='git')
 

	
 
        ## add file
 
        location = 'vcs'
 
        filename = 'nodes.py'
 
        response = self.app.post(url('files_add_home',
 
                                      repo_name=repo.repo_name,
 
                                      revision='tip', f_path='/'),
 
                                 params={
 
                                    'content': "def py():\n print 'hello'\n",
 
                                    'filename': filename,
 
                                    'location': location,
 
                                    '_authentication_token': self.authentication_token(),
 
                                 },
 
                                 status=302)
 
        response.follow()
 
        try:
 
            self.checkSessionFlash(response, 'Successfully committed to %s'
 
                                   % posixpath.join(location, filename))
 
            response = self.app.get(url('files_edit_home',
 
                                          repo_name=repo.repo_name,
 
                                          revision='tip', f_path=posixpath.join(location, filename)),
 
                                    status=302)
 
            self.checkSessionFlash(response,
 
                'You can only edit files with revision being a valid branch')
 
        finally:
 
            fixture.destroy_repo(repo.repo_name)
 

	
 
    def test_edit_file_view_commit_changes_git(self):
 
        self.log_user()
 
        repo = fixture.create_repo(u'test-edit-repo', repo_type='git')
 

	
 
        ## add file
 
        location = 'vcs'
 
        filename = 'nodes.py'
 
        response = self.app.post(url('files_add_home',
 
                                      repo_name=repo.repo_name,
 
                                      revision='tip',
 
                                      f_path='/'),
 
                                 params={
 
                                    'content': "def py():\n print 'hello'\n",
 
                                    'filename': filename,
 
                                    'location': location,
 
                                    '_authentication_token': self.authentication_token(),
 
                                 },
 
                                 status=302)
 
        response.follow()
 
        try:
 
            self.checkSessionFlash(response, 'Successfully committed to %s'
 
                                   % posixpath.join(location, filename))
 
            response = self.app.post(url('files_edit_home',
 
                                          repo_name=repo.repo_name,
 
                                          revision=repo.scm_instance.DEFAULT_BRANCH_NAME,
 
                                          f_path=posixpath.join(location, filename)),
 
                                     params={
 
                                        'content': "def py():\n print 'hello world'\n",
 
                                        'message': 'i committed',
 
                                        '_authentication_token': self.authentication_token(),
 
                                     },
 
                                    status=302)
 
            self.checkSessionFlash(response, 'Successfully committed to %s'
 
                                   % posixpath.join(location, filename))
 
        finally:
 
            fixture.destroy_repo(repo.repo_name)
 

	
 
    # Hg - delete
 
    def test_delete_file_view_hg(self):
 
        self.log_user()
 
        response = self.app.get(url('files_delete_home',
 
                                     repo_name=HG_REPO,
 
                                     revision='tip', f_path='vcs/nodes.py'))
 

	
 
    def test_delete_file_view_not_on_branch_hg(self):
 
        self.log_user()
 
        repo = fixture.create_repo(u'test-delete-repo', repo_type='hg')
 

	
 
        ## add file
 
        location = 'vcs'
 
        filename = 'nodes.py'
 
        response = self.app.post(url('files_add_home',
 
                                      repo_name=repo.repo_name,
 
                                      revision='tip', f_path='/'),
 
                                 params={
 
                                    'content': "def py():\n print 'hello'\n",
 
                                    'filename': filename,
 
                                    'location': location,
 
                                    '_authentication_token': self.authentication_token(),
 
                                 },
 
                                 status=302)
 
        response.follow()
 
        try:
 
            self.checkSessionFlash(response, 'Successfully committed to %s'
 
                                   % posixpath.join(location, filename))
 
            response = self.app.get(url('files_delete_home',
 
                                          repo_name=repo.repo_name,
 
                                          revision='tip', f_path=posixpath.join(location, filename)),
 
                                    status=302)
 
            self.checkSessionFlash(response,
 
                'You can only delete files with revision being a valid branch')
 
        finally:
 
            fixture.destroy_repo(repo.repo_name)
 

	
 
    def test_delete_file_view_commit_changes_hg(self):
 
        self.log_user()
 
        repo = fixture.create_repo(u'test-delete-repo', repo_type='hg')
 

	
 
        ## add file
 
        location = 'vcs'
 
        filename = 'nodes.py'
 
        response = self.app.post(url('files_add_home',
 
                                      repo_name=repo.repo_name,
 
                                      revision='tip',
 
                                      f_path='/'),
 
                                 params={
 
                                    'content': "def py():\n print 'hello'\n",
 
                                    'filename': filename,
 
                                    'location': location,
 
                                    '_authentication_token': self.authentication_token(),
 
                                 },
 
                                 status=302)
 
        response.follow()
 
        try:
 
            self.checkSessionFlash(response, 'Successfully committed to %s'
 
                                   % posixpath.join(location, filename))
 
            response = self.app.post(url('files_delete_home',
 
                                          repo_name=repo.repo_name,
 
                                          revision=repo.scm_instance.DEFAULT_BRANCH_NAME,
 
                                          f_path=posixpath.join(location, filename)),
 
                                     params={
 
                                        'message': 'i committed',
 
                                        '_authentication_token': self.authentication_token(),
 
                                     },
 
                                    status=302)
 
            self.checkSessionFlash(response,
 
                                   'Successfully deleted file %s' % posixpath.join(location, filename))
 
        finally:
 
            fixture.destroy_repo(repo.repo_name)
 

	
 
    # Git - delete
 
    def test_delete_file_view_git(self):
 
        self.log_user()
 
        response = self.app.get(url('files_delete_home',
 
                                     repo_name=HG_REPO,
 
                                     revision='tip', f_path='vcs/nodes.py'))
 

	
 
    def test_delete_file_view_not_on_branch_git(self):
 
        self.log_user()
 
        repo = fixture.create_repo(u'test-delete-repo', repo_type='git')
 

	
 
        ## add file
 
        location = 'vcs'
 
        filename = 'nodes.py'
 
        response = self.app.post(url('files_add_home',
 
                                      repo_name=repo.repo_name,
 
                                      revision='tip', f_path='/'),
 
                                 params={
 
                                    'content': "def py():\n print 'hello'\n",
 
                                    'filename': filename,
 
                                    'location': location,
 
                                    '_authentication_token': self.authentication_token(),
 
                                 },
 
                                 status=302)
 
        response.follow()
 
        try:
 
            self.checkSessionFlash(response, 'Successfully committed to %s'
 
                                   % posixpath.join(location, filename))
 
            response = self.app.get(url('files_delete_home',
 
                                          repo_name=repo.repo_name,
 
                                          revision='tip', f_path=posixpath.join(location, filename)),
 
                                    status=302)
 
            self.checkSessionFlash(response,
 
                'You can only delete files with revision being a valid branch')
 
        finally:
 
            fixture.destroy_repo(repo.repo_name)
 

	
 
    def test_delete_file_view_commit_changes_git(self):
 
        self.log_user()
 
        repo = fixture.create_repo(u'test-delete-repo', repo_type='git')
 

	
 
        ## add file
 
        location = 'vcs'
 
        filename = 'nodes.py'
 
        response = self.app.post(url('files_add_home',
 
                                      repo_name=repo.repo_name,
 
                                      revision='tip',
 
                                      f_path='/'),
 
                                 params={
 
                                    'content': "def py():\n print 'hello'\n",
 
                                    'filename': filename,
 
                                    'location': location,
 
                                    '_authentication_token': self.authentication_token(),
 
                                 },
 
                                 status=302)
 
        response.follow()
 
        try:
 
            self.checkSessionFlash(response, 'Successfully committed to %s'
 
                                   % posixpath.join(location, filename))
 
            response = self.app.post(url('files_delete_home',
 
                                          repo_name=repo.repo_name,
 
                                          revision=repo.scm_instance.DEFAULT_BRANCH_NAME,
 
                                          f_path=posixpath.join(location, filename)),
 
                                     params={
 
                                        'message': 'i committed',
 
                                        '_authentication_token': self.authentication_token(),
 
                                     },
 
                                    status=302)
 
            self.checkSessionFlash(response,
 
                                   'Successfully deleted file %s' % posixpath.join(location, filename))
 
        finally:
 
            fixture.destroy_repo(repo.repo_name)
 

	
 
    def test_png_diff_no_crash_hg(self):
 
        self.log_user()
 
        response = self.app.get(url('files_diff_home',
 
                                    repo_name=HG_REPO,
 
                                    f_path='docs/theme/ADC/static/documentation.png',
 
                                    diff1='tip', diff2='tip'))
 
        response.mustcontain("""<pre>Binary file</pre>""")
 

	
 
    def test_png_diff_no_crash_git(self):
 
        self.log_user()
 
        response = self.app.get(url('files_diff_home',
 
                                    repo_name=GIT_REPO,
 
                                    f_path='docs/theme/ADC/static/documentation.png',
 
                                    diff1='master', diff2='master'))
 
        response.mustcontain("""<pre>Binary file</pre>""")
0 comments (0 inline, 0 general)