Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error trying to upload a torrent file #266

Closed
josecelano opened this issue Aug 30, 2023 · 12 comments
Closed

Error trying to upload a torrent file #266

josecelano opened this issue Aug 30, 2023 · 12 comments
Labels
Bug Incorrect Behavior

Comments

@josecelano
Copy link
Member

josecelano commented Aug 30, 2023

I get the error:

{"error":"Uploaded torrent is not valid."}

trying to upload this torrent:

MC_GRID.zip-3cd18ff2d3eec881207dcc5ca5a2c3a2a3afe462.zip

I think the problem could be the field url-list. We do not have it in the Torrent struct.

{
   "announce": "https://academictorrents.com/announce.php",
   "announce-list": [
      [
         "https://academictorrents.com/announce.php"
      ],
      [
         "https://ipv6.academictorrents.com/announce.php"
      ],
      [
         "udp://tracker.opentrackr.org:1337/announce"
      ],
      [
         "udp://tracker.openbittorrent.com:80/announce"
      ]
   ],
   "created by": "BitComet/1.86",
   "creation date": 1650039422,
   "encoding": "UTF-8",
   "info": {
      "ed2k": "<hex>62 60 E2 30 EE AD 1A FC 19 5C B5 F7 11 E2 20 BE</hex>",
      "filehash": "<hex>E4 BB C7 8C CB 94 3A 14 2C 89 81 1A 0F 09 67 4B A6 4C 1B FA</hex>",
      "length": 12867952426,
      "name": "MC_GRID.zip",
      "name.utf-8": "MC_GRID.zip",
      "piece length": 2097152,
      "pieces": "<hex>...</hex>"
   },
   "nodes": [
      [
         "188.163.121.224",
         56711
      ],
      [
         "162.250.131.26",
         13386
      ],
      [
         "195.154.181.225",
         39616
      ],
      [
         "47.184.14.143",
         6881
      ],
      [
         "62.195.208.134",
         6881
      ],
      [
         "185.148.3.232",
         6881
      ],
      [
         "154.45.216.246",
         1043
      ],
      [
         "177.125.59.73",
         50321
      ],
      [
         "185.107.71.140",
         28015
      ],
      [
         "91.122.25.53",
         19020
      ]
   ],
   "url-list": [
      "magnet:?xt=urn:btih:HTIY74WT53EICID5ZROKLIWDUKR27ZDC&dn=MC_GRID.zip&xl=12867952426"
   ]
}
@josecelano josecelano added the Bug Incorrect Behavior label Aug 30, 2023
@josecelano
Copy link
Member Author

josecelano commented Aug 31, 2023

The url-list field is defined in the: BEP 19 - WebSeed - HTTP/FTP Seeding (GetRight style)

image

@da2ce7 I do not understand why it isn't an FTP or HTTP URL instead of a magnet link.

Anyway, we could add the field to our Torrent struct to allow uploading those torrents even if we don't support BEP 19 yet.

Anyway, I have to confirm that that field is what makes the torrent invalid when you upload it. Maybe we should show the "serde" error message, although is not clear either.

@josecelano
Copy link
Member Author

Hey @da2ce7 I've been debugging this issue, but I still do not know why it's not working. In the end, It was not the url-list field. I've created a new sample project to isolate the problem:

https://github.com/torrust/torrust-parse-torrent

It's a simple console app to decode the torrent file, reusing the code from this project. Surprisingly, It's working on the sample project. Maybe there was a bug on one of the dependencies we are using, and it has been fixed, and I'm using the newer version in the sample project.

I've checked the torrent buffer in both applications (the byte array, after uploading the torrent or after reading the file in the sample app case) and both are the same. That means, using exactly the same bytes in both cases the function:

pub fn decode_torrent(bytes: &[u8]) -> Result<Torrent, Box<dyn error::Error>> {
    match de::from_bytes::<Torrent>(bytes) {
        Ok(torrent) => Ok(torrent),
        Err(e) => {
            println!("{e:?}");
            Err(e.into())
        }
    }
}

is not working on the backed project but it's working on the sample project.

I'm going to compare the dependency versions in both projects.

The good thing about debugging this bug is I have written a custom torrent parser here:

https://github.com/torrust/torrust-parse-torrent/blob/main/src/utils/parse_torrent_verbose.rs

Maybe we could use it as a base for a verbose torrent parser if we want to throw meaningful errors in the future. The error we are getting now from "serde" is:

InvalidType("Invalid Type: sequence (expected: `field identifier`)")

Which is not very helpful.

@josecelano
Copy link
Member Author

It fails in this project even after upgrading serde dependencies :-/.

@josecelano
Copy link
Member Author

Since the pieces byte array is quite long for this torrent, maybe the error could be related to this issue:

toby/serde-bencode#28

But I do not think so because it's working on the sample project.

@josecelano
Copy link
Member Author

I've added a new binary to confirm that the problem is inside "serde" and not because we are not using the same byte array:

#267

The new binary does the same as we are doing in the other repo (which works):

https://github.com/torrust/torrust-parse-torrent

I've tested it after upgrading "serde" dependencies to use the identical versions we are using on the other project.

josecelano added a commit that referenced this issue Sep 11, 2023
…poses)

d9cdd65 feat: new binary to par torrent files (Jose Celano)

Pull request description:

  I'm trying to fix bug #266

  When I try to upload this torrent:

  https://academictorrents.com/details/3cd18ff2d3eec881207dcc5ca5a2c3a2a3afe462

  I get this error trying to parse it:

  ```
  InvalidType("Invalid Type: sequence (expected: `field identifier`)")
  ```

  I've tried to reproduce the error with a clean project:

  https://github.com/torrust/torrust-parse-torrent

  But It works.

  I've added a binary to reproduce the error in this project. You can run it with:

  ```
  cargo run --bin parse_torrent ./tests/fixtures/torrents/MC_GRID.zip-3cd18ff2d3eec881207dcc5ca5a2c3a2a3afe462.torrent
  ```

  Output:

  ```s
  Reading the torrent file ...
  Decoding torrent with standard serde implementation ...
  InvalidType("Invalid Type: sequence (expected: `field identifier`)")
  Error: Custom { kind: Other, error: "Error: invalid torrent!. Invalid Type: sequence (expected: `field identifier`)" }
  ```

  I need more ideas about how to solve this bug. I've checked the dependencies, and It does not work even if I use the same "serde" dependencies in this project.

  The torrent only has two different things:

  - The "pieces" field is big.
  - It has an extra field from BEP 19.

  I only have two more things to test:

  - ~~Create another torrent with a big "pieces" field. Only to check if the size is the problem~~. DONE.
  - Clone the "serve" crate and try to debug the code. I think the error is thrown [here](https://github.com/serde-rs/serde/blob/dad15b9fd0bef97b7a7c90a8a165b6ffbc682cae/serde/src/de/mod.rs#L1646). It seems the deserializer is expecting a file identifier (I guess for a Dictionary) and is getting a sequence.

Top commit has no ACKs.

Tree-SHA512: c9f02021801d492e34b7550662cfb6747bbf81f3724c3afdd3d92e22061066c400b721f35e80100d3253ff79fbda10d0bf0f7ea29df964f57ee97a1607b50fd3
@josecelano
Copy link
Member Author

THe bug is still not fixed but you can reproduce it with the following:

cargo run --bin parse_torrent ./tests/fixtures/torrents/MC_GRID.zip-3cd18ff2d3eec881207dcc5ca5a2c3a2a3afe462.torrent

@josecelano
Copy link
Member Author

josecelano commented Sep 15, 2023

I've been debugging for some hours, and I have been able to isolate the problem. Finally, it's the nodes field. I've created a minimal torrent file that causes the bug.

Working torrent file with only one node

working-with-one-node.zip

{
   "info": {
      "length": 8,
      "name": "minimal.txt",
      "piece length": 16384,
      "pieces": "<hex>30 9A 84 51 4A F1 09 D0 8C 34 42 11 26 67 7F 84 B3 23 4D 78</hex>"
   },
   "nodes": [
      [
         "188.163.121.224",
         56711
      ]
   ]
}

Not working torrent file with two nodes

not-working-with-two-nodes.zip

{
   "info": {
      "length": 8,
      "name": "minimal.txt",
      "piece length": 16384,
      "pieces": "<hex>30 9A 84 51 4A F1 09 D0 8C 34 42 11 26 67 7F 84 B3 23 4D 78</hex>"
   },
   "nodes": [
      [
         "188.163.121.224",
         56711
      ],
      [
         "162.250.131.26",
         13386
      ]
   ]
}
00000000: 6434 3a69 6e66 6f64 363a 6c65 6e67 7468  d4:infod6:length
00000010: 6938 6534 3a6e 616d 6531 313a 6d69 6e69  i8e4:name11:mini
00000020: 6d61 6c2e 7478 7431 323a 7069 6563 6520  mal.txt12:piece 
00000030: 6c65 6e67 7468 6931 3633 3834 6536 3a70  lengthi16384e6:p
00000040: 6965 6365 7332 303a 309a 8451 4af1 09d0  ieces20:0..QJ...
00000050: 8c34 4211 2667 7f84 b323 4d78 6535 3a6e  .4B.&g...#Mxe5:n
00000060: 6f64 6573 6c6c 3135 3a31 3838 2e31 3633  odesll15:188.163
00000070: 2e31 3231 2e32 3234 6935 3637 3131 6565  .121.224i56711ee
00000080: 6c31 343a 3136 322e 3235 302e 3133 312e  l14:162.250.131.
00000090: 3236 6931 3333 3836 6565 6565            26i13386eeee

The non-working torrent also works with the https://github.com/torrust/torrust-parse-torrent

@josecelano
Copy link
Member Author

I've been able to reproduce the error with only the library. I've opened a PR with a test reproducing the error.

It was failing since the beginning in both:

In both cases, the error was there but hidden, and I thought it was working :-(

cc @da2ce7

josecelano added a commit that referenced this issue Sep 15, 2023
3cf9c44 test: add two more test torrents (Jose Celano)
50cef81 chore: update config for debuggin in Visual Studio Code (Jose Celano)

Pull request description:

  Updated configuration for debugging in Visual Studio Code following this guide:

  https://www.forrestthewoods.com/blog/how-to-debug-rust-with-visual-studio-code/

  by @forrestthewoods

  Thank you very much @forrestthewoods!

  I've also added a minimal torrent to reproduce a bug described [here](#266).

ACKs for top commit:
  josecelano:
    ACK 3cf9c44

Tree-SHA512: 6a093101ca3172727610cd49abc94ef9a4bde20edb5d9bc7a718f105511d9c1a709ed39b132f3301b46e4a6c52120ffe4390bcf259d3a97b3fcb5b6f08cabcfd
@josecelano josecelano removed the Help Wanted More Contributions are Appreciated label Sep 20, 2023
josecelano added a commit to torrust/torrust-serde-bencode-archive that referenced this issue Sep 25, 2023
6ce2e9d test: add a failing test for nested list deserialization (Jose Celano)

Pull request description:

  Add failing test for this bug: torrust/torrust-index#266

ACKs for top commit:
  josecelano:
    ACK 6ce2e9d

Tree-SHA512: bd40e3e7b5926b4ce4bddf468c31c776729d5e23705334e134ea5aff228b8cc8fd67616aee13f32fc733abba194eab696e1d4c0c199b1ec1e1619458013bfc6b
@josecelano
Copy link
Member Author

@josecelano
Copy link
Member Author

I forked the serde-bencode crate, and I realized the bug was fixed in the repo, but it has not been included in a new release yet. Now I'm a mantainer of the serde-bencode repo. I will publish a new minor version soon. More info.

josecelano added a commit to toby/serde-bencode that referenced this issue Oct 9, 2023
f58baf7 chore: undo package rename (Jose Celano)
9833274 tests: add unit test for Value with serde_test (Jose Celano)
f94c198 chore: add dev dependency serde_test (Jose Celano)
1eab404 test: add unit tests to Value (Jose Celano)
b85b2a4 chore: add code coverage dir to gitignore (Jose Celano)
97168c5 chore: normalize todo mark to lowercase (Jose Celano)
e2751eb chore(dev): add cargo alias (Jose Celano)
fd7bdce test: add more tests for torrent files (Jose Celano)
9efb5e8 tests: enable tests with potencial unintended behavior (Jose Celano)
a7fd343 test: add more failing tests (Jose Celano)
a7587d4 docs: fix coverage workflow badge in README (Jose Celano)
252849f ci: add coverage workflow (Jose Celano)
5bc8202 ci: ignore failing tests (Jose Celano)
36e31ae docs: fix workflow badge link in README (Jose Celano)
885ebe9 chrore: update cSpell dictionary (Jose Celano)
5e9dd5c test: enable test (Jose Celano)
1ea262d ci: add benchmarking workflow (Jose Celano)
b3b2a44 fix: [#9] Clipply errors (Jose Celano)
cdd4200 fix: cargo build errors (Jose Celano)
caf8cf5 doc: add new workflow badgets to README (Jose Celano)
b3759c1 chore: bump actions/checkout from 3 to 4 (Jose Celano)
5b35fd7 ci: add cargo config (Jose Celano)
a833a34 ci: add formatting and checking workflows (Jose Celano)
e88aeec doc: add testing workflow badget to README (Jose Celano)
47549ef ci: add dependabot config (Jose Celano)
6ce2e9d test: add a failing test for nested list deserialization (Jose Celano)
decdff6 Avoid eagerly allocating too much memory (5225225)
a1597cc Add cargo-fuzz fuzzer (5225225)
43ba81e Introduce failing test for flattened enums (Nicholas Rempel)
625081a new torrust-serde-bencode package (Jose Celano)
403f043 Add test for ser/de of struct with vec of tuples (issue #17) (Adam Cigánek)
4faf05f Minor change to support rust version >= 1.40.0 (Adam Cigánek)
0c6c144 Fix deserialization of nested tuple (Adam Cigánek)
06bb9a6 Fix deserialization of flattened adjacently tagged enum (Adam Cigánek)
79b7540 Fix deserialization of adjacently tagged enums (Adam Cigánek)
1eecff4 Fix warnings and clippy lints (Adam Cigánek)
e6f9b53 Apply rustfmt (Adam Cigánek)

Pull request description:

  I have been working on a fork because this project was inactive, but I contacted @toby, who made me a maintainer and gave it access to publish new versions.

  I was trying to fix a bug I found [here](torrust/torrust-index#266). . After debugging, I realized the bug was already fixed in this repo but has yet to be included in a new release.

  I've been working on the fork and I would like to merge those changes back and publish the version `v0.2.4` on [crates.io](https://crates.io/crates/serde_bencode).

  The changes I've made so far are:

  - Fix cargo build and clippy warnings.
  - Merge PRs that were ready to merge.
  - Update dependencies.
  - Add GitHub workflows for checking, formatting, testing, benchmarking and coverage.
  - Add tests for torrent files, which I suppose is the most common use for this package.
  - Add unit tests for `Value` struct.

ACKs for top commit:
  josecelano:
    ACK e894328

Tree-SHA512: 331e82dba9e0a8f4f2c9be30a09537707c39e05223d601bbfb7f6792a7e7e95b9bf60a65b097ed6ee2aea0e2c22196a8d6ded8c82cd9a59db274774002e50370
josecelano added a commit that referenced this issue Oct 27, 2023
3550c44 fix: [#226] user serde-bencode v0.2.4 (Jose Celano)

Pull request description:

  Using our crate `torrust-serde-bencode` instead of the upstream repo fixes the problem with some torrent deserializations described in this [issue](#266).

  There was a problem with lists (field `nodes` in the torrent) containing other lists (tuples) with different value types.

  ```json
  {
     "info": {
        "length": 8,
        "name": "minimal.txt",
        "piece length": 16384,
        "pieces": "<hex>30 9A 84 51 4A F1 09 D0 8C 34 42 11 26 67 7F 84 B3 23 4D 78</hex>"
     },
     "nodes": [
        [
           "188.163.121.224",
           56711
        ],
        [
           "162.250.131.26",
           13386
        ]
     ]
  }
  ```

  I have been trying to reproduce the bug in our fork [here](torrust/torrust-serde-bencode-archive#12), but it works. I've added a test using the same torrent file that does not work here.

  If you run:

  ```console
  cargo run --bin parse_torrent ./tests/fixtures/torrents/not-working-with-two-nodes.torrent
  ```

  With the current `develop` branch, you'll see that it does **NOT** work. But It works using our fork.

  For this PR, I have changed the `cargo.toml` file to include the latest commit from the fork. And it works.

  Surprisingly, it also works with:

  ```toml
  torrust-serde-bencode = "^0.2.3"
  ```

  Which was supposed to be the same code as:

  ```toml
  serde_bencode = "^0.2.3"
  ```

  So, no idea why it's working with an almost-exact copy of the original package.

  Anyway, `serde_bencode` is not maintained. So I'll will:

  - Publish a new version for `torrust-serde-bencode`.
  - Change this PR to use the new version `torrust-serde-bencode = "^0.2.4"`.

ACKs for top commit:
  josecelano:
    ACK 3550c44

Tree-SHA512: 374a95b6315079a4efd7ff9768cecfe21cfc0ee53c4f0cc5d326aa473c66cff5d2a09a91d0521831cc779651d31e5483b1ea8373eb773dd63b7e33ae34737868
@josecelano
Copy link
Member Author

This was fixed after updating serde_beconde crate to 0.2.4.

https://crates.io/crates/serde_bencode/0.2.4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Incorrect Behavior
Projects
Archived in project
Development

No branches or pull requests

1 participant