Item12180: Item12241: Do better at hiding and saving Enable items. Cl…

…eanup pending changes properly after save. Update pending changes when returning to saved state. Don't log value of password fields during save. Remove undefined values from LSC. Don't bother fetching cert details unless logging. git-svn-id: http://svn.foswiki.org/trunk@16322 0b4bb1d4-4e5a-0410-9cc4-b2b747904278
foswiki · Jan 1, 2013 · 18b29ac · 18b29ac
1 parent 049b1df
commit 18b29ac
Show file tree

Hide file tree

Showing 7 changed files with 88 additions and 61 deletions.
diff --git a/core/lib/Foswiki.spec b/core/lib/Foswiki.spec
@@ -749,7 +749,7 @@ $Foswiki::cfg{Register}{UniqueEmail} = $FALSE;
 # <code>@(?!(example\.com|example\.net)$)</code>
 $Foswiki::cfg{Register}{EmailFilter} = '';
 
-# **STRING H**
+# **PASSWORD H**
 # Configuration password (not prompted)
 $Foswiki::cfg{Password} = '';
 

diff --git a/core/lib/Foswiki/Configure/Checkers/ConfigureGUI/Modals/SaveChanges.pm b/core/lib/Foswiki/Configure/Checkers/ConfigureGUI/Modals/SaveChanges.pm
@@ -5,7 +5,7 @@ use warnings;
 
 package Foswiki::Configure::Checkers::ConfigureGUI::Modals::SaveChanges;
 
-use Foswiki::Configure(qw(:auth :config :util));
+use Foswiki::Configure(qw(:auth :config :util $changesDiscarded));
 
 # Modal action checker for Save Changes.
 
@@ -23,14 +23,16 @@ sub generateForm {
     $template->renderButton;
     $template->renderFeedbackWindow;
 
-    my $updated  = $this->{item}{_fbChanged};
-    my $modified = keys %$updated;
+    my $updated = { %{ $this->{item}{_fbChanged} } };
 
     my $cart = Foswiki::Configure::Feedback::Cart->get($session);
 
     my $changesList = [];
     foreach my $key ( sortHashkeyList( keys %$updated ) ) {
-        next if ( $key =~ /^\{ConfigureGUI\}/ );
+        if ( $key =~ /^\{ConfigureGUI\}/ ) {
+            delete $updated->{$key};
+            next;
+        }
         my $valueString;
         my $type = $query->param("TYPEOF:$key") || 'UNKNOWN';
         if ( $key =~ /password/i ) {
@@ -44,6 +46,7 @@ sub generateForm {
         }
         push( @$changesList, { key => $key, value => $valueString } );
     }
+    my $modified = @$changesList;
     my @items;
     @items = sortHashkeyList( keys %$updated ) if $modified;
 
@@ -119,8 +122,7 @@ sub processForm {
 
     require Foswiki::Configure::Feedback::Cart;
 
-    my $updated  = $this->{item}{_fbChanged};
-    my $modified = keys %$updated;
+    my $updated = { %{ $this->{item}{_fbChanged} } };
 
     my $cart = Foswiki::Configure::Feedback::Cart->get($session);
 
@@ -140,6 +142,8 @@ sub processForm {
 
     undef $ui;
 
+    $changesDiscarded = -1;
+
     my $passChanged = ( defined $cart->param('{Password}') ) ? 1 : 0;
 
     Foswiki::Configure::Feedback::Cart->empty($session);
@@ -152,7 +156,10 @@ sub processForm {
 
     my $changesList = [];
     foreach my $key ( sortHashkeyList( keys %$updated ) ) {
-        next if ( $key =~ /^\{ConfigureGUI\}/ );
+        if ( $key =~ /^\{ConfigureGUI\}/ ) {
+            delete $updated->{$key};
+            next;
+        }
         my $valueString;
         my $type = $query->param("TYPEOF:$key") || 'UNKNOWN';
         if ( $key =~ /password/i ) {
@@ -166,6 +173,8 @@ sub processForm {
         }
         push( @$changesList, { key => $key, value => $valueString } );
     }
+    my $modified = @$changesList;
+
     push @$changesList, { key => 'No configuration items changed', value => '' }
       unless (@$changesList);
 

diff --git a/core/lib/Foswiki/Configure/Feedback.pm b/core/lib/Foswiki/Configure/Feedback.pm
@@ -323,6 +323,8 @@ sub deliver {
 
             return 1 unless ( $visitee->isa('Foswiki::Configure::Value') );
 
+            delete $visitee->{_fbChanged};
+
             my $keys = $visitee->getKeys();
             return 1 if ( $keys =~ /^\{ConfigureGUI}/ );
 
@@ -336,24 +338,27 @@ sub deliver {
                 )
               );
 
-            delete $visitee->{_fbChanged};
             return 1;
         }
         bless( $this, __PACKAGE__ );
     }
 
-    if ($changesDiscarded) {
+    if ( $changesDiscarded == 1 ) {
         %Foswiki::cfg = ( %{ _copy( $this->{oldCfg} ) } );
     }
     else {
-        $this->{valuer} =
-          Foswiki::Configure::Valuer->new( $this->{oldCfg}, \%Foswiki::cfg );
+        unless ( $changesDiscarded == -1 ) {    # unless saved
+            $this->{valuer} =                   # Find changes
+              Foswiki::Configure::Valuer->new( $this->{oldCfg},
+                \%Foswiki::cfg );
 
-        $root->visit($this);
+            $root->visit($this);
+        }
 
         $cart->removeParams($query);
 
-        $updated{$_} = 1 foreach $cart->param();
+        $updated{$_} = 1 foreach ( $cart->param() );
+
         checkpointChanges( $session, $query, \%updated );
     }
 
@@ -416,8 +421,6 @@ sub _copy {
 sub checkpointChanges {
     my ( $session, $query, $updated ) = @_;
 
-    return unless ( keys %$updated );
-
     require Foswiki::Configure::Feedback::Cart;
 
     my $cart = Foswiki::Configure::Feedback::Cart->get($session);

diff --git a/core/lib/Foswiki/Configure/Feedback/Cart.pm b/core/lib/Foswiki/Configure/Feedback/Cart.pm
@@ -22,7 +22,7 @@ sub get {
     my ($session) = @_;
 
     my $cart = $session->param('pending');
-    my @r = $cart = $class->verify($cart);
+    my @r    = $class->verify($cart);
 
     return wantarray ? @r : $r[0];
 }

diff --git a/core/lib/Foswiki/Configure/FoswikiCfg.pm b/core/lib/Foswiki/Configure/FoswikiCfg.pm
@@ -727,6 +727,8 @@ HERE
 sub _save {
     my $this = shift;
 
+    %dupItem = ();
+
     $this->{content} =~ s/\s*1;\s*$/\n/sg;
 
     # Sort the resulting data by hash key.  Attaches any comments to the
@@ -779,39 +781,47 @@ s/\A(.*?^\s*?\$(?:Foswiki::)?cfg($configItemRegex)\s*=.*?;\n)/push @content, $2;
 }
 
 # Visitor method called by node traversal during save. Incrementally modify
-# values, unless a value is reverting to the default in which case remove it.
+# values, unless a value is reverting to undefined, in which case remove it.
 sub startVisit {
     my ( $this, $visitee ) = @_;
 
-    if ( $visitee->isa('Foswiki::Configure::Value') ) {
-        my $keys = $visitee->getKeys();
-        return 1
-          if ( $keys =~ /^\{ConfigureGUI\}/
-            || $visitee->getTypeName() eq 'NULL' );
-        my $warble = $this->{valuer}->currentValue($visitee);
-        return 1 unless defined $warble;
-
-        if ( $this->{logger} ) {
-            my $logValue = $visitee->asString( $this->{valuer} );
-            $this->{logger}->logChange( $visitee->getKeys(), $logValue );
-        }
+    return 1 unless ( $visitee->isa('Foswiki::Configure::Value') );
+
+    my $keys     = $visitee->getKeys();
+    my $typeName = $visitee->getTypeName();
+
+    return 1
+      if ( $keys =~ /^\{ConfigureGUI\}/
+        || $typeName eq 'NULL' );
+
+    my $value = $this->{valuer}->currentValue($visitee);
+
+    if ( $this->{logger} ) {
+        my $logValue =
+            $typeName eq 'PASSWORD' ? '*' x 15
+          : defined $value ? $visitee->asString( $this->{valuer} )
+          :                  '<--undefined-->';
+        $this->{logger}->logChange( $visitee->getKeys(), $logValue );
+    }
+
+    if ( defined $value ) {
 
         # For some reason Data::Dumper ignores the second parameter sometimes
         # when -T is enabled, so have to do a substitution
-        my $txt = Data::Dumper->Dump( [$warble] );
+        my $txt = Data::Dumper->Dump( [$value] );
         $txt =~ s/VAR1/Foswiki::cfg$keys/;
 
- # Substitute any existing value, or append if not there
- #unless ( $this->{content} =~ s/^\s*?\$(Foswiki::)?cfg$keys\s*=.*?;\n/$txt/ms )
- #
- # SMELL:  The _updateEntry call is needed to fix up configs broken by Item9699.
-        unless ( $this->{content} =~
-s/^\s*?\$(?:Foswiki::)?cfg($keys)\s*=.*?;\n/&_updateEntry($1,$txt)/msge
-          )
-        {
-            $this->{content} .= $txt;
-        }
+        # Substitute any existing value, or append if not there
+
+        $this->{content} .= $txt
+          unless ( $this->{content} =~
+s/^\s*\$(?:Foswiki::)?cfg$keys\s*=.*?;\n/_updateEntry($keys,$txt)/msge
+          );
     }
+    else {
+        $this->{content} =~ s/^\s*?\$(?:Foswiki::)?cfg$keys\s*=.*?;\n//msg;
+    }
+
     return 1;
 }
 

diff --git a/core/lib/Foswiki/Configure/Valuer.pm b/core/lib/Foswiki/Configure/Valuer.pm
@@ -109,7 +109,7 @@ sub loadCGIParams {
         next unless $param =~ /^TYPEOF:($configItemRegex)/;    #
 
         my $keys = $1;
-        next if ( $keys =~ /^\{ConfigureGUI\}/ );
+        next if ( $keys =~ /^\{ConfigureGUI\}/ || $keys =~ /_\}$/ );
 
         # The value of TYPEOF: is the type name
         my $typename = $query->param($param);

diff --git a/core/lib/Foswiki/Net.pm b/core/lib/Foswiki/Net.pm
@@ -1523,37 +1523,33 @@ sub sslVerifyCert {
     # A server must have a certificate, so this shouldn't happen.
 
     unless ( $ctx && $peerCert ) {
-        $log->debug_print( 0, "Verify:   No certificate was supplied" );
+        $log->debug_print( 0, "Verify:   No certificate was supplied" )
+          if ($logcx);
         return 0;
     }
 
-    # Get certificate at current level of chain
+    # Process certificate at current level of chain
     # Note: The chain is built from the server up to the root,
     #       then verified from the root down to the server.
     #       Depth increases from 0 (the server) to n (the root)
 
-    my $cert  = Net::SSLeay::X509_STORE_CTX_get_current_cert($ctx);
-    my $error = Net::SSLeay::X509_STORE_CTX_get_error($ctx);
     my $depth = Net::SSLeay::X509_STORE_CTX_get_error_depth($ctx);
-
-    my $issuerName =
-      Net::SSLeay::X509_NAME_oneline(
-        Net::SSLeay::X509_get_issuer_name($cert) );
-    my $subjectName =
-      Net::SSLeay::X509_NAME_oneline(
-        Net::SSLeay::X509_get_subject_name($cert) );
-
-    if ( $depth > 20 ) {
-        $error = 22;    #X509_V_ERR_CERT_CHAIN_TOO_LONG
-        Net::SSLeay::X509_STORE_CTX_set_error( $ctx, $error );
+    if ( $depth > 20 ) {    #X509_V_ERR_CERT_CHAIN_TOO_LONG
+        Net::SSLeay::X509_STORE_CTX_set_error( $ctx, 22 );
         $ok = 0;
     }
     if ($ok) {
         $verified = 1 if ( $verified < 0 );
-        $log->debug_print( 0,
-            "Verified: " . fmtcertnames( "$subjectName\n", 'Verified: ', -4 ) )
-          if ($logcx);
-
+        if ($logcx) {
+            my $cert = Net::SSLeay::X509_STORE_CTX_get_current_cert($ctx);
+            my $subjectName =
+              Net::SSLeay::X509_NAME_oneline(
+                Net::SSLeay::X509_get_subject_name($cert) );
+
+            $log->debug_print( 0,
+                "Verified: "
+                  . fmtcertnames( "$subjectName\n", 'Verified: ', -4 ) );
+        }
         if ( $depth == 0 ) {
             my $host = {@sockopts}->{SSL_verifycn_name};
 
@@ -1618,6 +1614,15 @@ sub sslVerifyCert {
     else {
         $verified = 0;
         if ($logcx) {
+            my $error = Net::SSLeay::X509_STORE_CTX_get_error($ctx);
+            my $cert  = Net::SSLeay::X509_STORE_CTX_get_current_cert($ctx);
+            my $subjectName =
+              Net::SSLeay::X509_NAME_oneline(
+                Net::SSLeay::X509_get_subject_name($cert) );
+            my $issuerName =
+              Net::SSLeay::X509_NAME_oneline(
+                Net::SSLeay::X509_get_issuer_name($cert) );
+
             my $msg =
                 "Verify:   "
               . Net::SSLeay::X509_verify_cert_error_string($error) . "\n"