LEMONLDAP::NG : new feature: configuration syntax errors are now displayed in manager interface

=> TODO i18n in this function

build/lemonldap-ng/TODO

@@ -3,7 +3,7 @@ Lemonldap::NG TODO
 TODO list for Lemonldap::NG development
 
 - Priority: Normal	Status: In progress	Created: 2007\05\03 11-45-05
-Display errors in saveConf
+Display errors in saveConf => i18n to do
 - Priority: Normal	Status: Planning	Created: 2007\05\03 12-28-30
 Modify example to use nameVirtualHost instead of 127.0.0.x adresses
 - Priority: Low	Status: In progress	Created: 2007\05\03 10-41-36
@@ -22,7 +22,5 @@ Order rules :
  * split locationRules into 2 arrays
 - Priority: Low	Status: N/A	Created: 2007\05\05 21-58-53
 Documentation :
- * Translate FAQ in English (http://lemonldap.objectweb.org/)
  * Security document
- * AD Howto
  * apply.conf Howto

build/lemonldap-ng/debian/changelog

@@ -1,3 +1,9 @@
+lemonldap-ng (0.8.3) unstable; urgency=low
+
+  * Syntax errors in configuration are now displayed
+
+ -- Xavier Guimard <x.guimard@free.fr>  Sat, 23 Jun 2007 21:57:02 +0200
+
 lemonldap-ng (0.8.2.4) unstable; urgency=low
 
   * Bug in manager javascript.

build/lemonldap-ng/debian/liblemonldap-ng-manager-perl.install

@@ -3,6 +3,7 @@ debian/tmp/usr/share/perl5/Lemonldap/NG/Manager/_i18n.pm
 debian/tmp/usr/share/perl5/Lemonldap/NG/Manager/Help.pm
 debian/tmp/usr/share/perl5/Lemonldap/NG/Manager/Base.pm
 debian/tmp/usr/share/perl5/Lemonldap/NG/Manager/_HTML.pm
+debian/tmp/usr/share/perl5/Lemonldap/NG/Manager/_Response.pm
 debian/tmp/usr/share/perl5/Lemonldap/NG/Manager/SOAPServer.pm
 debian/tmp/usr/share/perl5/Lemonldap/NG/Manager/Restricted.pm
 debian/tmp/usr/share/perl5/auto/Lemonldap/NG/Manager

modules/lemonldap-ng-manager/Changes

@@ -1,5 +1,8 @@
 Revision history for Perl extension Lemonldap::NG::Manager.
 
+0.8   Sat jun 23 21:54:27 2007
+	- New feature: syntax errors are now displayed in the manager interface
+
 0.71  Mon jun 19 22:22:33 2007
 	- Bug in javascript : a 'z' is added in regexp
 

modules/lemonldap-ng-manager/MANIFEST

@@ -53,6 +53,7 @@ example/soapserver.pl
 lib/Lemonldap/NG/Manager.pm
 lib/Lemonldap/NG/Manager/_HTML.pm
 lib/Lemonldap/NG/Manager/_i18n.pm
+lib/Lemonldap/NG/Manager/_Response.pm
 lib/Lemonldap/NG/Manager/Apache/Session/SOAP.pm
 lib/Lemonldap/NG/Manager/Base.pm
 lib/Lemonldap/NG/Manager/Conf.pm

modules/lemonldap-ng-manager/lib/Lemonldap/NG/Manager.pm

@@ -7,6 +7,7 @@ use XML::Simple;
 use Lemonldap::NG::Manager::Base;
 use Lemonldap::NG::Manager::Conf;
 use Lemonldap::NG::Manager::_HTML;
+require Lemonldap::NG::Manager::_Response;
 require Lemonldap::NG::Manager::_i18n;
 require Lemonldap::NG::Manager::Help;
 use Lemonldap::NG::Manager::Conf::Constants;
@@ -16,7 +17,7 @@ use MIME::Base64;
 
 our @ISA = qw(Lemonldap::NG::Manager::Base);
 
-our $VERSION = '0.72';
+our $VERSION = '0.8';
 
 sub new {
     my ( $class, $args ) = @_;
@@ -260,7 +261,6 @@ sub buildTree {
     else {
     }
 
-    my $indice = 1;
     if ( $config->{locationRules} and %{ $config->{locationRules} } ) {
         $tree->{item}->{item}->{virtualHosts}->{item} = {};
         my $virtualHost = $tree->{item}->{item}->{virtualHosts}->{item};
@@ -327,25 +327,36 @@ sub xmlField {
 sub print_upload {
     my $self  = shift;
     my $datas = shift;
-    print $self->header( -type => "text/html" );
+    print $self->header( -type => "text/javascript" );
-    my $tmp = $self->upload($datas);
+    my $r = Lemonldap::NG::Manager::_Response->new();
-    if ($tmp) {
+    my $tmp = $self->upload($datas, $r);
-        print $tmp;
+    if ($tmp==0) {
+        $r->message(&txt_unknownError, &txt_checkLogs);
     }
-    else {
+    elsif ($tmp>0) {
-        print 0;
+        $r->setConfiguration($tmp);
+        $r->message(&txt_confSaved . " $tmp", &txt_warningConfNotApplied);
     }
+    elsif ($tmp == CONFIG_WAS_CHANGED) {
+        $r->message(&txt_saveFailure, &txt_configurationWasChanged);
+    }
+    elsif ($tmp == SYNTAX_ERROR) {
+        $r->message(&txt_saveFailure, &txt_syntaxError);
+    }
+    $r->send;
 }
 
 sub upload {
     my $self = shift;
-    my $config = $self->tree2conf(@_);
+    my $tree = shift;
-    return SYNTAX_ERROR unless( $self->checkConf($config) );
+    my $response = shift;
+    my $config = $self->tree2conf($tree, $response);
+    return SYNTAX_ERROR unless( $self->checkConf($config, $response) );
     return $self->config->saveConf($config);
 }
 
 sub tree2conf {
-    my ( $self, $tree ) = @_;
+    my ( $self, $tree, $response ) = @_;
     $tree = XMLin($$tree);
     my $config = {};
     # Load config number
@@ -411,78 +422,110 @@ sub tree2conf {
 sub checkConf {
     my $self = shift;
     my $config = shift;
+    my $response = shift;
     my $expr = '';
+    my $result = 1;
     # Check cookie name
-    return 0 unless( $config->{cookieName} =~ /^\w+$/ );
+    unless( $config->{cookieName} =~ /^\w+$/ ) {
+        $result = 0;
+        $response->error('"' . $config->{cookieName} . '" is not a valid cookie Name')
+    }
     # Check domain name
-    return 0 unless( $config->{domain} =~ /^\w[\w\.\-]*\w$/ );
+    unless( $config->{domain} =~ /^\w[\w\.\-]*\w$/ ) {
+        $result = 0;
+        $response->error('"' . $config->{domain} . '" is not a valid domain name');
+    }
     # Load variables
     foreach(keys %{ $config->{exportedVars} }) {
         # Reserved words
         if( $_ eq 'groups' ) {
-            print STDERR "$_ is not authorized in attribute names. Change it!\n";
+            $response->error( "\"$_\" is not authorized in attribute names. Change it!" );
-            return 0;
+            $result = 0;
         }
         if( $_ !~ /^\w+$/ ) {
-            print STDERR "$_ is not a valid attribute name\n";
+            $response->error("\"$_\" is not a valid attribute name");
-            return 0;
+            $result = 0;
+        }
+        if( $config->{exportedVars}->{$_} !~ /^\w+$/ ) {
+            $response->error("\"$config->{exportedVars}->{$_}\" is not a valid LDAP attribute");
+            $result = 0;
         }
         $expr .= "my \$$_ = '1';";
     }
     # Load and check macros
     my $safe = new Safe;
     $safe->share( '&encode_base64' );
+    $safe->reval( $expr );
+    if ( $@ ) {
+        $result = 0;
+        $response->error("Unknown errors in exported attributes ($@)");
+    }
     while( my($k, $v) = each( %{ $config->{macros} } ) ) {
         # Reserved words
         if( $k eq 'groups' ) {
-            print STDERR "$k is not authorized in macro names. Change it!\n";
+            $response->error("\"$k\" is not authorized in macro names. Change it!");
-            return 0;
+            $result = 0;
         }
         if( $k !~ /^[a-zA-Z]\w*$/ ) {
-            print STDERR "$k is not a valid macro name\n";
+            $response->error("$k is not a valid macro name");
-            return 0;
+            $result = 0;
+        }
+        if( $v =~ /(?<=[^=<\?])=(?!=)/ ) {
+            $response->warning("Macro $k contains an assignment ('='). Possible confusion with '=='.");
         }
         $expr .= "my \$$k = $v;";
     }
     # Test macro values;
     $safe->reval( $expr );
     if( $@ ) {
-        print STDERR "Error in macro syntax: $@\n";
+        $response->error("Error in macro syntax: $@");
-        return 0;
+        $result = 0;
     }
+    # TODO: check module name
+    # Check whatToTrace
+    unless ( $config->{whatToTrace} =~ /^\$?[a-zA-Z]\w*$/ ) {
+        $response->error("whatToTrace parameter can contain only an exported attribute or a macro");
+        $result = 0;
+    } 
     # Test groups
     $expr .= 'my $groups;';
     while( my($k,$v) = each( %{ $config->{groups} } ) ) {
         if( $k !~ /^[\w-]+$/ ) {
-            print STDERR "$k is not a valid group name\n";
+            $response->error("\"$k\" is not a valid group name");
-            return 0;
+            $result = 0;
+        }
+        if( $v =~ /(?<=[^=<\?])=(?!=)/ ) {
+            $response->warning("Group $k contains an assignment ('='). Possible confusion with '=='.");
         }
         $safe->reval( $expr . "\$groups = '$k' if($v);");
         if( $@ ) {
-            print STDERR "Syntax error in group $k: $@\n";
+            $response->error("Syntax error in group \"$k\": $@");
-            return 0;
+            $result = 0;
         }
     }
     # Test rules
     while( my($vh, $rules) = each( %{ $config->{locationRules} } ) ) {
         unless( $vh =~ /^\w[-\w\.]*$/ ) {
-            print STDERR "$vh is not a valid virtual host name\n";
+            $response->error("\"$vh\" is not a valid virtual host name");
-            return 0;
+            $result = 0;
         }
         while( my($reg, $v) = each( %{ $rules } ) ) {
             unless( $reg eq 'default' ) {
                 $reg =~ s/#/\\#/g;
                 $safe->reval( $expr . "my \$r = qr#$reg#;" );
                 if( $@ ) {
-                    print STDERR "Syntax error in regexp ($vh -> $reg)\n";
+                    $response->error("Syntax error in regexp ($vh -> $reg)");
-                    return 0;
+                    $result = 0;
                 }
             }
             unless( $v eq 'deny' or $v eq 'accept' ) {
+                if( $v =~ /(?<=[^=<\?])=(?!=)/ ) {
+                    $response->warning("Rule $vh -> $reg contains an assignment ('='). Possible confusion with '=='.");
+                }
                 $safe->reval( $expr . "my \$r=1 if($v);");
                 if( $@ ) {
-                    print STDERR "Syntax error in expression ($vh -> $reg)\n";
+                    $response->error("Syntax error in expression ($vh -> $reg)");
-                    return 0;
+                    $result = 0;
                 }
             }
         }
@@ -490,22 +533,25 @@ sub checkConf {
     # Test exported headers
     while( my($vh, $headers) = each( %{ $config->{exportedHeaders} } ) ) {
         unless( $vh =~ /^\w[-\w\.]*$/ ) {
-            print STDERR "$vh is not a valid virtual host name\n";
+            $response->error("\"$vh\" is not a valid virtual host name");
-            return 0;
+            $result = 0;
         }
         while( my($header, $v) = each( %{ $headers } ) ) {
             unless( $header =~ /^[\w][-\w]*$/ ) {
-                print STDERR "$header is not a valid HTTP header name ($vh)\n";
+                $response->error("\"$header\" is not a valid HTTP header name ($vh)");
-                return 0;
+                $result = 0;
+            }
+            if( $v =~ /(?<=[^=<\?])=(?!=)/ ) {
+                $response->warning("Header $vh -> $header contains an assignment ('='). Possible confusion with '=='.");
             }
             $safe->reval( $expr . "my \$r = $v;" );
             if( $@ ) {
-                print STDERR "Syntax error in header expression ($vh -> $header)\n";
+                $response->error("Syntax error in header expression ($vh -> $header)");
-                return 0;
+                $result = 0;
             }
         }
     }
-    1;
+    return $result;
 }
 
 # Apply subroutines

modules/lemonldap-ng-manager/lib/Lemonldap/NG/Manager/_HTML.pm

@@ -325,28 +325,12 @@ function help(s){
 
 function saveConf(){
   var h=tree2txt('root');
-  //document.getElementById('help').innerHTML="<pre>"+h+"</pre>";
   xhr_object.open("POST", "$ENV{SCRIPT_NAME}?lmQuery=upload",true);
   xhr_object.setRequestHeader("Content-type", "text/xml");
   xhr_object.setRequestHeader("Content-length", h.length);
   xhr_object.onreadystatechange = function() {
     if(xhr_object.readyState == 4){
-      var r=xhr_object.responseText;
+      window.setTimeout(xhr_object.responseText,0);
-      if(r>0) {
-        tree.setItemText('root','Configuration '+r);
-        document.getElementById('help').innerHTML='<h3>$text{confSaved} : '+r+'</h3>$text{warningConfNotApplied}';
-      }
-      else if(r<0) {
-        var txt='<h3>$text{saveFailure}: ';
-        if(r==#.CONFIG_WAS_CHANGED.qq#) {
-          txt+='$text{configurationWasChanged}';
-        }
-        else if(r==#.SYNTAX_ERROR.qq#) {
-          txt+='$text{syntaxError}';
-        }
-        document.getElementById('help').innerHTML=txt+'</h3>';
-      }
-      else document.getElementById('help').innerHTML='<h3>$text{unknownError}</h3>';
     }
     else document.getElementById('help').innerHTML='<h3>$text{waitingResult}</h3>';
   }

modules/lemonldap-ng-manager/lib/Lemonldap/NG/Manager/_Response.pm

@@ -0,0 +1,69 @@
+package Lemonldap::NG::Manager::_Response;
+
+our $VERSION = '0.1';
+
+sub new {
+    my $class = shift;
+    return bless { errors => [], warnings => [] }, $class;
+}
+
+sub print {
+    my $self = shift;
+    $self->{txt} .= $_ foreach(@_);
+}
+
+sub message {
+    my $self = shift;
+    my ($title, $txt) = @_;
+    $self->{txt} = "<h3>$title</h3><p>$txt</p>" . $self->{txt};
+}
+
+sub warning {
+    my $self = shift;
+    push @{$self->{warnings}}, @_;
+}
+
+sub error {
+    my $self = shift;
+    return scalar @{$self->{errors}} unless(@_);
+    push @{$self->{errors}}, @_;
+}
+
+sub setConfiguration {
+    my $self = shift;
+    $self->{configuration} = shift;
+}
+
+sub send {
+    my $self = shift;
+    my $buf;
+    $buf = "tree.setItemText('root','Configuration $self->{configuration}');"
+        if($self->{configuration});
+    if ( $self->error ) {
+        $self->{txt} .= "<h4>Errors</h4><ul>";
+        foreach ( @{$self->{errors}} ) {
+            s/</&lt;/g;
+            s/>/&gt;/g;
+            $self->{txt} .= "<li>$_</li>"
+        }
+        $self->{txt} .= "</ul>";
+    }
+    if ( $self->warning ) {
+        $self->{txt} .= "<h4>Warnings</h4><ul>";
+        foreach ( @{$self->{warnings}} ) {
+            s/</&lt;/g;
+            s/>/&gt;/g;
+            $self->{txt} .= "<li>$_</li>"
+        }
+        $self->{txt} .= "</ul>";
+    }
+    if($self->{txt}) {
+        $self->{txt} =~ s/'/\\'/g;
+        $self->{txt} =~ s/[\r\n]//g;
+        $buf .= "document.getElementById('help').innerHTML='$self->{txt}';";
+    }
+    print $buf;
+}
+
+1;
+__END__

modules/lemonldap-ng-manager/lib/Lemonldap/NG/Manager/_i18n.pm

@@ -94,7 +94,8 @@ sub fr {
         confirmDeleteConf       => "Vous allez effacer cette configuration. Confirmez-vous ?",
         configurationDeleted    => 'Configuration éffacée',
         configurationNotDeleted => 'Configuration non éffacée',
-        invalidVirtualHostName  => "Nom de d'hôte virtuel incorrect",       
+        invalidVirtualHostName  => "Nom de d'hôte virtuel incorrect",
+        checkLogs               => "Consultez les journaux d'Apache",
     };
 }
 
@@ -158,6 +159,7 @@ sub en {
         confirmDeleteConf       => "You're going to delete configuration. Do you confirm ?",
         configurationDeleted    => 'Configuration deleted',
         configurationNotDeleted => 'Configuration not deleted',
-        invalidVirtualHostName  => 'Invalid virtual host name',       
+        invalidVirtualHostName  => 'Invalid virtual host name',
+        checkLogs               => 'Check Apache logs',
     };
 }