GNU bug report logs - #21757
md5sum: Accept different line endings in .md5 files

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 21757 in the body.
You can then email your comments to 21757 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: MI <mi.lists <at> alma.ch>
To: bug-coreutils <at> gnu.org
Subject: md5sum: Accept different line endings in .md5 files
Date: Sun, 25 Oct 2015 19:05:56 +0100

Currently, "md5sum -c" gives a cryptic error when the line endings in the .md5 file 
are not LF (\x0A). It would be helpful if md5sum would just "do the right" and 
understand the 2 other newline standards.

.md5 files created on non-Unix systems may use line endings other than LF . Windows 
uses CRLF (\x0D\x0A), and maybe some Mac utility still uses CR (\x0D).

The error message is cryptic in a Linux terminal with CRLF files, because it 
overwrites the file name, since it just prints out the CR character:

    $ md5sum -w -c Some_File_From_Windows.md5
    : No such file or directoryWindows.mxf
    : FAILED open or readndows.mxf
    md5sum: WARNING: 1 listed file could not be read

Message #10 received at 21757-done <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: MI <mi.lists <at> alma.ch>, 21757-done <at> debbugs.gnu.org
Subject: Re: bug#21757: md5sum: Accept different line endings in .md5 files
Date: Mon, 26 Oct 2015 01:51:10 +0000

[Message part 1 (text/plain, inline)]

On 25/10/15 18:05, MI wrote:
> Currently, "md5sum -c" gives a cryptic error when the line endings in the .md5 file 
> are not LF (\x0A). It would be helpful if md5sum would just "do the right" and 
> understand the 2 other newline standards.
> 
> .md5 files created on non-Unix systems may use line endings other than LF . Windows 
> uses CRLF (\x0D\x0A), and maybe some Mac utility still uses CR (\x0D).
> 
> The error message is cryptic in a Linux terminal with CRLF files, because it 
> overwrites the file name, since it just prints out the CR character:
> 
>      $ md5sum -w -c Some_File_From_Windows.md5
>      : No such file or directoryWindows.mxf
>      : FAILED open or readndows.mxf
>      md5sum: WARNING: 1 listed file could not be read

This is mainly a quoting issue.  With the attached patch,
the much more obvious error is output:

  md5sum: ‘...Windows.mxf\r’: No such file or directory
  ‘...Windows.mxf\r’: FAILED open or read
  md5sum: WARNING: 1 listed file could not be read

As for special case handling of '\r', it's probably best not
to do that as it would introduce edge case issues for --check.
I.E. note supporting file names with '\r' chars, or incorrectly
validating an attacker that did:

  cp name name$'\r'
  echo attack > name

cheers,
Pádraig.

[md5sum-quote.patch (text/x-patch, attachment)]

Message #13 received at 21757 <at> debbugs.gnu.org (full text, mbox):

From: Bernhard Voelker <mail <at> bernhard-voelker.de>
To: 21757 <at> debbugs.gnu.org, P <at> draigBrady.com, mi.lists <at> alma.ch
Subject: Re: bug#21757: md5sum: Accept different line endings in .md5 files
Date: Mon, 26 Oct 2015 08:11:25 +0100

On 10/26/2015 02:51 AM, Pádraig Brady wrote:
> From 5e3d017e7bc66cf6f666160f774944c2ff52d1ae Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?P=C3=A1draig=20Brady?= <P <at> draigBrady.com>
> Date: Mon, 26 Oct 2015 01:26:04 +0000
> Subject: [PATCH] md5sum: quote all printed file names
> 
> This is especially significant when using --check
> with files generated on a windows system, where the \r
> characters produce corrupted and confusing error messages.
> This also ensures status messages are output on a single line.
> 
> * src/md5sum.c: Use quote() for printed file names.
> * tests/misc/md5sum.pl: Adjust accordingly.
> * NEWS: Mention the change in behavior.
> Fixes http://bugs.gnu.org/21757

tests/misc/sha1sum.pl now fails; you forgot to adjust the ERR
messages in there.

Otherwise, the patch looks good to me.
Thanks.

BTW: I was quite surprised to see that the other utilities -
sha{224,256,384,512}sum - don't have a single negative test.
I mean, there should be at least one case for each of these
utilities to check whether an invalid checksum leads to an
error message and an exit status != 0.

Have a nice day,
Berny

Message #16 received at 21757 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Bernhard Voelker <mail <at> bernhard-voelker.de>, 21757 <at> debbugs.gnu.org,
 mi.lists <at> alma.ch
Subject: Re: bug#21757: md5sum: Accept different line endings in .md5 files
Date: Tue, 27 Oct 2015 17:21:31 +0000

On 26/10/15 07:11, Bernhard Voelker wrote:
> On 10/26/2015 02:51 AM, Pádraig Brady wrote:
>> From 5e3d017e7bc66cf6f666160f774944c2ff52d1ae Mon Sep 17 00:00:00 2001
>> From: =?UTF-8?q?P=C3=A1draig=20Brady?= <P <at> draigBrady.com>
>> Date: Mon, 26 Oct 2015 01:26:04 +0000
>> Subject: [PATCH] md5sum: quote all printed file names
>>
>> This is especially significant when using --check
>> with files generated on a windows system, where the \r
>> characters produce corrupted and confusing error messages.
>> This also ensures status messages are output on a single line.
>>
>> * src/md5sum.c: Use quote() for printed file names.
>> * tests/misc/md5sum.pl: Adjust accordingly.
>> * NEWS: Mention the change in behavior.
>> Fixes http://bugs.gnu.org/21757
> 
> tests/misc/sha1sum.pl now fails; you forgot to adjust the ERR
> messages in there.

Pushed with that fix.

> Otherwise, the patch looks good to me.
> Thanks.
> 
> BTW: I was quite surprised to see that the other utilities -
> sha{224,256,384,512}sum - don't have a single negative test.
> I mean, there should be at least one case for each of these
> utilities to check whether an invalid checksum leads to an
> error message and an exit status != 0.

Maybe, though it would only be testing the same logic
as they share the same source as sha1sum?

thanks for the review,
Pádraig.

Message #19 received at 21757 <at> debbugs.gnu.org (full text, mbox):

From: Bernhard Voelker <mail <at> bernhard-voelker.de>
To: Pádraig Brady <P <at> draigBrady.com>, 21757 <at> debbugs.gnu.org, 
 mi.lists <at> alma.ch
Subject: Re: bug#21757: md5sum: Accept different line endings in .md5 files
Date: Wed, 28 Oct 2015 07:40:14 +0100

On 10/27/2015 06:21 PM, Pádraig Brady wrote:
> On 26/10/15 07:11, Bernhard Voelker wrote:
>> BTW: I was quite surprised to see that the other utilities -
>> sha{224,256,384,512}sum - don't have a single negative test.
>> I mean, there should be at least one case for each of these
>> utilities to check whether an invalid checksum leads to an
>> error message and an exit status != 0.
> 
> Maybe, though it would only be testing the same logic
> as they share the same source as sha1sum?

I know, but who knows if it will always be like that?  And what
about exclusive code paths?  And finally, we're linking against
other libraries like glibc and openssl which could introduce
strange effects, or which do simply not work as expected on
some funny platforms/builds.

I think a couple of tests would not harm:
* verify pre-known checksum for empty input,
* verify pre-known checksum for non-empty input,
* verify that --check fails for badly formatted checksum line,
* verify that --check works for a good checksum,
* verify that --check fails for a bad checksum.

I'm a bit busy this week, so - unless someone else comes up with
a patch - I'll try to propose a patch next week.

Thanks & have a nice day,
Berny

Message #22 received at 21757 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Bernhard Voelker <mail <at> bernhard-voelker.de>, 21757 <at> debbugs.gnu.org,
 mi.lists <at> alma.ch
Subject: Re: bug#21757: md5sum: Accept different line endings in .md5 files
Date: Wed, 28 Oct 2015 14:14:02 +0000

On 28/10/15 06:40, Bernhard Voelker wrote:
> On 10/27/2015 06:21 PM, Pádraig Brady wrote:
>> On 26/10/15 07:11, Bernhard Voelker wrote:
>>> BTW: I was quite surprised to see that the other utilities -
>>> sha{224,256,384,512}sum - don't have a single negative test.
>>> I mean, there should be at least one case for each of these
>>> utilities to check whether an invalid checksum leads to an
>>> error message and an exit status != 0.
>>
>> Maybe, though it would only be testing the same logic
>> as they share the same source as sha1sum?
> 
> I know, but who knows if it will always be like that?  And what
> about exclusive code paths?  And finally, we're linking against
> other libraries like glibc and openssl which could introduce
> strange effects, or which do simply not work as expected on
> some funny platforms/builds.
> 
> I think a couple of tests would not harm:
> * verify pre-known checksum for empty input,
> * verify pre-known checksum for non-empty input,
> * verify that --check fails for badly formatted checksum line,
> * verify that --check works for a good checksum,
> * verify that --check fails for a bad checksum.
> 
> I'm a bit busy this week, so - unless someone else comes up with
> a patch - I'll try to propose a patch next week.

+1

It would be good to refactor at the same time equiv to:

for chk in md5 sha1 sha224 ...; do
  ${chk}sum ...
done

cheers,
Pádraig

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.