This is an automated email from the ASF dual-hosted git repository.
ctubbsii pushed a commit to branch 2.1
in repository https://gitbox.apache.org/repos/asf/accumulo.git
The following commit(s) were added to refs/heads/2.1 by this push:
new 861e9c5e00 Fix String-to-bytes and bytes-to-String for UTF-8 (#3815)
861e9c5e00 is described below
commit 861e9c5e00606b25d170cd9669830c4e445ef984
Author: Christopher Tubbs <ctubb...@apache.org>
AuthorDate: Fri Oct 13 17:00:09 2023 -0400
Fix String-to-bytes and bytes-to-String for UTF-8 (#3815)
* Fix various issues of improper conversion from byte arrays to String
or vice versa where the charset was not specified explicitly, in order
to standardize on the consistent use of UTF-8, so we don't have errors
due to mismatches in decoding. This is particularly important in the
serialization and deserialization of service lock data, which was
being encoded inconsistently prior to this patch.
* Also avoid unnecessary conversion of Text to String in order to
convert to bytes, especially since they may contain binary data. If
they contain binary data, it will now be preserved. If the Text was
constructed from a String and encoded as UTF-8 bytes, then the UTF-8
encoding will now be preserved, rather than converted to String type
and then subsequently converted to bytes using the system's default
encoding, which isn't predictable.
This does not attempt to fix UTF-8 encoding issues for any test code,
but does attempt to fully address it for all non-test code found in the
2.1 branch at the time this patch was created.
---
.../org/apache/accumulo/core/clientImpl/TableOperationsImpl.java | 2 +-
core/src/main/java/org/apache/accumulo/core/data/Mutation.java | 4 ++--
.../apache/accumulo/miniclusterImpl/MiniAccumuloClusterImpl.java | 2 +-
.../org/apache/accumulo/coordinator/CompactionCoordinator.java | 3 ++-
.../main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java | 3 ++-
.../src/main/java/org/apache/accumulo/manager/Manager.java | 8 ++++----
.../org/apache/accumulo/manager/ManagerClientServiceHandler.java | 2 +-
.../accumulo/manager/tableOps/compact/CompactionDriver.java | 3 ++-
.../java/org/apache/accumulo/tserver/TabletClientHandler.java | 2 +-
.../src/main/java/org/apache/accumulo/tserver/log/DfsLogger.java | 2 +-
.../src/main/java/org/apache/accumulo/tserver/log/LogSorter.java | 4 +++-
.../java/org/apache/accumulo/tserver/logger/LogFileValue.java | 2 +-
.../main/java/org/apache/accumulo/tserver/logger/LogReader.java | 2 +-
.../java/org/apache/accumulo/shell/commands/GetAuthsCommand.java | 4 +++-
14 files changed, 25 insertions(+), 18 deletions(-)
diff --git
a/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java
b/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java
index 8887113565..16f1d92284 100644
---
a/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java
+++
b/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java
@@ -1705,7 +1705,7 @@ public class TableOperationsImpl extends
TableOperationsHelper {
args.add(0, ByteBuffer.wrap(tableName.getBytes(UTF_8)));
args.add(1,
ByteBuffer.wrap(Boolean.toString(keepOffline).getBytes(UTF_8)));
args.add(2,
ByteBuffer.wrap(Boolean.toString(keepMapping).getBytes(UTF_8)));
-
checkedImportDirs.stream().map(String::getBytes).map(ByteBuffer::wrap).forEach(args::add);
+ checkedImportDirs.stream().map(s ->
s.getBytes(UTF_8)).map(ByteBuffer::wrap).forEach(args::add);
try {
doTableFateOperation(tableName, AccumuloException.class,
FateOperation.TABLE_IMPORT, args,
diff --git a/core/src/main/java/org/apache/accumulo/core/data/Mutation.java
b/core/src/main/java/org/apache/accumulo/core/data/Mutation.java
index edc3f3fea7..53f834cedf 100644
--- a/core/src/main/java/org/apache/accumulo/core/data/Mutation.java
+++ b/core/src/main/java/org/apache/accumulo/core/data/Mutation.java
@@ -1095,7 +1095,7 @@ public class Mutation implements Writable {
*/
@Override
public TimestampOptions visibility(Text colVis) {
- return visibility(colVis.toString().getBytes());
+ return visibility(colVis.copyBytes());
}
/**
@@ -1201,7 +1201,7 @@ public class Mutation implements Writable {
*/
@Override
public Mutation put(Text val) {
- return put(val.toString().getBytes(), false);
+ return put(val.copyBytes(), false);
}
/**
diff --git
a/minicluster/src/main/java/org/apache/accumulo/miniclusterImpl/MiniAccumuloClusterImpl.java
b/minicluster/src/main/java/org/apache/accumulo/miniclusterImpl/MiniAccumuloClusterImpl.java
index a6470efe83..15096b139f 100644
---
a/minicluster/src/main/java/org/apache/accumulo/miniclusterImpl/MiniAccumuloClusterImpl.java
+++
b/minicluster/src/main/java/org/apache/accumulo/miniclusterImpl/MiniAccumuloClusterImpl.java
@@ -549,7 +549,7 @@ public class MiniAccumuloClusterImpl implements
AccumuloCluster {
while (true) {
try (Socket s = new Socket("localhost",
config.getZooKeeperPort())) {
s.setReuseAddress(true);
- s.getOutputStream().write("ruok\n".getBytes());
+ s.getOutputStream().write("ruok\n".getBytes(UTF_8));
s.getOutputStream().flush();
byte[] buffer = new byte[100];
int n = s.getInputStream().read(buffer);
diff --git
a/server/compaction-coordinator/src/main/java/org/apache/accumulo/coordinator/CompactionCoordinator.java
b/server/compaction-coordinator/src/main/java/org/apache/accumulo/coordinator/CompactionCoordinator.java
index 8cdda4fd50..914f4fe112 100644
---
a/server/compaction-coordinator/src/main/java/org/apache/accumulo/coordinator/CompactionCoordinator.java
+++
b/server/compaction-coordinator/src/main/java/org/apache/accumulo/coordinator/CompactionCoordinator.java
@@ -18,6 +18,7 @@
*/
package org.apache.accumulo.coordinator;
+import static java.nio.charset.StandardCharsets.UTF_8;
import static
org.apache.accumulo.core.util.UtilWaitThread.sleepUninterruptibly;
import java.lang.reflect.InvocationTargetException;
@@ -212,7 +213,7 @@ public class CompactionCoordinator extends AbstractServer
CoordinatorLockWatcher coordinatorLockWatcher = new
CoordinatorLockWatcher();
coordinatorLock = new
ServiceLock(getContext().getZooReaderWriter().getZooKeeper(),
ServiceLock.path(lockPath), zooLockUUID);
- coordinatorLock.lock(coordinatorLockWatcher,
coordinatorClientAddress.getBytes());
+ coordinatorLock.lock(coordinatorLockWatcher,
coordinatorClientAddress.getBytes(UTF_8));
coordinatorLockWatcher.waitForChange();
if (coordinatorLockWatcher.isAcquiredLock()) {
diff --git
a/server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java
b/server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java
index 8b8a4ff9c6..550b3426a5 100644
--- a/server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java
+++ b/server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java
@@ -18,6 +18,7 @@
*/
package org.apache.accumulo.gc;
+import static java.nio.charset.StandardCharsets.UTF_8;
import static
org.apache.accumulo.core.util.UtilWaitThread.sleepUninterruptibly;
import java.io.FileNotFoundException;
@@ -387,7 +388,7 @@ public class SimpleGarbageCollector extends AbstractServer
implements Iface {
ServiceLock lock =
new ServiceLock(getContext().getZooReaderWriter().getZooKeeper(),
path, zooLockUUID);
if (lock.tryLock(lockWatcher,
- new ServerServices(addr.toString(),
Service.GC_CLIENT).toString().getBytes())) {
+ new ServerServices(addr.toString(),
Service.GC_CLIENT).toString().getBytes(UTF_8))) {
log.debug("Got GC ZooKeeper lock");
return;
}
diff --git
a/server/manager/src/main/java/org/apache/accumulo/manager/Manager.java
b/server/manager/src/main/java/org/apache/accumulo/manager/Manager.java
index bab128fb23..b86cba576c 100644
--- a/server/manager/src/main/java/org/apache/accumulo/manager/Manager.java
+++ b/server/manager/src/main/java/org/apache/accumulo/manager/Manager.java
@@ -524,7 +524,7 @@ public class Manager extends AbstractServer
void setManagerGoalState(ManagerGoalState state) {
try {
getContext().getZooReaderWriter().putPersistentData(
- getZooKeeperRoot() + Constants.ZMANAGER_GOAL_STATE,
state.name().getBytes(),
+ getZooKeeperRoot() + Constants.ZMANAGER_GOAL_STATE,
state.name().getBytes(UTF_8),
NodeExistsPolicy.OVERWRITE);
} catch (Exception ex) {
log.error("Unable to set manager goal state in zookeeper");
@@ -536,7 +536,7 @@ public class Manager extends AbstractServer
try {
byte[] data = getContext().getZooReaderWriter()
.getData(getZooKeeperRoot() + Constants.ZMANAGER_GOAL_STATE);
- return ManagerGoalState.valueOf(new String(data));
+ return ManagerGoalState.valueOf(new String(data, UTF_8));
} catch (Exception e) {
log.error("Problem getting real goal state from zookeeper: ", e);
sleepUninterruptibly(1, SECONDS);
@@ -1233,7 +1233,7 @@ public class Manager extends AbstractServer
String address = sa.address.toString();
log.info("Setting manager lock data to {}", address);
try {
- managerLock.replaceLockData(address.getBytes());
+ managerLock.replaceLockData(address.getBytes(UTF_8));
} catch (KeeperException | InterruptedException e) {
throw new IllegalStateException("Exception updating manager lock", e);
}
@@ -1521,7 +1521,7 @@ public class Manager extends AbstractServer
ManagerLockWatcher managerLockWatcher = new ManagerLockWatcher();
managerLock = new ServiceLock(zooKeeper, zManagerLoc, zooLockUUID);
- managerLock.lock(managerLockWatcher, managerClientAddress.getBytes());
+ managerLock.lock(managerLockWatcher,
managerClientAddress.getBytes(UTF_8));
managerLockWatcher.waitForChange();
diff --git
a/server/manager/src/main/java/org/apache/accumulo/manager/ManagerClientServiceHandler.java
b/server/manager/src/main/java/org/apache/accumulo/manager/ManagerClientServiceHandler.java
index 5d04d9c2ec..1a8d0d0cac 100644
---
a/server/manager/src/main/java/org/apache/accumulo/manager/ManagerClientServiceHandler.java
+++
b/server/manager/src/main/java/org/apache/accumulo/manager/ManagerClientServiceHandler.java
@@ -140,7 +140,7 @@ public class ManagerClientServiceHandler implements
ManagerClientService.Iface {
throw new ThriftTableOperationException(tableId.canonical(), null,
TableOperation.FLUSH,
TableOperationExceptionType.OTHER, null);
}
- return Long.parseLong(new String(fid));
+ return Long.parseLong(new String(fid, UTF_8));
}
@Override
diff --git
a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/compact/CompactionDriver.java
b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/compact/CompactionDriver.java
index cf2b669030..b17ce8b63c 100644
---
a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/compact/CompactionDriver.java
+++
b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/compact/CompactionDriver.java
@@ -18,6 +18,7 @@
*/
package org.apache.accumulo.manager.tableOps.compact;
+import static java.nio.charset.StandardCharsets.UTF_8;
import static
org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType.COMPACT_ID;
import static
org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType.LOCATION;
import static
org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType.PREV_ROW;
@@ -81,7 +82,7 @@ class CompactionDriver extends ManagerRepo {
String zCancelID =
createCompactionCancellationPath(manager.getInstanceID(), tableId);
ZooReaderWriter zoo = manager.getContext().getZooReaderWriter();
- if (Long.parseLong(new String(zoo.getData(zCancelID))) >= compactId) {
+ if (Long.parseLong(new String(zoo.getData(zCancelID), UTF_8)) >=
compactId) {
// compaction was canceled
throw new AcceptableThriftTableOperationException(tableId.canonical(),
null,
TableOperation.COMPACT, TableOperationExceptionType.OTHER,
diff --git
a/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletClientHandler.java
b/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletClientHandler.java
index ed430d00ff..509dcffc70 100644
---
a/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletClientHandler.java
+++
b/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletClientHandler.java
@@ -999,7 +999,7 @@ public class TabletClientHandler implements
TabletClientService.Iface {
public void splitTablet(TInfo tinfo, TCredentials credentials, TKeyExtent
tkeyExtent,
ByteBuffer splitPoint) throws NotServingTabletException,
ThriftSecurityException {
- TableId tableId = TableId.of(new
String(ByteBufferUtil.toBytes(tkeyExtent.table)));
+ TableId tableId = TableId.of(new
String(ByteBufferUtil.toBytes(tkeyExtent.table), UTF_8));
NamespaceId namespaceId = getNamespaceId(credentials, tableId);
if (!security.canSplitTablet(credentials, tableId, namespaceId)) {
diff --git
a/server/tserver/src/main/java/org/apache/accumulo/tserver/log/DfsLogger.java
b/server/tserver/src/main/java/org/apache/accumulo/tserver/log/DfsLogger.java
index 09e809b51b..da578e64b4 100644
---
a/server/tserver/src/main/java/org/apache/accumulo/tserver/log/DfsLogger.java
+++
b/server/tserver/src/main/java/org/apache/accumulo/tserver/log/DfsLogger.java
@@ -376,7 +376,7 @@ public class DfsLogger implements Comparable<DfsLogger> {
decryptingInput = input;
} else {
throw new IllegalArgumentException(
- "Unsupported write ahead log version " + new String(magicBuffer));
+ "Unsupported write ahead log version " + new String(magicBuffer,
UTF_8));
}
} catch (EOFException e) {
// Explicitly catch any exceptions that should be converted to
LogHeaderIncompleteException
diff --git
a/server/tserver/src/main/java/org/apache/accumulo/tserver/log/LogSorter.java
b/server/tserver/src/main/java/org/apache/accumulo/tserver/log/LogSorter.java
index 9472eeaf5f..68083170e9 100644
---
a/server/tserver/src/main/java/org/apache/accumulo/tserver/log/LogSorter.java
+++
b/server/tserver/src/main/java/org/apache/accumulo/tserver/log/LogSorter.java
@@ -18,6 +18,8 @@
*/
package org.apache.accumulo.tserver.log;
+import static java.nio.charset.StandardCharsets.UTF_8;
+
import java.io.DataInputStream;
import java.io.EOFException;
import java.io.IOException;
@@ -82,7 +84,7 @@ public class LogSorter {
@Override
public void process(String child, byte[] data) {
- String work = new String(data);
+ String work = new String(data, UTF_8);
String[] parts = work.split("\\|");
String src = parts[0];
String dest = parts[1];
diff --git
a/server/tserver/src/main/java/org/apache/accumulo/tserver/logger/LogFileValue.java
b/server/tserver/src/main/java/org/apache/accumulo/tserver/logger/LogFileValue.java
index eca861c430..3f1d8f3a3d 100644
---
a/server/tserver/src/main/java/org/apache/accumulo/tserver/logger/LogFileValue.java
+++
b/server/tserver/src/main/java/org/apache/accumulo/tserver/logger/LogFileValue.java
@@ -84,7 +84,7 @@ public class LogFileValue implements Writable {
}
builder.append(" ").append(new String(m.getRow(), UTF_8)).append("\n");
for (ColumnUpdate update : m.getUpdates()) {
- String value = new String(update.getValue());
+ String value = new String(update.getValue(), UTF_8);
builder.append(" ").append(new String(update.getColumnFamily(),
UTF_8)).append(":")
.append(new String(update.getColumnQualifier(), UTF_8)).append(" ")
.append(update.hasTimestamp() ? "[user]:" :
"[system]:").append(update.getTimestamp())
diff --git
a/server/tserver/src/main/java/org/apache/accumulo/tserver/logger/LogReader.java
b/server/tserver/src/main/java/org/apache/accumulo/tserver/logger/LogReader.java
index 851352d837..8f2fb08bd9 100644
---
a/server/tserver/src/main/java/org/apache/accumulo/tserver/logger/LogReader.java
+++
b/server/tserver/src/main/java/org/apache/accumulo/tserver/logger/LogReader.java
@@ -217,7 +217,7 @@ public class LogReader implements KeywordExecutable {
}
} else {
throw new IllegalArgumentException(
- "Unsupported write ahead log version " + new String(magicBuffer));
+ "Unsupported write ahead log version " + new String(magicBuffer,
UTF_8));
}
} catch (EOFException e) {
log.warn("Could not read header for {} . Ignoring...", path);
diff --git
a/shell/src/main/java/org/apache/accumulo/shell/commands/GetAuthsCommand.java
b/shell/src/main/java/org/apache/accumulo/shell/commands/GetAuthsCommand.java
index b4aa5c5598..78066666f3 100644
---
a/shell/src/main/java/org/apache/accumulo/shell/commands/GetAuthsCommand.java
+++
b/shell/src/main/java/org/apache/accumulo/shell/commands/GetAuthsCommand.java
@@ -18,6 +18,8 @@
*/
package org.apache.accumulo.shell.commands;
+import static java.nio.charset.StandardCharsets.UTF_8;
+
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
@@ -51,7 +53,7 @@ public class GetAuthsCommand extends Command {
protected List<String> sortAuthorizations(Authorizations auths) {
List<String> list = new ArrayList<>();
for (byte[] auth : auths) {
- list.add(new String(auth));
+ list.add(new String(auth, UTF_8));
}
list.sort(String.CASE_INSENSITIVE_ORDER);
return list;
