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/13 14:01:55 UTC
[GitHub] [incubator-shardingsphere] longjy opened a new pull request #3959:
Get ciphertext column as a string
longjy opened a new pull request #3959: Get ciphertext column as a string
URL: https://github.com/apache/incubator-shardingsphere/pull/3959
Encrypt the field, and take the value of string type directly. For example, after digital encryption, the database can be saved as varchar, and the corresponding field of bean can still be number
please see https://github.com/apache/incubator-shardingsphere/pull/3934
----------------------------------------------------------------
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 issue #3959: Get
ciphertext column as a string
Posted by GitBox <gi...@apache.org>.
terrymanu commented on issue #3959: Get ciphertext column as a string
URL: https://github.com/apache/incubator-shardingsphere/pull/3959#issuecomment-575556753
Hi,
> mysql is not a varchar field type
What is mean?
> getString will not have a type exception.
What is type exception? Did you mean cast exception?
> Can the user do a type conversion here and return the expected type
I can't get the point, is it a question sentence?
The question I want to know is what the objective of your pull request? Did you meet some problem?
----------------------------------------------------------------
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 #3959: Get ciphertext column as a string
Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #3959: Get ciphertext column as a string
URL: https://github.com/apache/incubator-shardingsphere/pull/3959#discussion_r368348351
##########
File path: encrypt-core/encrypt-core-merge/src/main/java/org/apache/shardingsphere/encrypt/merge/dql/EncryptMergedResult.java
##########
@@ -44,15 +44,15 @@
public boolean next() throws SQLException {
return mergedResult.next();
}
-
+
@Override
public Object getValue(final int columnIndex, final Class<?> type) throws SQLException {
- Object value = mergedResult.getValue(columnIndex, type);
- if (null == value || !queryWithCipherColumn) {
- return value;
+ Optional<Encryptor> encryptor;
+ if (!queryWithCipherColumn || !(encryptor = metaData.findEncryptor(columnIndex)).isPresent()) {
+ return mergedResult.getValue(columnIndex, type);
}
- Optional<Encryptor> encryptor = metaData.findEncryptor(columnIndex);
- return encryptor.isPresent() ? encryptor.get().decrypt(value.toString()) : value;
+ String ciphertext = (String) mergedResult.getValue(columnIndex, String.class);
+ return null == ciphertext ? null : encryptor.get().decrypt(ciphertext);
Review comment:
The logic below may better:
- judge queryWithCipherColumn or not
- if !queryWithCipherColumn, return `mergedResult.getValue(columnIndex, type)`
- if queryWithCipherColumn, get value via `mergedResult.getValue(columnIndex, String.class)`
- if value is null, return null
- if value is not null, findEncryptor and return decrypt value
----------------------------------------------------------------
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 #3959: Get ciphertext column as a string
Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #3959: Get ciphertext column as a string
URL: https://github.com/apache/incubator-shardingsphere/pull/3959#discussion_r368522986
##########
File path: encrypt-core/encrypt-core-merge/src/main/java/org/apache/shardingsphere/encrypt/merge/dql/EncryptMergedResult.java
##########
@@ -44,15 +44,15 @@
public boolean next() throws SQLException {
return mergedResult.next();
}
-
+
@Override
public Object getValue(final int columnIndex, final Class<?> type) throws SQLException {
- Object value = mergedResult.getValue(columnIndex, type);
- if (null == value || !queryWithCipherColumn) {
- return value;
+ Optional<Encryptor> encryptor;
+ if (!queryWithCipherColumn || !(encryptor = metaData.findEncryptor(columnIndex)).isPresent()) {
+ return mergedResult.getValue(columnIndex, type);
}
- Optional<Encryptor> encryptor = metaData.findEncryptor(columnIndex);
- return encryptor.isPresent() ? encryptor.get().decrypt(value.toString()) : value;
+ String ciphertext = (String) mergedResult.getValue(columnIndex, String.class);
+ return null == ciphertext ? null : encryptor.get().decrypt(ciphertext);
Review comment:
Yes, it is too complicated
----------------------------------------------------------------
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] longjy commented on issue #3959: Get
ciphertext column as a string
Posted by GitBox <gi...@apache.org>.
longjy commented on issue #3959: Get ciphertext column as a string
URL: https://github.com/apache/incubator-shardingsphere/pull/3959#issuecomment-574499466
After verification, mysql is not a varchar field type, getString will not have a type exception.
The Encryptor class decrypt method returns an Object type. Can the user do a type conversion here and return the expected type
----------------------------------------------------------------
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] longjy commented on a change in pull
request #3959: Get ciphertext column as a string
Posted by GitBox <gi...@apache.org>.
longjy commented on a change in pull request #3959: Get ciphertext column as a string
URL: https://github.com/apache/incubator-shardingsphere/pull/3959#discussion_r368802132
##########
File path: encrypt-core/encrypt-core-merge/src/main/java/org/apache/shardingsphere/encrypt/merge/dql/EncryptMergedResult.java
##########
@@ -44,15 +44,15 @@
public boolean next() throws SQLException {
return mergedResult.next();
}
-
+
@Override
public Object getValue(final int columnIndex, final Class<?> type) throws SQLException {
- Object value = mergedResult.getValue(columnIndex, type);
- if (null == value || !queryWithCipherColumn) {
- return value;
+ Optional<Encryptor> encryptor;
+ if (!queryWithCipherColumn || !(encryptor = metaData.findEncryptor(columnIndex)).isPresent()) {
+ return mergedResult.getValue(columnIndex, type);
}
- Optional<Encryptor> encryptor = metaData.findEncryptor(columnIndex);
- return encryptor.isPresent() ? encryptor.get().decrypt(value.toString()) : value;
+ String ciphertext = (String) mergedResult.getValue(columnIndex, String.class);
+ return null == ciphertext ? null : encryptor.get().decrypt(ciphertext);
Review comment:
Code style fixed
----------------------------------------------------------------
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 issue #3959: Get
ciphertext column as a string
Posted by GitBox <gi...@apache.org>.
terrymanu commented on issue #3959: Get ciphertext column as a string
URL: https://github.com/apache/incubator-shardingsphere/pull/3959#issuecomment-574468461
The code you modify
```java
(String) mergedResult.getValue(columnIndex, String.class)
```
It is still use getString from original ResultSet. Is it correct if your type of field is BigDecimal?
----------------------------------------------------------------
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 edited a comment on issue
#3959: Get ciphertext column as a string
Posted by GitBox <gi...@apache.org>.
terrymanu edited a comment on issue #3959: Get ciphertext column as a string
URL: https://github.com/apache/incubator-shardingsphere/pull/3959#issuecomment-574212590
Hi, I am still want ask the same question. I just copy my comment from #3934
I have a little confuse, why force get value from ResultSet by String type?
I can't get the point, what is the issue in original code?
----------------------------------------------------------------
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] longjy edited a comment on issue #3959:
Get ciphertext column as a string
Posted by GitBox <gi...@apache.org>.
longjy edited a comment on issue #3959: Get ciphertext column as a string
URL: https://github.com/apache/incubator-shardingsphere/pull/3959#issuecomment-574232873
> Hi, I am still want ask the same question. I just copy my comment from #3934
>
> I have a little confuse, why force get value from ResultSet by String type?
> I can't get the point, what is the issue in original code?
If the BigDecimal field is encrypted, the database will store string. In general, ORM will directly get BigDecimal according to the java bean field type. The original code may have a new BigDecimal ("ciphertext") situation and throw an exception
----------------------------------------------------------------
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 merged pull request #3959: Get
ciphertext column as a string
Posted by GitBox <gi...@apache.org>.
terrymanu merged pull request #3959: Get ciphertext column as a string
URL: https://github.com/apache/incubator-shardingsphere/pull/3959
----------------------------------------------------------------
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] longjy commented on a change in pull
request #3959: Get ciphertext column as a string
Posted by GitBox <gi...@apache.org>.
longjy commented on a change in pull request #3959: Get ciphertext column as a string
URL: https://github.com/apache/incubator-shardingsphere/pull/3959#discussion_r368492269
##########
File path: encrypt-core/encrypt-core-merge/src/main/java/org/apache/shardingsphere/encrypt/merge/dql/EncryptMergedResult.java
##########
@@ -44,15 +44,15 @@
public boolean next() throws SQLException {
return mergedResult.next();
}
-
+
@Override
public Object getValue(final int columnIndex, final Class<?> type) throws SQLException {
- Object value = mergedResult.getValue(columnIndex, type);
- if (null == value || !queryWithCipherColumn) {
- return value;
+ Optional<Encryptor> encryptor;
+ if (!queryWithCipherColumn || !(encryptor = metaData.findEncryptor(columnIndex)).isPresent()) {
+ return mergedResult.getValue(columnIndex, type);
}
- Optional<Encryptor> encryptor = metaData.findEncryptor(columnIndex);
- return encryptor.isPresent() ? encryptor.get().decrypt(value.toString()) : value;
+ String ciphertext = (String) mergedResult.getValue(columnIndex, String.class);
+ return null == ciphertext ? null : encryptor.get().decrypt(ciphertext);
Review comment:
The encryptor should also judge whether it is null. For example, the id(bigint) is unencrypted and should be executed
`mergedResult.getValue(columnIndex, type)`
Or you mean it's too complicated:
`if (!queryWithCipherColumn || !(encryptor = metaData.findEncryptor(columnIndex)).isPresent())`
----------------------------------------------------------------
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] longjy commented on issue #3959: Get
ciphertext column as a string
Posted by GitBox <gi...@apache.org>.
longjy commented on issue #3959: Get ciphertext column as a string
URL: https://github.com/apache/incubator-shardingsphere/pull/3959#issuecomment-575967925
table
name | type
---|---
id | bigint
amount | varchar
bean
```
class Table{
private Long id;
private Bigdecimal amount;
}
```
ShardingEncryptor
```
//plaintext is Bigdecimal type
@Override
public String encrypt(Object plaintext) {
return "test" + plaintext.toString();
}
//ciphertext is string
@Override
public Object decrypt(String ciphertext) {
return new BigDecimal(ciphertext.replace("test", ""));
}
```
As shown above,amount is encrypted field,sql is
```
select * from table;
```
The original code will be
```
mergedResult.getValue(2, Bigdecimal.class);
```
but db amount column is "testXXXX";
throw exception:
```
java.sql.SQLDataException: Cannot determine value type from string "testXXXXX"
```
If mergedResult.getValue(2, String.class) get ciphertext,then ShardingEncryptor decrypt to Bigdecimal. will avoid this problem.
If the table column is encrypted without affecting the type modification in the code, it will be easier for the user to access
----------------------------------------------------------------
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 issue #3959: Get
ciphertext column as a string
Posted by GitBox <gi...@apache.org>.
terrymanu commented on issue #3959: Get ciphertext column as a string
URL: https://github.com/apache/incubator-shardingsphere/pull/3959#issuecomment-576514620
Thank you very much
----------------------------------------------------------------
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] longjy commented on issue #3959: Get
ciphertext column as a string
Posted by GitBox <gi...@apache.org>.
longjy commented on issue #3959: Get ciphertext column as a string
URL: https://github.com/apache/incubator-shardingsphere/pull/3959#issuecomment-574232873
> Hi, I am still want ask the same question. I just copy my comment from #3934
>
> I have a little confuse, why force get value from ResultSet by String type?
> I can't get the point, what is the issue in original code?
If the BigDecimal field is encrypted, the database will store string. In general, ORM will directly get BigDecimal according to the java bean field type. The original code may have a new BigDecimal ("ciphertext") situation and throw an exception
----------------------------------------------------------------
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 edited a comment on issue
#3959: Get ciphertext column as a string
Posted by GitBox <gi...@apache.org>.
terrymanu edited a comment on issue #3959: Get ciphertext column as a string
URL: https://github.com/apache/incubator-shardingsphere/pull/3959#issuecomment-574468461
The code you modified
```java
(String) mergedResult.getValue(columnIndex, String.class)
```
It is still use getString from original ResultSet. Is it correct if your type of field is BigDecimal?
----------------------------------------------------------------
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 #3959: Get ciphertext column as a string
Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #3959: Get ciphertext column as a string
URL: https://github.com/apache/incubator-shardingsphere/pull/3959#discussion_r368348129
##########
File path: encrypt-core/encrypt-core-merge/src/main/java/org/apache/shardingsphere/encrypt/merge/dql/EncryptMergedResult.java
##########
@@ -44,15 +44,15 @@
public boolean next() throws SQLException {
return mergedResult.next();
}
-
+
Review comment:
Please keep origin 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 issue #3959: Get
ciphertext column as a string
Posted by GitBox <gi...@apache.org>.
terrymanu commented on issue #3959: Get ciphertext column as a string
URL: https://github.com/apache/incubator-shardingsphere/pull/3959#issuecomment-574212590
Hi, I am still want ask the question. I just copy my comment from #3934
I have a little confuse, why force get value from ResultSet by String type?
I can't get the point, what is the issue in original code?
----------------------------------------------------------------
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 issue #3959: Get
ciphertext column as a string
Posted by GitBox <gi...@apache.org>.
terrymanu commented on issue #3959: Get ciphertext column as a string
URL: https://github.com/apache/incubator-shardingsphere/pull/3959#issuecomment-576079900
Ok, I understand. Let's talk about code style for this PR.
I will use line comment to discuss with you.
----------------------------------------------------------------
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