jt2594838 commented on code in PR #213:
URL: https://github.com/apache/tsfile/pull/213#discussion_r1734082648
##########
java/tsfile/src/main/java/org/apache/tsfile/common/conf/TSFileConfig.java:
##########
@@ -140,14 +139,14 @@ public class TSFileConfig implements Serializable {
/** Data compression method, TsFile supports UNCOMPRESSED, SNAPPY, ZSTD or
LZ4. */
private CompressionType compressor = CompressionType.LZ4;
- /** encryptFlag, this should be false by default */
- private String encryptFlag = "false";
+ /** encryptFlag, true means opening the encrypt function. */
+ private boolean encryptFlag = false;
- /** encryptKey, this should be null by default */
+ /** encryptKey, this should be 16 bytes long String. */
Review Comment:
remove "long".
##########
java/tsfile/src/main/java/org/apache/tsfile/encrypt/IDecryptor.java:
##########
@@ -39,6 +42,8 @@
/** encrypt data according to tsfileconfig. */
public interface IDecryptor extends Serializable {
+ Logger logger = LoggerFactory.getLogger(IDecryptor.class);
Review Comment:
loggers should be static final members in general.
##########
java/tsfile/src/main/java/org/apache/tsfile/common/conf/TSFileConfig.java:
##########
@@ -229,19 +228,19 @@ public TSFileConfig() {
}
public boolean getEncryptFlag() {
- return Objects.equals(this.encryptFlag, "true");
+ return encryptFlag;
}
public void setEncryptFlag(String encryptFlag) {
- this.encryptFlag = encryptFlag;
+ this.encryptFlag = Objects.equals(encryptFlag, "true");
Review Comment:
Boolean.parseBoolean may be more generic.
##########
java/tsfile/src/main/java/org/apache/tsfile/encrypt/IDecryptor.java:
##########
@@ -128,7 +134,7 @@ class AES128Decryptor implements IDecryptor {
| NoSuchPaddingException
| NoSuchAlgorithmException
| InvalidKeyException e) {
- throw new EncryptException("AES128Decryptor init failed");
+ throw new EncryptException("AES128Decryptor init failed " +
e.getMessage());
Review Comment:
Not good enough. Exceptions can take another exception as their cause during
construction like this.
In this way, the stack traces of the cause can be preserved, which may be
very helpful during debugging.
##########
java/tsfile/src/main/java/org/apache/tsfile/read/reader/chunk/ChunkReader.java:
##########
@@ -167,6 +168,33 @@ public static ByteBuffer readCompressedPageData(PageHeader
pageHeader, ByteBuffe
}
public static ByteBuffer uncompressPageData(
+ PageHeader pageHeader, IUnCompressor unCompressor, ByteBuffer
compressedPageData)
+ throws IOException {
+ int compressedPageBodyLength = pageHeader.getCompressedSize();
+ byte[] uncompressedPageData = new byte[pageHeader.getUncompressedSize()];
+ byte[] decryptedPageData = new byte[pageHeader.getCompressedSize()];
+ try {
+ unCompressor.uncompress(
+ compressedPageData.array(),
+ compressedPageData.arrayOffset() + compressedPageData.position(),
+ compressedPageBodyLength,
+ uncompressedPageData,
+ 0);
+ } catch (Exception e) {
+ throw new IOException(
+ "Uncompress error! uncompress size: "
+ + pageHeader.getUncompressedSize()
+ + "compressed size: "
+ + pageHeader.getCompressedSize()
+ + "page header: "
+ + pageHeader
+ + e.getMessage());
+ }
+ compressedPageData.position(compressedPageData.position() +
compressedPageBodyLength);
+ return ByteBuffer.wrap(uncompressedPageData);
+ }
Review Comment:
What is `decryptedPageData` for here?
##########
java/tsfile/src/main/java/org/apache/tsfile/common/conf/TSFileConfig.java:
##########
@@ -136,6 +140,15 @@ public class TSFileConfig implements Serializable {
/** Data compression method, TsFile supports UNCOMPRESSED, SNAPPY, ZSTD or
LZ4. */
private CompressionType compressor = CompressionType.LZ4;
+ /** encryptFlag, this should be false by default */
+ private String encryptFlag = "false";
+
+ /** encryptKey, this should be null by default */
+ private String encryptKey = "abcdefghijklmnop";
+
+ /** encryptType, this should be null by defalut */
+ private String encryptType = "UNENCRYPTED";
+
Review Comment:
I am fine with both, so you may leave it.
##########
java/tsfile/src/main/java/org/apache/tsfile/read/TsFileSequenceReader.java:
##########
@@ -1774,14 +1820,45 @@ public ByteBuffer readCompressedPage(PageHeader header)
throws IOException {
public ByteBuffer readPage(PageHeader header, CompressionType type) throws
IOException {
ByteBuffer buffer = readData(-1, header.getCompressedSize());
- if (header.getUncompressedSize() == 0 || type ==
CompressionType.UNCOMPRESSED) {
+ IDecryptor decryptor = getDecryptor();
+ if (header.getUncompressedSize() == 0) {
return buffer;
- } // FIXME if the buffer is not array-implemented.
- IUnCompressor unCompressor = IUnCompressor.getUnCompressor(type);
- ByteBuffer uncompressedBuffer =
ByteBuffer.allocate(header.getUncompressedSize());
- unCompressor.uncompress(
- buffer.array(), buffer.position(), buffer.remaining(),
uncompressedBuffer.array(), 0);
- return uncompressedBuffer;
+ }
+ if (decryptor == null || decryptor.getEncryptionType() ==
EncryptionType.UNENCRYPTED) {
+ if (type == CompressionType.UNCOMPRESSED) {
+ return buffer;
+ } else {
+ IUnCompressor unCompressor = IUnCompressor.getUnCompressor(type);
+ ByteBuffer uncompressedBuffer =
ByteBuffer.allocate(header.getUncompressedSize());
+ unCompressor.uncompress(
+ buffer.array(), buffer.position(), buffer.remaining(),
uncompressedBuffer.array(), 0);
+ return uncompressedBuffer;
+ }
+ } else {
+ if (type == CompressionType.UNCOMPRESSED) {
+ ByteBuffer decryptedBuffer =
ByteBuffer.allocate(header.getUncompressedSize());
+ System.arraycopy(
+ decryptor.decrypt(buffer.array(), buffer.position(),
buffer.remaining()),
+ 0,
+ decryptedBuffer.array(),
+ 0,
+ buffer.remaining());
+ return decryptedBuffer;
+ } else {
+ ByteBuffer decryptedBuffer =
ByteBuffer.allocate(header.getCompressedSize());
+ System.arraycopy(
+ decryptor.decrypt(buffer.array(), buffer.position(),
buffer.remaining()),
+ 0,
+ decryptedBuffer.array(),
+ 0,
+ buffer.remaining());
+ IUnCompressor unCompressor = IUnCompressor.getUnCompressor(type);
+ ByteBuffer uncompressedBuffer =
ByteBuffer.allocate(header.getUncompressedSize());
+ unCompressor.uncompress(
+ decryptedBuffer.array(), 0, buffer.remaining(),
uncompressedBuffer.array(), 0);
+ return uncompressedBuffer;
+ }
+ }
Review Comment:
Yes, you may still avoid copying data in this way.
If not encrypted, decrypt(data) just returns data and it is the same for
uncompress(data).
##########
java/tsfile/src/main/java/org/apache/tsfile/read/reader/chunk/ChunkReader.java:
##########
@@ -176,21 +204,11 @@ public static ByteBuffer uncompressPageData(
byte[] uncompressedPageData = new byte[pageHeader.getUncompressedSize()];
byte[] decryptedPageData = new byte[pageHeader.getCompressedSize()];
Review Comment:
No need to initialize here
##########
java/tsfile/src/main/java/org/apache/tsfile/encrypt/IDecryptor.java:
##########
@@ -0,0 +1,154 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.tsfile.encrypt;
+
+import org.apache.tsfile.exception.encrypt.EncryptException;
+import org.apache.tsfile.exception.encrypt.EncryptKeyLengthNotMatchException;
+import org.apache.tsfile.file.metadata.enums.EncryptionType;
+
+import javax.crypto.BadPaddingException;
+import javax.crypto.Cipher;
+import javax.crypto.IllegalBlockSizeException;
+import javax.crypto.NoSuchPaddingException;
+import javax.crypto.spec.IvParameterSpec;
+import javax.crypto.spec.SecretKeySpec;
+
+import java.io.Serializable;
+import java.security.InvalidAlgorithmParameterException;
+import java.security.InvalidKeyException;
+import java.security.NoSuchAlgorithmException;
+import java.util.Arrays;
+
+/** encrypt data according to tsfileconfig. */
+public interface IDecryptor extends Serializable {
Review Comment:
It is unnecessary. Remove it.
##########
java/tsfile/src/main/java/org/apache/tsfile/read/TsFileSequenceReader.java:
##########
@@ -371,23 +370,8 @@ public IDecryptor getDecryptor() throws IOException {
try {
readFileMetadata();
} catch (Exception e) {
- String encryptType;
- byte[] dataEncryptKey;
- if (config.getEncryptFlag()) {
- encryptType = config.getEncryptType();
- try {
- MessageDigest md = MessageDigest.getInstance("MD5");
- md.update("IoTDB is the best".getBytes());
- md.update(config.getEncryptKey().getBytes());
- dataEncryptKey = md.digest();
- } catch (Exception e1) {
- throw new EncryptException("md5 function not found while use md5 to
generate data key");
- }
- } else {
- encryptType = "UNENCRYPTED";
- dataEncryptKey = null;
- }
- return IDecryptor.getDecryptor(encryptType, dataEncryptKey);
+ logger.error("Something error happened while reading file metadata of
file {}", file);
Review Comment:
```
logger.error("Something error happened while reading file metadata of file
{}", file, e);
```
##########
java/tsfile/src/main/java/org/apache/tsfile/file/metadata/TsFileMetadata.java:
##########
@@ -105,12 +113,89 @@ public static TsFileMetadata deserializeFrom(ByteBuffer
buffer, DeserializeConfi
String value = ReadWriteIOUtils.readVarIntString(buffer);
propertiesMap.put(key, value);
}
+ // if the file is not encrypted, set the default value(for compatible
reason)
+ if (!propertiesMap.containsKey("encryptLevel")) {
+ propertiesMap.put("encryptLevel", "0");
+ propertiesMap.put("encryptType", "UNENCRYPTED");
+ propertiesMap.put("encryptKey", "");
+ } else if (propertiesMap.get("encryptLevel") == null) {
+ propertiesMap.put("encryptLevel", "0");
+ propertiesMap.put("encryptType", "UNENCRYPTED");
+ propertiesMap.put("encryptKey", "");
+ } else if (propertiesMap.get("encryptLevel").equals("0")) {
+ propertiesMap.put("encryptType", "UNENCRYPTED");
+ propertiesMap.put("encryptKey", "");
+ } else if (propertiesMap.get("encryptLevel").equals("1")) {
+ if (!propertiesMap.containsKey("encryptType")) {
+ throw new EncryptException("TsfileMetadata lack of encryptType while
encryptLevel is 2");
+ }
+ if (!propertiesMap.containsKey("encryptKey")) {
+ throw new EncryptException("TsfileMetadata lack of encryptKey while
encryptLevel is 2");
+ }
+ if (propertiesMap.get("encryptKey") == null ||
propertiesMap.get("encryptKey").isEmpty()) {
+ throw new EncryptException("TsfileMetadata null encryptKey while
encryptLevel is 2");
+ }
Review Comment:
I mean this.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
