You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by GitBox <gi...@apache.org> on 2020/01/10 05:52:39 UTC

[GitHub] [incubator-shardingsphere] minghu-zhang opened a new pull request #3925: Itrus bug fix

minghu-zhang opened a new pull request #3925: Itrus bug fix
URL: https://github.com/apache/incubator-shardingsphere/pull/3925
 
 
   Fixes #3924.
   
   Changes proposed in this pull request:
   - org.apache.shardingsphere.encrypt.strategy.impl.AESEncryptor
   - org.apache.shardingsphere.encrypt.strategy.impl.MD5Encryptor
   - org.apache.shardingsphere.underlying.common.util.StringUtil
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] SteNicholas commented on a change in pull request #3925: Solve the bug that the encryption result exception value is 'null' string when the encryption field value is null

Posted by GitBox <gi...@apache.org>.
SteNicholas commented on a change in pull request #3925: Solve the bug that the encryption result exception value is 'null' string when the encryption field value is null
URL: https://github.com/apache/incubator-shardingsphere/pull/3925#discussion_r365297216
 
 

 ##########
 File path: encrypt-core/encrypt-core-common/src/main/java/org/apache/shardingsphere/encrypt/strategy/impl/AESEncryptor.java
 ##########
 @@ -43,44 +44,47 @@
 @Getter
 @Setter
 public final class AESEncryptor implements Encryptor {
-    
+
     private static final String AES_KEY = "aes.key.value";
-    
+
     private Properties properties = new Properties();
-    
+
     @Override
     public String getType() {
         return "AES";
     }
-    
+
     @Override
     public void init() {
     }
-    
+
 
 Review comment:
   This should recover to origin style. You should check style of pull request.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] minghu-zhang commented on a change in pull request #3925: Solve the bug that the encryption result exception value is 'null' string when the encryption field value is null

Posted by GitBox <gi...@apache.org>.
minghu-zhang commented on a change in pull request #3925: Solve the bug that the encryption result exception value is 'null' string when the encryption field value is null
URL: https://github.com/apache/incubator-shardingsphere/pull/3925#discussion_r365629462
 
 

 ##########
 File path: encrypt-core/encrypt-core-common/src/main/java/org/apache/shardingsphere/encrypt/strategy/impl/AESEncryptor.java
 ##########
 @@ -43,44 +44,47 @@
 @Getter
 @Setter
 public final class AESEncryptor implements Encryptor {
-    
+
     private static final String AES_KEY = "aes.key.value";
-    
+
     private Properties properties = new Properties();
-    
+
     @Override
     public String getType() {
         return "AES";
     }
-    
+
     @Override
     public void init() {
     }
-    
+
     @Override
     @SneakyThrows
     public String encrypt(final Object plaintext) {
+        if (StringUtil.isNullOrEmpty(plaintext)) {
 
 Review comment:
   if value is null, the encryption method will get a 'null' string result.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] minghu-zhang commented on a change in pull request #3925: Solve the bug that the encryption result exception value is 'null' string when the encryption field value is null

Posted by GitBox <gi...@apache.org>.
minghu-zhang commented on a change in pull request #3925: Solve the bug that the encryption result exception value is 'null' string when the encryption field value is null
URL: https://github.com/apache/incubator-shardingsphere/pull/3925#discussion_r365630210
 
 

 ##########
 File path: shardingsphere-underlying/shardingsphere-common/src/main/java/org/apache/shardingsphere/underlying/common/util/StringUtil.java
 ##########
 @@ -52,4 +52,14 @@ public static boolean isIntValue(final String value) {
             return false;
         }
     }
+
+    /**
+     * Judge is Null value or empty.
+     *
+     * @param value to be judged string value
+     * @return is long value or not
+     */
+    public static boolean isNullOrEmpty(final Object value) {
+        return value == null || "".equals(value.toString());
+    }
 
 Review comment:
   > And according to style guide, null should be before ==
   
   Sorry, I refer to the Objects. isNull() method.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] minghu-zhang commented on issue #3925: Solve the bug that the encryption result exception value is 'null' string when the encryption field value is null

Posted by GitBox <gi...@apache.org>.
minghu-zhang commented on issue #3925: Solve the bug that the encryption result exception value is 'null' string when the encryption field value is null
URL: https://github.com/apache/incubator-shardingsphere/pull/3925#issuecomment-573484029
 
 
   > @minghu-zhang Hi minghui, thanks for your pr.
   > 
   > I found you added the judgement of `StringUtil.isNullOrEmpty(plaintext)` in `encrypt()` and `decrypt()` for `AESEncryptor` and `encrypt()` for `MD5Encryptor`, i am curious what will happen if the parameter for those functions is null or empty string?
   
   if value is null, the encryption method will get a 'null' string result.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] coveralls commented on issue #3925: Solve the bug that the encryption result exception value is 'null' string when the encryption field value is null

Posted by GitBox <gi...@apache.org>.
coveralls commented on issue #3925: Solve the bug that the encryption result exception value is 'null' string when the encryption field value is null
URL: https://github.com/apache/incubator-shardingsphere/pull/3925#issuecomment-573011303
 
 
   ## Pull Request Test Coverage Report for [Build 1335](https://coveralls.io/builds/28027441)
   
   * **4** of **7**   **(57.14%)**  changed or added relevant lines in **3** files are covered.
   * **47** unchanged lines in **2** files lost coverage.
   * Overall coverage increased (+**0.3%**) to **66.099%**
   
   ---
   
   |  Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
   | :-----|--------------|--------|---: |
   | [encrypt-core/encrypt-core-common/src/main/java/org/apache/shardingsphere/encrypt/strategy/impl/AESEncryptor.java](https://coveralls.io/builds/28027441/source?filename=encrypt-core%2Fencrypt-core-common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fencrypt%2Fstrategy%2Fimpl%2FAESEncryptor.java#L65) | 3 | 4 | 75.0%
   | [encrypt-core/encrypt-core-common/src/main/java/org/apache/shardingsphere/encrypt/strategy/impl/MD5Encryptor.java](https://coveralls.io/builds/28027441/source?filename=encrypt-core%2Fencrypt-core-common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fencrypt%2Fstrategy%2Fimpl%2FMD5Encryptor.java#L51) | 1 | 2 | 50.0%
   | [shardingsphere-underlying/shardingsphere-common/src/main/java/org/apache/shardingsphere/underlying/common/util/StringUtil.java](https://coveralls.io/builds/28027441/source?filename=shardingsphere-underlying%2Fshardingsphere-common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Funderlying%2Fcommon%2Futil%2FStringUtil.java#L63) | 0 | 1 | 0.0%
   <!-- | **Total:** | **4** | **7** | **57.14%** | -->
   
   |  Files with Coverage Reduction | New Missed Lines | % |
   | :-----|--------------|--: |
   | [sharding-jdbc/sharding-jdbc-core/src/main/java/org/apache/shardingsphere/shardingjdbc/jdbc/core/statement/ShadowStatement.java](https://coveralls.io/builds/28027441/source?filename=sharding-jdbc%2Fsharding-jdbc-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fshardingjdbc%2Fjdbc%2Fcore%2Fstatement%2FShadowStatement.java#L89) | 17 | 71.23% |
   | [sharding-jdbc/sharding-jdbc-core/src/main/java/org/apache/shardingsphere/shardingjdbc/jdbc/core/connection/ShadowConnection.java](https://coveralls.io/builds/28027441/source?filename=sharding-jdbc%2Fsharding-jdbc-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fshardingjdbc%2Fjdbc%2Fcore%2Fconnection%2FShadowConnection.java#L69) | 30 | 7.69% |
   <!-- | **Total:** | **47** |  | -->
   
   |  Totals | [![Coverage Status](https://coveralls.io/builds/28027441/badge)](https://coveralls.io/builds/28027441) |
   | :-- | --: |
   | Change from base [Build 666](https://coveralls.io/builds/28020827): |  0.3% |
   | Covered Lines: | 10821 |
   | Relevant Lines: | 16371 |
   
   ---
   ##### 💛  - [Coveralls](https://coveralls.io)
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] minghu-zhang closed pull request #3925: Solve the bug that the encryption result exception value is 'null' string when the encryption field value is null

Posted by GitBox <gi...@apache.org>.
minghu-zhang closed pull request #3925: Solve the bug that the encryption result exception value is 'null' string when the encryption field value is null
URL: https://github.com/apache/incubator-shardingsphere/pull/3925
 
 
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] terrymanu commented on a change in pull request #3925: Solve the bug that the encryption result exception value is 'null' string when the encryption field value is null

Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #3925: Solve the bug that the encryption result exception value is 'null' string when the encryption field value is null
URL: https://github.com/apache/incubator-shardingsphere/pull/3925#discussion_r365186175
 
 

 ##########
 File path: shardingsphere-underlying/shardingsphere-common/src/main/java/org/apache/shardingsphere/underlying/common/util/StringUtil.java
 ##########
 @@ -52,4 +52,14 @@ public static boolean isIntValue(final String value) {
             return false;
         }
     }
+
+    /**
+     * Judge is Null value or empty.
+     *
+     * @param value to be judged string value
+     * @return is long value or not
+     */
+    public static boolean isNullOrEmpty(final Object value) {
+        return value == null || "".equals(value.toString());
+    }
 
 Review comment:
   Guava already this method, please reuse that one

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] tristaZero commented on issue #3925: Solve the bug that the encryption result exception value is 'null' string when the encryption field value is null

Posted by GitBox <gi...@apache.org>.
tristaZero commented on issue #3925: Solve the bug that the encryption result exception value is 'null' string when the encryption field value is null
URL: https://github.com/apache/incubator-shardingsphere/pull/3925#issuecomment-572999809
 
 
   @minghu-zhang Hi minghui, thanks for your pr. 
   
   I found you added the judgement of `StringUtil.isNullOrEmpty(plaintext)` in `encrypt()` and `decrypt()` for `AESEncryptor` and `encrypt()` for `MD5Encryptor`, i am curious what will happen if the parameter for those functions is null or empty string?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] tristaZero edited a comment on issue #3925: Solve the bug that the encryption result exception value is 'null' string when the encryption field value is null

Posted by GitBox <gi...@apache.org>.
tristaZero edited a comment on issue #3925: Solve the bug that the encryption result exception value is 'null' string when the encryption field value is null
URL: https://github.com/apache/incubator-shardingsphere/pull/3925#issuecomment-575972627
 
 
   Hi @minghu-zhang,  i thought your PR could be merged into ShardingSphere after some little changes, why did not you close it?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] tristaZero commented on issue #3925: Solve the bug that the encryption result exception value is 'null' string when the encryption field value is null

Posted by GitBox <gi...@apache.org>.
tristaZero commented on issue #3925: Solve the bug that the encryption result exception value is 'null' string when the encryption field value is null
URL: https://github.com/apache/incubator-shardingsphere/pull/3925#issuecomment-575972627
 
 
   Hi @minghu-zhang,  i thought your PR could be merged into ShardingSphere after some optimization, why do not you closed it?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] minghu-zhang commented on issue #3925: Solve the bug that the encryption result exception value is 'null' string when the encryption field value is null

Posted by GitBox <gi...@apache.org>.
minghu-zhang commented on issue #3925: Solve the bug that the encryption result exception value is 'null' string when the encryption field value is null
URL: https://github.com/apache/incubator-shardingsphere/pull/3925#issuecomment-573487896
 
 
   > @minghu-zhang Hi minghui, thanks for your pr.
   > 
   > I found you added the judgement of `StringUtil.isNullOrEmpty(plaintext)` in `encrypt()` and `decrypt()` for `AESEncryptor` and `encrypt()` for `MD5Encryptor`, i am curious what will happen if the parameter for those functions is null or empty string?
   
   the method String.valueof() will get a 'null' string value when value is null. the encrypted result is not what we want when the value is empty string. 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] SteNicholas commented on a change in pull request #3925: Solve the bug that the encryption result exception value is 'null' string when the encryption field value is null

Posted by GitBox <gi...@apache.org>.
SteNicholas commented on a change in pull request #3925: Solve the bug that the encryption result exception value is 'null' string when the encryption field value is null
URL: https://github.com/apache/incubator-shardingsphere/pull/3925#discussion_r365297824
 
 

 ##########
 File path: shardingsphere-underlying/shardingsphere-common/src/main/java/org/apache/shardingsphere/underlying/common/util/StringUtil.java
 ##########
 @@ -52,4 +52,14 @@ public static boolean isIntValue(final String value) {
             return false;
         }
     }
+
+    /**
+     * Judge is Null value or empty.
+     *
+     * @param value to be judged string value
+     * @return is long value or not
+     */
+    public static boolean isNullOrEmpty(final Object value) {
+        return value == null || "".equals(value.toString());
+    }
 
 Review comment:
   And according to style guide, null should be before ==

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] terrymanu commented on a change in pull request #3925: Solve the bug that the encryption result exception value is 'null' string when the encryption field value is null

Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #3925: Solve the bug that the encryption result exception value is 'null' string when the encryption field value is null
URL: https://github.com/apache/incubator-shardingsphere/pull/3925#discussion_r365186307
 
 

 ##########
 File path: encrypt-core/encrypt-core-common/src/main/java/org/apache/shardingsphere/encrypt/strategy/impl/AESEncryptor.java
 ##########
 @@ -43,44 +44,47 @@
 @Getter
 @Setter
 public final class AESEncryptor implements Encryptor {
-    
+
 
 Review comment:
   Please keep the original indent

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] terrymanu commented on a change in pull request #3925: Solve the bug that the encryption result exception value is 'null' string when the encryption field value is null

Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #3925: Solve the bug that the encryption result exception value is 'null' string when the encryption field value is null
URL: https://github.com/apache/incubator-shardingsphere/pull/3925#discussion_r365185843
 
 

 ##########
 File path: encrypt-core/encrypt-core-common/src/main/java/org/apache/shardingsphere/encrypt/strategy/impl/AESEncryptor.java
 ##########
 @@ -43,44 +44,47 @@
 @Getter
 @Setter
 public final class AESEncryptor implements Encryptor {
-    
+
     private static final String AES_KEY = "aes.key.value";
-    
+
     private Properties properties = new Properties();
-    
+
     @Override
     public String getType() {
         return "AES";
     }
-    
+
     @Override
     public void init() {
     }
-    
+
     @Override
     @SneakyThrows
     public String encrypt(final Object plaintext) {
+        if (StringUtil.isNullOrEmpty(plaintext)) {
 
 Review comment:
   The empty string can be encrypted, is it? Only null value can not encrypt.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] tristaZero commented on issue #3925: Solve the bug that the encryption result exception value is 'null' string when the encryption field value is null

Posted by GitBox <gi...@apache.org>.
tristaZero commented on issue #3925: Solve the bug that the encryption result exception value is 'null' string when the encryption field value is null
URL: https://github.com/apache/incubator-shardingsphere/pull/3925#issuecomment-577495005
 
 
   This PR has been finished by #4068.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services