Bug #96225 for Config-Properties: Problem with 5

Wed Jun 04 20:13:45 2014 bfishburne [...] gmail.com - Ticket created

Subject:	Problem with 5_save.t
Date:	Wed, 4 Jun 2014 20:13:35 -0400
To:	bug-Config-Properties [...] rt.cpan.org
From:	William Fishburne <bfishburne [...] gmail.com>

Hi, I am running Windows 7 Home Premium (64 bit) SP1 on a 64 bit system. Strawberry Perl: This is perl 5, version 18, subversion 2 (v5.18.2) built for MSWin32-x64-multi-thread I went to CPAN and ran an install on Config::Properties (which is great, incidentally) It failed on t/5_save.t: t/5_save.t ..... 1/26 # Failed test 'same key/value' # at t/5_save.t line 32. # got: undef # expected: '4 19:31:41 2014' # Looks like you planned 26 tests but ran 27. # Looks like you failed 1 test of 27 run. t/5_save.t ..... Dubious, test returned 1 (wstat 256, 0x100) I took a look at 5_save.t and it looks like the culprit involves the header being stored on the test configuration file. It is attached to this email and the file name is r54_HaG3rm (I uncommented the line that deleted it and also uncommented the line that gave me the name of the temporary file). The file seemed to have a larger/different header then the tests expected. Brett Fishburne. -- *"Success is liking yourself, liking what you do, and liking how you do it." -- *Maya Angelou

Download r54_HaG3rm
application/octet-stream 1.1k

Message body not shown because it is not plain text.

Tue May 05 11:49:48 2015 Alexander.Brett [...] Sophos.com - Correspondence added

Subject:	[rt.cpan.org #96225] Diagnosis & fix
Date:	Tue, 5 May 2015 15:49:32 +0000
To:	"bug-Config-Properties [...] rt.cpan.org" <bug-Config-Properties [...] rt.cpan.org>
From:	Alexander Brett <Alexander.Brett [...] Sophos.com>

Hiya, I have analysed and fixed this issue. The problem is that 'tempfile()' produces a filehandle which is in binmode, which means that '$cfg->store($fh, "test header");' writes the file with LF, not CRLF. However, when the file is reopened, it is not in binmode, so it is read as if it had CRLF, which causes the timestamp to be incorrectly interpreted as a property, rather than a comment. The fix is to add 'binmode R;' after opening R in t/5_test.t. Hope this helps, Alex @@ -20,4 +20,9 @@ open(R, '<', $fn) or die "unable to open temporal file with properties saved"; + +# On windows, tempfile() creates a filehandle with binmode on, so +# $fn was written with LF, not CRLF. In order to read this file +# we ought to binmode this filehandle too. +binmode R; my $cfg2=Config::Properties->new(); Show quoted text

________________________________ Sophos Ltd, a company registered in England and Wales, number 2096520, The Pentagon, Abingdon Science Park, Abingdon, OX14 3YP, United Kingdom

Tue May 05 11:49:48 2015 The RT System itself - Status changed from 'new' to 'open'

Tue May 05 15:41:34 2015 bfishburne [...] gmail.com - Correspondence added

Subject:	Re: [rt.cpan.org #96225] Diagnosis & fix
Date:	Tue, 5 May 2015 15:41:19 -0400
To:	bug-Config-Properties [...] rt.cpan.org
From:	William Fishburne <bfishburne [...] gmail.com>

Thank you! On Tue, May 5, 2015 at 11:49 AM, Alexander Brett via RT < bug-Config-Properties@rt.cpan.org> wrote: Show quoted text

> <URL: https://rt.cpan.org/Ticket/Display.html?id=96225 > > > Hiya, > > I have analysed and fixed this issue. The problem is that 'tempfile()' > produces a filehandle which is in binmode, which means that > '$cfg->store($fh, "test header");' writes the file with LF, not CRLF. > However, when the file is reopened, it is not in binmode, so it is read as > if it had CRLF, which causes the timestamp to be incorrectly interpreted as > a property, rather than a comment. > > The fix is to add 'binmode R;' after opening R in t/5_test.t. > > Hope this helps, > Alex > > @@ -20,4 +20,9 @@ > open(R, '<', $fn) > or die "unable to open temporal file with properties saved"; > + > +# On windows, tempfile() creates a filehandle with binmode on, so > +# $fn was written with LF, not CRLF. In order to read this file > +# we ought to binmode this filehandle too. > +binmode R; > > my $cfg2=Config::Properties->new(); > > > ________________________________ > > Sophos Ltd, a company registered in England and Wales, number 2096520, The > Pentagon, Abingdon Science Park, Abingdon, OX14 3YP, United Kingdom > >

-- *"Success is liking yourself, liking what you do, and liking how you do it." -- *Maya Angelou

Wed May 06 09:20:15 2015 Alexander.Brett [...] Sophos.com - Correspondence added

Subject:	Re: [rt.cpan.org #96225] Diagnosis & fix
Date:	Wed, 6 May 2015 13:20:05 +0000
To:	"bug-Config-Properties [...] rt.cpan.org" <bug-Config-Properties [...] rt.cpan.org>
From:	Alexander Brett <Alexander.Brett [...] Sophos.com>

Since I'm not a maintainer, I can't actually apply the fix, but I'm hoping ~SALVA sees this and patches quickly :) Show quoted text

________________________________ Sophos Ltd, a company registered in England and Wales, number 2096520, The Pentagon, Abingdon Science Park, Abingdon, OX14 3YP, United Kingdom

Thu May 07 05:22:30 2015 salva [...] cpan.org - Correspondence added

According to Java properties documentation (http://docs.oracle.com/javase/7/docs/api/java/util/Properties.html): "A natural line is defined as a line of characters that is terminated either by a set of line terminator characters (\n or \r or \r\n) or by the end of the stream" So, the actual problem was that Config::Properties was only accepting the OS native line terminator instead of all of them. It should be fixed on the new version I have just uploaded to CPAN: https://metacpan.org/release/SALVA/Config-Properties-1.78 Thank you both for reporting the bug and the problem analysis!

Thu May 07 05:22:36 2015 salva [...] cpan.org - Status changed from 'open' to 'resolved'

Bug #96225 for Config-Properties: Problem with 5_save.t