[Rt-commit] rt 02/02: Use JSON format for complicated configuration contents when possible

sunnavy sunnavy at bestpractical.com
Wed Jul 21 19:12:00 UTC 2021


This is an automated email from the git hooks/post-receive script.

sunnavy pushed a commit to branch 5.0/configuration-json-format
in repository rt.

commit b066f4c701b301c0b4fceef8ab76a613b971f766
Author: sunnavy <sunnavy at bestpractical.com>
AuthorDate: Wed Jul 21 06:24:20 2021 +0800

    Use JSON format for complicated configuration contents when possible
    
    Format "perl" will still be used for cases where JSON doesn't support
    like regex.
    
    The main reason to prefer JSON here is to handle unicode more robustly.
    E.g. perl treats the following strings differently in a subtle way:
    
        # Internal UTF-8 flag is off.
        $a = "\x{c7}"; # Ç
    
        # Internal UTF-8 flag is on.
        use utf8;
        $a = 'Ç';
    
        # Internal UTF-8 flag is on.
        use Encode;
        $a = decode('UTF-8', encode('UTF-8', "\x{c7}"));
    
    Thus contents containing unicode data could confuse perl sometimes and
    cause RT to decode data incorrectly.
---
 etc/upgrade/upgrade-configurations.in |  5 ++--
 lib/RT/Configuration.pm               | 48 +++++++++++++++++++++++++----------
 t/web/admin_tools_editconfig.t        |  7 +----
 3 files changed, 39 insertions(+), 21 deletions(-)

diff --git a/etc/upgrade/upgrade-configurations.in b/etc/upgrade/upgrade-configurations.in
index 9d85d66a9c..9782576144 100644
--- a/etc/upgrade/upgrade-configurations.in
+++ b/etc/upgrade/upgrade-configurations.in
@@ -116,9 +116,10 @@ while ( my $config = $configs->Next ) {
     }
     else {
         $RT::Handle->BeginTransaction();
-        my ( $ret, $msg ) = $config->__Set( Field => 'ContentType', Value => 'perl' );
+        my ( $content, $content_type ) = $config->EncodeContent($thawed);
+        my ( $ret,     $msg )          = $config->__Set( Field => 'ContentType', Value => $content_type );
         if ($ret) {
-            ( $ret, $msg ) = $config->__Set( Field => 'Content', Value => $config->_SerializeContent($thawed) );
+            ( $ret, $msg ) = $config->__Set( Field => 'Content', Value => $content );
         }
 
         if ($ret) {
diff --git a/lib/RT/Configuration.pm b/lib/RT/Configuration.pm
index 26bbb0d85c..422a5e5da9 100644
--- a/lib/RT/Configuration.pm
+++ b/lib/RT/Configuration.pm
@@ -141,10 +141,7 @@ sub Create {
     $RT::Handle->Commit;
     RT->Config->ApplyConfigChangeToAllServerProcesses;
 
-    my $old_value = RT->Config->Get($args{Name});
-    if ( ref $old_value ) {
-        $old_value = $self->_SerializeContent($old_value);
-    }
+    my ($old_value) = $self->EncodeContent( scalar RT->Config->Get( $args{Name} ) );
     RT->Logger->info($self->CurrentUser->Name . " changed " . $args{Name});
     return ( $id, $self->loc( '[_1] changed from "[_2]" to "[_3]"', $self->Name, $old_value // '', $content // '' ) );
 }
@@ -292,11 +289,7 @@ sub SetContent {
     my ( $ok, $msg ) = $self->ValidateContent( Content => $raw_value );
     return ( 0, $msg ) unless $ok;
 
-    my $value = $raw_value;
-    if (ref $value) {
-        $value = $self->_SerializeContent($value, $self->Name);
-        $content_type = 'perl';
-    }
+    ( my $value, $content_type ) = $self->EncodeContent($raw_value);
     if ($self->Content eq $value) {
         return (0, $self->loc("[_1] update: Nothing changed", ucfirst($self->Name)));
     }
@@ -380,6 +373,25 @@ sub ValidateContent {
     return ( 1, $self->loc('Content valid') );
 }
 
+=head2 EncodeContent
+
+Returns a pair of encoded content and content type.
+
+=cut
+
+sub EncodeContent {
+    my $self    = shift;
+    my $content = shift;
+    return ( $content, '' ) unless ref $content;
+
+    if ( my $json = $self->_EnJSONContent($content) ) {
+        return ( $json, 'application/json' );
+    }
+    else {
+        return ( $self->_SerializeContent($content), 'perl' );
+    }
+}
+
 =head1 PRIVATE METHODS
 
 Documented for internal use only, do not call these from outside
@@ -406,10 +418,7 @@ sub _Create {
         return ( 0, $self->loc("You cannot update [_1] using database config; you must edit your site config", $args{'Name'}) );
     }
 
-    if ( ref( $args{'Content'} ) ) {
-        $args{'Content'} = $self->_SerializeContent( $args{'Content'}, $args{'Name'} );
-        $args{'ContentType'} = 'perl';
-    }
+    ( $args{'Content'}, $args{'ContentType'} ) = $self->EncodeContent( $args{'Content'} );
 
     my ( $id, $msg ) = $self->SUPER::Create(
         map { $_ => $args{$_} } qw(Name Content ContentType),
@@ -481,6 +490,19 @@ sub _DeserializeContent {
     return $thawed;
 }
 
+sub _EnJSONContent {
+    my $self    = shift;
+    my $content = shift;
+
+    my $json = eval { JSON::to_json( $content, { pretty => 1, canonical => 1 } ) };
+    if ($@) {
+        return wantarray ? ( undef, $@ ) : undef;
+    }
+
+    chomp $json;
+    return $json;
+}
+
 sub _DeJSONContent {
     my $self = shift;
     my $content = shift;
diff --git a/t/web/admin_tools_editconfig.t b/t/web/admin_tools_editconfig.t
index 0b3addbd71..d5353e3a67 100644
--- a/t/web/admin_tools_editconfig.t
+++ b/t/web/admin_tools_editconfig.t
@@ -140,12 +140,7 @@ sub stringify {
     my $value = shift;
 
     return $value unless ref $value;
-
-    local $Data::Dumper::Terse = 1;
-    local $Data::Dumper::Indent = 2;
-    local $Data::Dumper::Sortkeys = 1;
-
-    my $output = Data::Dumper::Dumper $value;
+    my $output = JSON::to_json( $value, { pretty => 1, canonical => 1 } );
     chomp $output;
     return $output;
 }

-- 
To stop receiving notification emails like this one, please contact
sysadmin at bestpractical.com.


More information about the rt-commit mailing list