From 8b207eed04fc11657d25dd85a8267a3ac4441eee Mon Sep 17 00:00:00 2001 From: Valdis Date: Thu, 1 Oct 2020 23:18:58 +0300 Subject: [PATCH 1/6] test --- .../common/creation/MetadataBuilderTest.java | 50 ++++++++++++++----- 1 file changed, 38 insertions(+), 12 deletions(-) diff --git a/common/src/test/java/com/turn/ttorrent/common/creation/MetadataBuilderTest.java b/common/src/test/java/com/turn/ttorrent/common/creation/MetadataBuilderTest.java index 193b25a2c..9f5f3bf1a 100644 --- a/common/src/test/java/com/turn/ttorrent/common/creation/MetadataBuilderTest.java +++ b/common/src/test/java/com/turn/ttorrent/common/creation/MetadataBuilderTest.java @@ -18,7 +18,11 @@ import com.turn.ttorrent.Constants; import com.turn.ttorrent.bcodec.BEValue; +import com.turn.ttorrent.common.TorrentMetadata; +import com.turn.ttorrent.common.TorrentParser; +import com.turn.ttorrent.common.TorrentSerializer; import com.turn.ttorrent.common.TorrentUtils; +import org.apache.commons.codec.binary.Hex; import org.testng.annotations.Test; import java.io.ByteArrayInputStream; @@ -33,10 +37,10 @@ public class MetadataBuilderTest { public void testMultiFileModeWithOneFile() throws IOException { - Map map = new MetadataBuilder() + MetadataBuilder builder = new MetadataBuilder() .setDirectoryName("root") - .addDataSource(new ByteArrayInputStream(new byte[]{1, 2}), "path/some_file", true) - .buildBEP().getMap(); + .addDataSource(new ByteArrayInputStream(new byte[]{1, 2}), "path/some_file", true); + Map map = builder.buildBEP().getMap(); Map info = map.get(INFO_TABLE).getMap(); assertEquals(info.get(NAME).getString(), "root"); List files = info.get(FILES).getList(); @@ -53,11 +57,14 @@ public void testMultiFileModeWithOneFile() throws IOException { path.append("/").append(iterator.next().getString()); } assertEquals(path.toString(), "path/some_file"); + + assertConsistentWithSerializer(builder.buildBinary()); + assertConsistentWithSerializer(builder.build()); } public void testBuildWithSpecifiedHashes() throws IOException { byte[] expectedHash = TorrentUtils.calculateSha1Hash(new byte[]{1, 2, 3}); - Map metadata = new MetadataBuilder() + MetadataBuilder builder = new MetadataBuilder() .setPiecesHashesCalculator(new PiecesHashesCalculator() { @Override public HashingResult calculateHashes(List sources, int pieceSize) { @@ -69,23 +76,26 @@ public HashingResult calculateHashes(List sources, int pieceSi Collections.singletonList("file"), Collections.singletonList(42L)) .setPieceLength(512) - .setTracker("http://localhost:12346") - .buildBEP().getMap(); + .setTracker("http://localhost:12346"); + Map metadata = builder.buildBEP().getMap(); assertEquals(metadata.get(ANNOUNCE).getString(), "http://localhost:12346"); Map info = metadata.get(INFO_TABLE).getMap(); assertEquals(info.get(PIECES).getBytes(), expectedHash); assertEquals(info.get(NAME).getString(), "file"); assertEquals(info.get(FILE_LENGTH).getLong(), 42); + + assertConsistentWithSerializer(builder.buildBinary()); + assertConsistentWithSerializer(builder.build()); } public void testSingleFile() throws IOException { byte[] data = {1, 2, 12, 4, 5}; - Map metadata = new MetadataBuilder() + MetadataBuilder builder = new MetadataBuilder() .addDataSource(new ByteArrayInputStream(data), "singleFile.txt", true) - .setTracker("http://localhost:12346") - .buildBEP().getMap(); + .setTracker("http://localhost:12346"); + Map metadata = builder.buildBEP().getMap(); assertEquals(metadata.get(ANNOUNCE).getString(), "http://localhost:12346"); assertNull(metadata.get(CREATION_DATE_SEC)); assertNull(metadata.get(COMMENT)); @@ -98,17 +108,19 @@ public void testSingleFile() throws IOException { assertEquals(info.get(FILE_LENGTH).getInt(), data.length); assertEquals(info.get(NAME).getString(), "singleFile.txt"); + assertConsistentWithSerializer(builder.buildBinary()); + assertConsistentWithSerializer(builder.build()); } public void testMultiFileWithOneFileValues() throws IOException { byte[] data = {34, 2, 12, 4, 5}; List paths = Arrays.asList("unix/path", "win\\path"); - Map metadata = new MetadataBuilder() + MetadataBuilder builder = new MetadataBuilder() .addDataSource(new ByteArrayInputStream(data), paths.get(0), true) .addDataSource(new ByteArrayInputStream(data), paths.get(1), true) - .setDirectoryName("downloadDirName") - .buildBEP().getMap(); + .setDirectoryName("downloadDirName"); + Map metadata = builder.buildBEP().getMap(); Map info = metadata.get(INFO_TABLE).getMap(); assertEquals(info.get(PIECES).getBytes().length, Constants.PIECE_HASH_SIZE); @@ -131,5 +143,19 @@ public void testMultiFileWithOneFileValues() throws IOException { } } + assertConsistentWithSerializer(builder.buildBinary()); + assertConsistentWithSerializer(builder.build()); + } + + private static void assertConsistentWithSerializer(TorrentMetadata metadata) throws IOException { + assertConsistentWithSerializer(new TorrentSerializer().serialize(metadata)); + } + + private static void assertConsistentWithSerializer(byte[] binary) throws IOException { + TorrentMetadata metadata = new TorrentParser().parse(binary); + byte[] backToBinary = new TorrentSerializer().serialize(metadata); + assertEquals(binary, backToBinary, + "\n Before:" + Hex.encodeHexString(binary) + + " \n After :" + Hex.encodeHexString(backToBinary)); } } From 8ab29bd1755ba3371b7f33af3c91eb315053692a Mon Sep 17 00:00:00 2001 From: Valdis Date: Thu, 1 Oct 2020 23:21:01 +0300 Subject: [PATCH 2/6] fix private field according to bep_0027 the "private=1" should be included to denote private torrent, but there is nothing about "private=0", so I assume we can omit it like in the serializer --- .../com/turn/ttorrent/common/creation/MetadataBuilder.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/common/src/main/java/com/turn/ttorrent/common/creation/MetadataBuilder.java b/common/src/main/java/com/turn/ttorrent/common/creation/MetadataBuilder.java index 71d07da0c..065d02684 100644 --- a/common/src/main/java/com/turn/ttorrent/common/creation/MetadataBuilder.java +++ b/common/src/main/java/com/turn/ttorrent/common/creation/MetadataBuilder.java @@ -355,7 +355,9 @@ private BEValue doBuild() throws IOException { Map info = new HashMap(); info.put(PIECE_LENGTH, new BEValue(pieceLength)); info.put(PIECES, concatHashes(hashingResult.getHashes())); - info.put(PRIVATE, new BEValue(isPrivate ? 1 : 0)); + if (isPrivate) { + info.put(PRIVATE, new BEValue(1)); + } info.put(NAME, new BEValue(name)); if (isSingleMode) { Long sourceSize = hashingResult.getSourceSizes().get(0); From 03a63c230e321ab06fac30e2b5a596c034c1db7f Mon Sep 17 00:00:00 2001 From: Valdis Date: Thu, 1 Oct 2020 23:36:02 +0300 Subject: [PATCH 3/6] output printable character when there are issues --- .../common/creation/MetadataBuilderTest.java | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/common/src/test/java/com/turn/ttorrent/common/creation/MetadataBuilderTest.java b/common/src/test/java/com/turn/ttorrent/common/creation/MetadataBuilderTest.java index 9f5f3bf1a..ffb54097a 100644 --- a/common/src/test/java/com/turn/ttorrent/common/creation/MetadataBuilderTest.java +++ b/common/src/test/java/com/turn/ttorrent/common/creation/MetadataBuilderTest.java @@ -155,7 +155,20 @@ private static void assertConsistentWithSerializer(byte[] binary) throws IOExcep TorrentMetadata metadata = new TorrentParser().parse(binary); byte[] backToBinary = new TorrentSerializer().serialize(metadata); assertEquals(binary, backToBinary, - "\n Before:" + Hex.encodeHexString(binary) + - " \n After :" + Hex.encodeHexString(backToBinary)); + "\n Before:" + printable(binary) + + " \n After :" + printable(backToBinary) + "\n" + ); + } + + private static String printable(byte[] binary) { + StringBuilder buf = new StringBuilder(); + for (byte b : binary) { + if ((' ' <= b) && (b <= 'z')) { + buf.append((char) b); + } else { + buf.append('.'); + } + } + return buf.toString(); } } From 769d87d0e03b984fbcd6620005111a41238dd0a1 Mon Sep 17 00:00:00 2001 From: Valdis Date: Fri, 9 Oct 2020 23:05:49 +0300 Subject: [PATCH 4/6] deal with single files that specify a custom path --- .../ttorrent/common/TorrentSerializer.java | 23 ++++++++++++------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/common/src/main/java/com/turn/ttorrent/common/TorrentSerializer.java b/common/src/main/java/com/turn/ttorrent/common/TorrentSerializer.java index 1aa287a31..a2f5f3105 100644 --- a/common/src/main/java/com/turn/ttorrent/common/TorrentSerializer.java +++ b/common/src/main/java/com/turn/ttorrent/common/TorrentSerializer.java @@ -7,10 +7,7 @@ import java.io.IOException; import java.io.UnsupportedEncodingException; import java.nio.ByteBuffer; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.List; -import java.util.Map; +import java.util.*; import static com.turn.ttorrent.common.TorrentMetadataKeys.*; @@ -40,11 +37,17 @@ public byte[] serialize(TorrentMetadata metadata) throws IOException { } infoTable.put(NAME, new BEValue(metadata.getDirectoryName())); - if (metadata.getFiles().size() == 1) { - final TorrentFile torrentFile = metadata.getFiles().get(0); - infoTable.put(FILE_LENGTH, new BEValue(torrentFile.size)); - putOptionalIfPresent(infoTable, MD5_SUM, torrentFile.md5Hash); + boolean needsMultiFileMode; + if (metadata.getFiles().size() != 1) { + needsMultiFileMode = true; } else { + // if the only file has a custom path (i.e. not at the root) we must keep that + TorrentFile onlyFile = metadata.getFiles().get(0); + List filePathInSingleMode = Collections.singletonList(metadata.getDirectoryName()); + needsMultiFileMode = !filePathInSingleMode.equals(onlyFile.relativePath); + } + + if (needsMultiFileMode) { List files = new ArrayList(); for (TorrentFile torrentFile : metadata.getFiles()) { Map entry = new HashMap(); @@ -54,6 +57,10 @@ public byte[] serialize(TorrentMetadata metadata) throws IOException { files.add(new BEValue(entry)); } infoTable.put(FILES, new BEValue(files)); + } else { + final TorrentFile torrentFile = metadata.getFiles().get(0); + infoTable.put(FILE_LENGTH, new BEValue(torrentFile.size)); + putOptionalIfPresent(infoTable, MD5_SUM, torrentFile.md5Hash); } mapMetadata.put(INFO_TABLE, new BEValue(infoTable)); From c5e051aba954ae90d1ceeb14f454e1f2bdc84e04 Mon Sep 17 00:00:00 2001 From: Valdis Date: Fri, 9 Oct 2020 23:08:26 +0300 Subject: [PATCH 5/6] show how it is still broken --- .../common/creation/MetadataBuilderTest.java | 27 ++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/common/src/test/java/com/turn/ttorrent/common/creation/MetadataBuilderTest.java b/common/src/test/java/com/turn/ttorrent/common/creation/MetadataBuilderTest.java index ffb54097a..ccaaf1caa 100644 --- a/common/src/test/java/com/turn/ttorrent/common/creation/MetadataBuilderTest.java +++ b/common/src/test/java/com/turn/ttorrent/common/creation/MetadataBuilderTest.java @@ -22,7 +22,6 @@ import com.turn.ttorrent.common.TorrentParser; import com.turn.ttorrent.common.TorrentSerializer; import com.turn.ttorrent.common.TorrentUtils; -import org.apache.commons.codec.binary.Hex; import org.testng.annotations.Test; import java.io.ByteArrayInputStream; @@ -62,6 +61,32 @@ public void testMultiFileModeWithOneFile() throws IOException { assertConsistentWithSerializer(builder.build()); } + public void testMultiFileModeWithOneFileSameName() throws IOException { + MetadataBuilder builder = new MetadataBuilder() + .setDirectoryName("abc") + .addDataSource(new ByteArrayInputStream(new byte[]{1, 2}), "abc", true); + Map map = builder.buildBEP().getMap(); + Map info = map.get(INFO_TABLE).getMap(); + assertEquals(info.get(NAME).getString(), "abc"); + List files = info.get(FILES).getList(); + assertEquals(files.size(), 1); + Map file = files.get(0).getMap(); + assertEquals(file.get(FILE_LENGTH).getInt(), 2); + + StringBuilder path = new StringBuilder(); + Iterator iterator = file.get(FILE_PATH).getList().iterator(); + if (iterator.hasNext()) { + path = new StringBuilder(iterator.next().getString()); + } + while (iterator.hasNext()) { + path.append("/").append(iterator.next().getString()); + } + assertEquals(path.toString(), "abc"); + + assertConsistentWithSerializer(builder.buildBinary()); + assertConsistentWithSerializer(builder.build()); + } + public void testBuildWithSpecifiedHashes() throws IOException { byte[] expectedHash = TorrentUtils.calculateSha1Hash(new byte[]{1, 2, 3}); MetadataBuilder builder = new MetadataBuilder() From 492e4cda533e56744aded9e6a80825194a3ee5e4 Mon Sep 17 00:00:00 2001 From: Valdis Date: Fri, 9 Oct 2020 23:24:31 +0300 Subject: [PATCH 6/6] A complete fix, but it requires changes to TorrentMetadata --- .../turn/ttorrent/common/TorrentMetadata.java | 7 ++++++- .../ttorrent/common/TorrentMetadataImpl.java | 12 +++++++++-- .../turn/ttorrent/common/TorrentParser.java | 6 ++++-- .../ttorrent/common/TorrentSerializer.java | 21 ++++++------------- .../ttorrent/common/TorrentParserTest.java | 2 +- .../ttorrent/client/CommunicationManager.java | 2 +- .../turn/ttorrent/client/SharedTorrent.java | 11 +++++++--- 7 files changed, 36 insertions(+), 25 deletions(-) diff --git a/common/src/main/java/com/turn/ttorrent/common/TorrentMetadata.java b/common/src/main/java/com/turn/ttorrent/common/TorrentMetadata.java index 22d6d1e36..d2fb7b293 100644 --- a/common/src/main/java/com/turn/ttorrent/common/TorrentMetadata.java +++ b/common/src/main/java/com/turn/ttorrent/common/TorrentMetadata.java @@ -64,7 +64,12 @@ public interface TorrentMetadata extends TorrentHash { int getPiecesCount(); /** - * @return The filename of the directory in which to store all the files + * @return The filename of single file or the directory in which to store all the files + */ + String getName(); + + /** + * @return The filename of the directory in which to store all the files, null if it is a single file */ String getDirectoryName(); diff --git a/common/src/main/java/com/turn/ttorrent/common/TorrentMetadataImpl.java b/common/src/main/java/com/turn/ttorrent/common/TorrentMetadataImpl.java index 8b831d036..1f8781d0d 100644 --- a/common/src/main/java/com/turn/ttorrent/common/TorrentMetadataImpl.java +++ b/common/src/main/java/com/turn/ttorrent/common/TorrentMetadataImpl.java @@ -1,6 +1,5 @@ package com.turn.ttorrent.common; -import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import java.util.List; @@ -15,6 +14,7 @@ public class TorrentMetadataImpl implements TorrentMetadata { private final String myComment; private final String myCreatedBy; private final String myName; + private final String myDirName; private final List myFiles; private final int myPieceCount; private final int myPieceLength; @@ -28,6 +28,7 @@ public class TorrentMetadataImpl implements TorrentMetadata { String comment, String createdBy, String name, + String dirName, List files, int pieceCount, int pieceLength, @@ -39,6 +40,7 @@ public class TorrentMetadataImpl implements TorrentMetadata { myComment = comment; myCreatedBy = createdBy; myName = name; + myDirName = dirName; myFiles = files; myPieceCount = pieceCount; myPieceLength = pieceLength; @@ -47,10 +49,16 @@ public class TorrentMetadataImpl implements TorrentMetadata { } @Override - public String getDirectoryName() { + public String getName() { return myName; } + @Override + public String getDirectoryName() { + return myDirName; + } + + @Override public List getFiles() { return myFiles; diff --git a/common/src/main/java/com/turn/ttorrent/common/TorrentParser.java b/common/src/main/java/com/turn/ttorrent/common/TorrentParser.java index 81ea51917..c08e0d06e 100644 --- a/common/src/main/java/com/turn/ttorrent/common/TorrentParser.java +++ b/common/src/main/java/com/turn/ttorrent/common/TorrentParser.java @@ -55,9 +55,10 @@ public TorrentMetadata parse(byte[] metadata) throws InvalidBEncodingException, final boolean torrentContainsManyFiles = infoTable.get(FILES) != null; - final String dirName = getRequiredValueOrThrowException(infoTable, NAME).getString(); + final String name = getRequiredValueOrThrowException(infoTable, NAME).getString(); + final String dirName = torrentContainsManyFiles ? name : null; - final List files = parseFiles(infoTable, torrentContainsManyFiles, dirName); + final List files = parseFiles(infoTable, torrentContainsManyFiles, name); if (piecesHashes.length % Constants.PIECE_HASH_SIZE != 0) throw new InvalidBEncodingException("Incorrect size of pieces hashes"); @@ -78,6 +79,7 @@ public TorrentMetadata parse(byte[] metadata) throws InvalidBEncodingException, creationDate, comment, createdBy, + name, dirName, files, piecesCount, diff --git a/common/src/main/java/com/turn/ttorrent/common/TorrentSerializer.java b/common/src/main/java/com/turn/ttorrent/common/TorrentSerializer.java index a2f5f3105..56ba11f2b 100644 --- a/common/src/main/java/com/turn/ttorrent/common/TorrentSerializer.java +++ b/common/src/main/java/com/turn/ttorrent/common/TorrentSerializer.java @@ -36,18 +36,13 @@ public byte[] serialize(TorrentMetadata metadata) throws IOException { infoTable.put(PRIVATE, new BEValue(1)); } - infoTable.put(NAME, new BEValue(metadata.getDirectoryName())); - boolean needsMultiFileMode; - if (metadata.getFiles().size() != 1) { - needsMultiFileMode = true; - } else { - // if the only file has a custom path (i.e. not at the root) we must keep that - TorrentFile onlyFile = metadata.getFiles().get(0); - List filePathInSingleMode = Collections.singletonList(metadata.getDirectoryName()); - needsMultiFileMode = !filePathInSingleMode.equals(onlyFile.relativePath); - } + infoTable.put(NAME, new BEValue(metadata.getName())); - if (needsMultiFileMode) { + if (metadata.getFiles().size() == 1 && metadata.getDirectoryName() == null) { + final TorrentFile torrentFile = metadata.getFiles().get(0); + infoTable.put(FILE_LENGTH, new BEValue(torrentFile.size)); + putOptionalIfPresent(infoTable, MD5_SUM, torrentFile.md5Hash); + } else { List files = new ArrayList(); for (TorrentFile torrentFile : metadata.getFiles()) { Map entry = new HashMap(); @@ -57,10 +52,6 @@ public byte[] serialize(TorrentMetadata metadata) throws IOException { files.add(new BEValue(entry)); } infoTable.put(FILES, new BEValue(files)); - } else { - final TorrentFile torrentFile = metadata.getFiles().get(0); - infoTable.put(FILE_LENGTH, new BEValue(torrentFile.size)); - putOptionalIfPresent(infoTable, MD5_SUM, torrentFile.md5Hash); } mapMetadata.put(INFO_TABLE, new BEValue(infoTable)); diff --git a/common/src/test/java/com/turn/ttorrent/common/TorrentParserTest.java b/common/src/test/java/com/turn/ttorrent/common/TorrentParserTest.java index dee596968..0a2ea15ef 100644 --- a/common/src/test/java/com/turn/ttorrent/common/TorrentParserTest.java +++ b/common/src/test/java/com/turn/ttorrent/common/TorrentParserTest.java @@ -55,7 +55,7 @@ public void parseTest() throws IOException { assertEquals(torrentMetadata.getPieceLength(), 4); assertEquals(torrentMetadata.getAnnounce(), "http://localhost/announce"); - assertEquals(torrentMetadata.getDirectoryName(), "test.file"); + assertEquals(torrentMetadata.getName(), "test.file"); assertNull(torrentMetadata.getAnnounceList()); List announceList = new ArrayList(); diff --git a/ttorrent-client/src/main/java/com/turn/ttorrent/client/CommunicationManager.java b/ttorrent-client/src/main/java/com/turn/ttorrent/client/CommunicationManager.java index 0bbd503d9..0a74167cb 100644 --- a/ttorrent-client/src/main/java/com/turn/ttorrent/client/CommunicationManager.java +++ b/ttorrent-client/src/main/java/com/turn/ttorrent/client/CommunicationManager.java @@ -767,7 +767,7 @@ private void validatePieceAsync(final SharedTorrent torrent, final Piece piece, isTorrentComplete = torrent.isComplete(); if (isTorrentComplete) { - logger.info("Download of {} complete.", torrent.getDirectoryName()); + logger.info("Download of {} complete.", torrent.getName()); torrent.finish(); } diff --git a/ttorrent-client/src/main/java/com/turn/ttorrent/client/SharedTorrent.java b/ttorrent-client/src/main/java/com/turn/ttorrent/client/SharedTorrent.java index db85ca2dd..c62e82970 100644 --- a/ttorrent-client/src/main/java/com/turn/ttorrent/client/SharedTorrent.java +++ b/ttorrent-client/src/main/java/com/turn/ttorrent/client/SharedTorrent.java @@ -243,7 +243,7 @@ private void hashSingleThread() { initPieces(); logger.debug("Analyzing local data for {} with {} threads...", - myTorrentMetadata.getDirectoryName(), TorrentCreator.HASHING_THREADS_COUNT); + myTorrentMetadata.getName(), TorrentCreator.HASHING_THREADS_COUNT); for (int idx = 0; idx < this.pieces.length; idx++) { byte[] hash = new byte[Constants.PIECE_HASH_SIZE]; this.piecesHashes.get(hash); @@ -268,7 +268,7 @@ private void hashSingleThread() { } public synchronized void close() { - logger.trace("Closing torrent", myTorrentMetadata.getDirectoryName()); + logger.trace("Closing torrent", myTorrentMetadata.getName()); try { this.pieceStorage.close(); isFileChannelOpen = false; @@ -279,7 +279,7 @@ public synchronized void close() { } public synchronized void closeFully() { - logger.trace("Closing torrent", myTorrentMetadata.getDirectoryName()); + logger.trace("Closing torrent", myTorrentMetadata.getName()); try { this.pieceStorage.closeFully(); isFileChannelOpen = false; @@ -745,6 +745,11 @@ public String toString() { "}"; } + @Override + public String getName() { + return myTorrentMetadata.getName(); + } + @Override public String getDirectoryName() { return myTorrentMetadata.getDirectoryName();