From 0477b2742cc65d4aee6fa305ae26958eda397b32 Mon Sep 17 00:00:00 2001 From: Jonathan Coates Date: Sat, 18 Dec 2021 11:23:12 +0000 Subject: [PATCH] Use duplicate() instead of rewind() It's just more confusing having to keep track of where the ByteBuffer is at. In this case, I think we were forgetting to rewind after computing the digest. Hopefully we'll be able to drop some of these in 1.17 as Java 16 has a few more ByteBuffer methods Fixes #992 --- .../computercraft/client/gui/ComputerScreenBase.java | 1 - .../shared/computer/core/IContainerComputer.java | 2 +- .../computercraft/shared/computer/upload/FileSlice.java | 7 +++---- .../computercraft/shared/computer/upload/FileUpload.java | 5 ++--- .../shared/network/server/UploadFileMessage.java | 8 +++----- 5 files changed, 9 insertions(+), 14 deletions(-) diff --git a/src/main/java/dan200/computercraft/client/gui/ComputerScreenBase.java b/src/main/java/dan200/computercraft/client/gui/ComputerScreenBase.java index 90329531f..eb126d554 100644 --- a/src/main/java/dan200/computercraft/client/gui/ComputerScreenBase.java +++ b/src/main/java/dan200/computercraft/client/gui/ComputerScreenBase.java @@ -173,7 +173,6 @@ public abstract class ComputerScreenBase extend return; } - buffer.rewind(); toUpload.add( new FileUpload( name, buffer, digest ) ); } catch( IOException e ) diff --git a/src/main/java/dan200/computercraft/shared/computer/core/IContainerComputer.java b/src/main/java/dan200/computercraft/shared/computer/core/IContainerComputer.java index f06547620..288d6faa3 100644 --- a/src/main/java/dan200/computercraft/shared/computer/core/IContainerComputer.java +++ b/src/main/java/dan200/computercraft/shared/computer/core/IContainerComputer.java @@ -56,7 +56,7 @@ public interface IContainerComputer void continueUpload( @Nonnull UUID uploadId, @Nonnull List slices ); /** - * Finish off an upload. This either writes the uploaded files or + * Finish off an upload. This either writes the uploaded files or informs the user that files will be overwritten. * * @param uploader The player uploading files. * @param uploadId The unique ID of this upload. diff --git a/src/main/java/dan200/computercraft/shared/computer/upload/FileSlice.java b/src/main/java/dan200/computercraft/shared/computer/upload/FileSlice.java index 2f290e660..c9103891b 100644 --- a/src/main/java/dan200/computercraft/shared/computer/upload/FileSlice.java +++ b/src/main/java/dan200/computercraft/shared/computer/upload/FileSlice.java @@ -53,10 +53,9 @@ public class FileSlice return; } - bytes.rewind(); - file.position( offset ); - file.put( bytes ); - file.rewind(); + ByteBuffer other = file.duplicate(); + other.position( offset ); // TODO: In 1.17 we can use a separate put(idx, _) method. + other.put( bytes ); if( bytes.remaining() != 0 ) throw new IllegalStateException( "Should have read the whole buffer" ); } diff --git a/src/main/java/dan200/computercraft/shared/computer/upload/FileUpload.java b/src/main/java/dan200/computercraft/shared/computer/upload/FileUpload.java index 6f70ad9e1..98000521d 100644 --- a/src/main/java/dan200/computercraft/shared/computer/upload/FileUpload.java +++ b/src/main/java/dan200/computercraft/shared/computer/upload/FileUpload.java @@ -56,7 +56,7 @@ public class FileUpload public boolean checksumMatches() { - // This is meant to be a checksum. Doesn't need to be cryptographically secure, hence non constant time. + // This is meant to be a checksum. Doesn't need to be cryptographically secure, hence non-constant time. byte[] digest = getDigest( bytes ); return digest != null && Arrays.equals( checksum, digest ); } @@ -66,9 +66,8 @@ public class FileUpload { try { - bytes.rewind(); MessageDigest digest = MessageDigest.getInstance( "SHA-256" ); - digest.update( bytes ); + digest.update( bytes.duplicate() ); return digest.digest(); } catch( NoSuchAlgorithmException e ) diff --git a/src/main/java/dan200/computercraft/shared/network/server/UploadFileMessage.java b/src/main/java/dan200/computercraft/shared/network/server/UploadFileMessage.java index 19445e63b..b26571e96 100644 --- a/src/main/java/dan200/computercraft/shared/network/server/UploadFileMessage.java +++ b/src/main/java/dan200/computercraft/shared/network/server/UploadFileMessage.java @@ -5,7 +5,6 @@ */ package dan200.computercraft.shared.network.server; -import dan200.computercraft.ComputerCraft; import dan200.computercraft.shared.computer.core.IContainerComputer; import dan200.computercraft.shared.computer.core.ServerComputer; import dan200.computercraft.shared.computer.upload.FileSlice; @@ -122,9 +121,9 @@ public class UploadFileMessage extends ComputerServerMessage buf.writeByte( slice.getFileId() ); buf.writeVarInt( slice.getOffset() ); - slice.getBytes().rewind(); - buf.writeShort( slice.getBytes().remaining() ); - buf.writeBytes( slice.getBytes() ); + ByteBuffer bytes = slice.getBytes().duplicate(); + buf.writeShort( bytes.remaining() ); + buf.writeBytes( bytes ); } } @@ -158,7 +157,6 @@ public class UploadFileMessage extends ComputerServerMessage int canWrite = Math.min( remaining, capacity - currentOffset ); - ComputerCraft.log.info( "Adding slice from {} to {}", currentOffset, currentOffset + canWrite - 1 ); contents.position( currentOffset ).limit( currentOffset + canWrite ); slices.add( new FileSlice( fileId, currentOffset, contents.slice() ) ); currentOffset += canWrite;