[obs-commits] [openSUSE/open-build-service] 5a83a1: [backend] [api] check the sha256 of file if needed
![](https://seccdn.libravatar.org/avatar/bc67c2666cfb0f5c7770293291610cc9.jpg?s=120&d=mm&r=g)
Branch: refs/heads/master
Home: https://github.com/openSUSE/open-build-service
Commit: 5a83a195ffbdc1115566ed28b38c8c2665e50ba3
https://github.com/openSUSE/open-build-service/commit/5a83a195ffbdc1115566ed...
Author: marco
![](https://seccdn.libravatar.org/avatar/8f947802c8278f7de589d948d49d1c97.jpg?s=120&d=mm&r=g)
Hi, On 2017-10-27 05:59:59 -0700, Michael Schroeder wrote:
Branch: refs/heads/master Home: https://github.com/openSUSE/open-build-service Commit: 5a83a195ffbdc1115566ed28b38c8c2665e50ba3 https://github.com/openSUSE/open-build-service/commit/5a83a195ffbdc1115566ed... Author: marco
Date: 2017-10-27 (Fri, 27 Oct 2017) Changed paths: M docs/api/api/api.txt M src/api/app/controllers/source_controller.rb M src/backend/BSSrcrep.pm M src/backend/BSXML.pm M src/backend/bs_srcserver
Log Message: ----------- [backend] [api] check the sha256 of file if needed
This will set $entry->{'hash'} = 'missing' if there is already a file on the server with the same md5sum, but in another project with the same package name.
Hmm isn't the code doing something different here (IMHO, the "wrong" thing)?
@@ -2969,6 +2979,21 @@ sub sourcecommitfilelist { push @missing, $entry; } else { die("duplicate file: $entry->{'name'}\n") if exists $files->{$entry->{'name'}}; + if ($entry->{'hash'}) { + my $fd = gensym; + BSRevision::revopen($orev, $entry->{'name'}, $entry->{'md5'}, $fd); + my $sha256 = Digest::SHA->new(256); + my $hash_to_check = "sha256:" . $sha256->addfile($fd)->hexdigest; + if ($hash_to_check ne $entry->{'hash'}) { + die("SHA missmatch for same md5sum in $packid for file $entry->{'name'} with sum $entry->{'md5'}\n"); + }
(nitpick: "mismatch")
+ } elsif ($cgi->{'withvalidate'}) { + if ((!$ofiles->{$entry->{'name'}} || $ofiles->{$entry->{'name'}} ne $entry->{'md5'}) || + (!$ofiles_expanded->{$entry->{'name'}} || $ofiles_expanded->{$entry->{'name'}} ne $entry->{'md5'})) { + $entry->{'hash'} = 'missing'; + push @missing, $entry;
If I understand this feature correctly, the "ne" should be an "eq", because if $ofiles->{$entry->{'name'}} equals $entry->{'md5'} it might be a potential md5 collision and we should check the sha256. Actually, I do not understand these $ofiles and $ofiles_expanded checks at all. IMHO, we should always set $entry->{'hash'} = 'missing', because if the file exists in the srcrep, there might be a potential md5 collision. That is, it should be something like this if ($entry->{'hash'}) { ... } else { $entry->{'hash'} = 'missing'; push @missing, $entry; } Marcus
participants (2)
-
Marcus Hüwe
-
Michael Schroeder