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

added new preformatted type to bencode entry #698

Merged
merged 3 commits into from May 6, 2016
Merged

Conversation

arvidn
Copy link
Owner

@arvidn arvidn commented May 4, 2016

to support carrying a verbatim copy of an already bencoded subtree. This is to support saving torrents in resume data and create_torrent based on existing torrents, in order to preseve key order

@sledgehammer999
Copy link
Contributor

How am I supposed to test this? I compiled and run it but it still mutates the hash. Do you want me to use some specific function?

@arvidn
Copy link
Owner Author

arvidn commented May 4, 2016

hm.. I thought it would fix that case. the internal entry hierarchy in create_torrent is supposed to preserve the info-dict from the torrent, when passing in a torrent_handle to it (with this patch). and bencode() is supposed to support include such pre-encoded sections.

would you mind stepping into the create_torrent constructor to make sure the metadata buffer is preserved verbatim?

the other place where this could be broken is in bencode(). You do use the bencode() function in libtorrent to encode the entry, right?

The unit test passes, so I'm a bit surprised it didn't work. I may have overlooked some code path that you're hitting.

@sledgehammer999
Copy link
Contributor

Sadly I cannot test further tonight. However I noticed a slight change from my previous test. I don't know if it is your patch or that I was using RC_1_0.

When the metadata are fetched from DHT we save them in our folder as hash.torrent and also creating hash.fastresume. Without your patch this hash was the mutated one. With your patch it is the initial hash. However, upon restart and loading those files libtorrent shows the mutated hash instead. Could the loading functions mutate it instead? We use this signature to load it: torrent_info(std::string const& filename, error_code& ec, int flags = 0);

@sledgehammer999
Copy link
Contributor

Well I just checked again. The initially saved torrent is mutated. The info fields are rearranged. ie the length field gets rearranged above the name field.

@arvidn
Copy link
Owner Author

arvidn commented May 5, 2016

basically any time the torrent gets turned into an entry, it will get reordered, so if your load path includes a decode step into an entry, that's a problem. However, there are plenty of alternatives for loading. bdecode_node is probably the best one. It's faster, uses less memory and doesn't reorder fields.

@sledgehammer999
Copy link
Contributor

As I said it is written mutated to disk(create_torrent -> bencode). Also bdecode_node isn't available in RC_1_0.

@codecov-io
Copy link

Current coverage is 68.34%

Merging #698 into RC_1_1 will increase coverage by +<.01%

  1. 14 files (not in diff) in src were modified. more
    • Misses -8
    • Hits +9
  2. 2 files (not in diff) in include/libtorrent were modified. more
    • Misses -3
    • Hits +3
  3. File src/torrent.cpp was modified. more
    • Misses -9
    • Partials 0
    • Hits +9
@@             RC_1_1       #698   diff @@
==========================================
  Files           264        264          
  Lines         42467      42504    +37   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits          29003      29046    +43   
+ Misses        13464      13458     -6   
  Partials          0          0          

Powered by Codecov. Last updated by f065cac...e6e24bf

@arvidn
Copy link
Owner Author

arvidn commented May 5, 2016

I added a unit test and discovered a bug in create_torrent. it should work now.

@sledgehammer999
Copy link
Contributor

Great! Now it seems to work!!! Please backport to RC_1_0 if possible.

TEST_CHECK(encode(e) == "i3e");
TEST_CHECK(decode(encode(e)) == e);
}
TORRENT_TEST(intergers)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean integers ? Same typo(?) in the following tests too.

@sledgehammer999
Copy link
Contributor

And reading up on bencode from wikipedia, I realize that we face this problem because of some broken torrent creator program/library. Dictionaries are supposed to be sorted alphabetically but that broken implementation doesn't do that. Too bad the metadata don't record the torrent creator.

arvidn and others added 3 commits May 5, 2016 17:12
…rbatim copy of an already bencoded subtree. This is to support saving torrents in resume data and create_torrent based on existing torrents, in order to preseve key order
@arvidn arvidn merged commit 9854366 into RC_1_1 May 6, 2016
@arvidn arvidn deleted the preformatted-entry-1.1 branch May 6, 2016 01:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants