Skip Menu |

This queue is for tickets about the Config-IniFiles CPAN distribution.

Report information
The Basics
Id: 51445
Status: resolved
Priority: 0/
Queue: Config-IniFiles

People
Owner: Nobody in particular
Requestors: SHE [...] cpan.org
Cc:
AdminCc:

Bug Information
Severity: Important
Broken in: (no value)
Fixed in: (no value)



Subject: 2.52 CRLF-ini with multi-value params fails under Linux
This ini file 1:# This ini-file has DOS-style line endings (CRLF) 2:[app_params] 3:pathes = <<EOTP 4:path1 5:path2 6:EOTP 7: read by the script below works on windows: Show quoted text
>initest.pl
Created pathes: path1 path2 fails on Linux: # ./initest.pl Can't use an undefined value as a symbol reference at /usr/lib/perl5/site_perl/5.8.8/Config/IniFiles.pm line 682, <GEN0> line 56. After changing the ini to UNIX line feeds it is working on both systems. The idea however, is that the same configurations should be interchangeable between platforms. And in our case should be editable by layouters with little understanding of the backgrounds. Same test script on both systems: #!/usr/bin/perl use strict; use warnings; use Config::IniFiles; my $defaultcfg = new Config::IniFiles -file => './initest.ini' or die "Error reading from './initest.ini': $!"; print "Created\n"; if ($defaultcfg->exists('app_params', 'pathes')) { my @pathes = $defaultcfg->val('app_params', 'pathes'); print "pathes:\n", join("\n", @pathes), "\n"; } exit 0; Trying to figure it out myself, I found that in sub ReadConfig on Linux and inside the loop on "here" values the \x0a part is being left over by the generic line separator: $self->{line_ends}='\x0d' $eotmark='EOTP' $_=' path1'=\x0a\x70\x61\x74\x68\x31 $_=' path2'=\x0a\x70\x61\x74\x68\x32 $_=' EOTP'=\x0a\x45\x4f\x54\x50 $_=''= $foundeot='0' Unfortunately I have no idea how this can be solved in an easy way. (And without possibly corrupting unicode input.) Thank you, Steffen Heinrich
Subject: cause and possible patch
After some more delving I come to think that it is not possible to determine a separator consisting of 2 bytes in the intended way, by just reading in 1 byte at a time. The reason why this never became obvious is that on windows the \x0d (CR) characters are stripped from a file unless it gets opened in binmode. (This of course, would cripple any utf encoding.) So, \x0a is the proper line separator there. And on Unix systems it is rarely causing problems because the remaining \x0a (after the separator has been set to \x0d) is attached to the beginning of next line, usually a param name, which will get trimmed before further processing. Only in 'here' syntax, when multiple values are defined, the end label cannot be found. I suggest the following patch to sub _nextline: OUT: 656 do { 657 local $/=\1; my $nextchar=<$fh>; 658 return undef if (!defined $nextchar); 659 $_ .= $nextchar; 660 } until (m/(\015\012?|\012|\025|\n)$/s); 661 $self->{line_ends}=$1; Replace with IN: { local $/=\1; my $nextchar; do { $nextchar=<$fh>; return undef if (!defined $nextchar); $_ .= $nextchar; } until (m/((\015|\012|\025|\n)$)/s); $self->{line_ends}=$1; if ($nextchar eq "\x0d") { # peek at the next char $nextchar = <$fh>; if ($nextchar eq "\x0a") { $self->{line_ends} .= "\x0a"; } else { seek $fh, -1, 1; } } } Regards, Steffen
Hi Steffen! If possible, please write a test case, and submit a unified diff patch against: https://config-inifiles.svn.sourceforge.net/svnroot/config-inifiles/trunk/ (Use "svn diff"). This will help me apply it. Regards, -- Shlomi Fish On Wed Nov 11 14:46:44 2009, SHE wrote: Show quoted text
> After some more delving I come to think that it is not possible to > determine a separator consisting of 2 bytes in the intended way, by just > reading in 1 byte at a time. > The reason why this never became obvious is that on windows the \x0d > (CR) characters are stripped from a file unless it gets opened in binmode. > (This of course, would cripple any utf encoding.) > So, \x0a is the proper line separator there. > > And on Unix systems it is rarely causing problems because the remaining > \x0a (after the separator has been set to \x0d) is attached to the > beginning of next line, usually a param name, which will get trimmed > before further processing. > > Only in 'here' syntax, when multiple values are defined, the end label > cannot be found. > > I suggest the following patch to sub _nextline: > > OUT: > > 656 do { > 657 local $/=\1; my $nextchar=<$fh>; > 658 return undef if (!defined $nextchar); > 659 $_ .= $nextchar; > 660 } until (m/(\015\012?|\012|\025|\n)$/s); > 661 $self->{line_ends}=$1; > > Replace with > IN: > { > local $/=\1; > my $nextchar; > do { > $nextchar=<$fh>; > return undef if (!defined $nextchar); > $_ .= $nextchar; > } until (m/((\015|\012|\025|\n)$)/s); > > $self->{line_ends}=$1; > > if ($nextchar eq "\x0d") { > # peek at the next char > $nextchar = <$fh>; > if ($nextchar eq "\x0a") { > $self->{line_ends} .= "\x0a"; > } else { > seek $fh, -1, 1; > } > } > } > > Regards, Steffen
On Do. 12. Nov. 2009, 03:02:08, SHLOMIF wrote: Show quoted text
> Hi Steffen! > > If possible, please write a test case, and submit a unified diff patch > against: > > https://config-inifiles.svn.sourceforge.net/svnroot/config-inifiles/trunk/ > > (Use "svn diff"). > > This will help me apply it. > > Regards, > > -- Shlomi Fish >
Hello Shlomi! the diff is attached. But since I've never done that test case stuff before it might take some time I'm afraid - shame on me. :-( Could you maybe provide a helpful link to get me started? It would be much appreciated. Thanks, Steffen
Index: trunk/config-inifiles/lib/Config/IniFiles.pm =================================================================== --- trunk/config-inifiles/lib/Config/IniFiles.pm (Revision 146) +++ trunk/config-inifiles/lib/Config/IniFiles.pm (Arbeitskopie) @@ -653,12 +653,25 @@ if (!exists $self->{line_ends}) { # no $self->{line_ends} is a hint set by caller that we are at # the first line (kludge kludge). - do { - local $/=\1; my $nextchar=<$fh>; - return undef if (!defined $nextchar); - $_ .= $nextchar; - } until (m/(\015\012?|\012|\025|\n)$/s); - $self->{line_ends}=$1; + { + local $/=\1; + my $nextchar; + do { + $nextchar=<$fh>; + return undef if (!defined $nextchar); + $_ .= $nextchar; + } until (m/((\015|\012|\025|\n)$)/s); + $self->{line_ends}=$1; + if ($nextchar eq "\x0d") { + # peek at the next char + $nextchar = <$fh>; + if ($nextchar eq "\x0a") { + $self->{line_ends} .= "\x0a"; + } else { + seek $fh, -1, 1; + } + } + } # If there's a UTF BOM (Byte-Order-Mark) in the first # character of the first line then remove it before processing
On Thu Nov 12 11:53:21 2009, SHE wrote: Show quoted text
> On Do. 12. Nov. 2009, 03:02:08, SHLOMIF wrote:
> > Hi Steffen! > > > > If possible, please write a test case, and submit a unified diff patch > > against: > > > >
https://config-inifiles.svn.sourceforge.net/svnroot/config-inifiles/trunk/ Show quoted text
> > > > (Use "svn diff"). > > > > This will help me apply it. > > > > Regards, > > > > -- Shlomi Fish > >
> Hello Shlomi! > > the diff is attached. > But since I've never done that test case stuff before it might take some > time I'm afraid - shame on me. > :-( > > Could you maybe provide a helpful link to get me started? > It would be much appreciated. >
Hi Steffen! I guess you should read Test-Tutorial (though I'm not sure if I have ever read it, but it has a good repuation): http://search.cpan.org/dist/Test-Simple/lib/Test/Tutorial.pod And touting my own horn, I'd like to note this: http://www.shlomifish.org/lecture/Perl/Newbies/lecture5/ The automated tests part is at the start. Are you familiar with "./Build test"? Make sure it passes all tests (including the new ones) after the patch. Regards, -- Shlomi Fish
On Do. 12. Nov. 2009, 14:09:55, SHLOMIF wrote: Show quoted text
> On Thu Nov 12 11:53:21 2009, SHE wrote:
> > On Do. 12. Nov. 2009, 03:02:08, SHLOMIF wrote:
> > > Hi Steffen! > > > > > > If possible, please write a test case, and submit a unified diff patch > > > against: > > > > > >
> https://config-inifiles.svn.sourceforge.net/svnroot/config-inifiles/trunk/
> > > > > > (Use "svn diff"). > > > > > > This will help me apply it. > > > > > > Regards, > > > > > > -- Shlomi Fish > > >
> > Hello Shlomi! > > > > the diff is attached. > > But since I've never done that test case stuff before it might take some > > time I'm afraid - shame on me. > > :-( > > > > Could you maybe provide a helpful link to get me started? > > It would be much appreciated. > >
> > Hi Steffen! > > I guess you should read Test-Tutorial (though I'm not sure if I have > ever read it, but it has a good repuation): > > http://search.cpan.org/dist/Test-Simple/lib/Test/Tutorial.pod > > And touting my own horn, I'd like to note this: > > http://www.shlomifish.org/lecture/Perl/Newbies/lecture5/ > > The automated tests part is at the start. Are you familiar with "./Build > test"? Make sure it passes all tests (including the new ones) after the > patch. > > Regards, > > -- Shlomi Fish
Hi Shlomi, here is it, my first test case ever. No sorry, I am not familiar with ./Build test and don't know how to invoke it. But a normal make test went equally well on windows and linux (and failed on linux with the unpatched version). Thank you for your help, Steffen (I very much enjoy your web site. Will take a closer look occasionally.)
#!/usr/bin/perl # This script attempts to reproduce: # http://rt.cpan.org/Public/Bug/Display.html?id=51445 # # #51445: 2.52 CRLF-ini with multi-value params fails under Linux use Test::More tests => 20; use strict; use warnings; use Config::IniFiles; { # being pedantic, we don't take line breaks from this or an external file for granted my $sample_ini = "<eol> <sol># this is a sample file for testing the proper detection of line endings in Config::Inifiles<eol> <sol><eol> <sol>[single values]<eol> <sol>firstval = first value<eol> <sol>secondval=2nd<eol> <sol><eol> <sol># in v2.52 on linux multi values with crlf lines are failing<eol> <sol>[multi value]<eol> <sol>Pathes=<<EOT<eol> <sol>path1<eol> <sol>path2<eol> <sol>EOT<eol> <sol><eol> <sol>"; chdir('t') if ( -d 't' ); foreach my $lf (("\x0d\x0a", "\x0d", "\x0a", "\x15", "\n")) { my $ini = $sample_ini; $ini =~ s/<eol>[^<]*<sol>/$lf/g; open my $INI, '>:raw', 'test25.ini' or die $!; print $INI $ini; close $INI; my $lf_print = join('', map {sprintf '\x%0.2x', ord $_} split(//, $lf)); my $cfg = Config::IniFiles->new(-file => 'test25.ini'); # TEST ok($cfg, "Loading from a '$lf_print'-separated file"); # TEST my $value = $cfg->val('single values', 'firstval'); is ( $value, 'first value', "Reading a single value from a '$lf_print'-separated file" ); # TEST $value = $cfg->val('single values', 'secondval'); is ( $value, '2nd', "Reading a single value from a '$lf_print'-separated file" ); my @vals = $cfg->val("multi value", "Pathes"); # TEST is_deeply( \@vals, ['path1', 'path2'], "Reading a multiple value from a '$lf_print'-separated file", ); } }
On Thu Nov 12 18:53:47 2009, SHE wrote: Show quoted text
> On Do. 12. Nov. 2009, 14:09:55, SHLOMIF wrote:
> > On Thu Nov 12 11:53:21 2009, SHE wrote:
> > > On Do. 12. Nov. 2009, 03:02:08, SHLOMIF wrote:
> > > > Hi Steffen! > > > > > > > > If possible, please write a test case, and submit a unified diff
patch Show quoted text
> > > > against: > > > > > > > >
> >
https://config-inifiles.svn.sourceforge.net/svnroot/config-inifiles/trunk/ Show quoted text
> > > > > > > > (Use "svn diff"). > > > > > > > > This will help me apply it. > > > > > > > > Regards, > > > > > > > > -- Shlomi Fish > > > >
> > > Hello Shlomi! > > > > > > the diff is attached. > > > But since I've never done that test case stuff before it might
take some Show quoted text
> > > time I'm afraid - shame on me. > > > :-( > > > > > > Could you maybe provide a helpful link to get me started? > > > It would be much appreciated. > > >
> > > > Hi Steffen! > > > > I guess you should read Test-Tutorial (though I'm not sure if I have > > ever read it, but it has a good repuation): > > > > http://search.cpan.org/dist/Test-Simple/lib/Test/Tutorial.pod > > > > And touting my own horn, I'd like to note this: > > > > http://www.shlomifish.org/lecture/Perl/Newbies/lecture5/ > > > > The automated tests part is at the start. Are you familiar with "./Build > > test"? Make sure it passes all tests (including the new ones) after the > > patch. > > > > Regards, > > > > -- Shlomi Fish
> > Hi Shlomi, > > here is it, my first test case ever. >
Great, I applied it along with your patch (I had to apply the patch semi-manually, possibly due to CRLF/LF problems) and modified the test case a bit to avoid chdir'ing into t/ and to delete the file after use. Many thanks! Show quoted text
> No sorry, I am not familiar with ./Build test and don't know how to > invoke it.
perl Build.PL ; ./Build ; ./Build test is the Module-Build alternative to ExtUtils-MakeMaker's "perl Makefile.PL ; make ; make test". But EU-MM will also work here by virtue of Module::Build::Compat. Show quoted text
> But a normal make test went equally well on windows and linux (and > failed on linux with the unpatched version). >
Yes, I saw that. Show quoted text
> Thank you for your help,
You're welcome. Show quoted text
> Steffen > > (I very much enjoy your web site. Will take a closer look occasionally.)
I'm glad you do and thanks. Regards, -- Shlomi Fish
Fixed in 2.53. Closing as resolved. Thanks!