handroll bencode #46
handroll bencode #46
Conversation
mistermoe
requested review from
alecthomas,
KendallWeihe,
mihai-chiorean,
tomdaffurn and
wesbillman
as code owners
| func Marshal(input any) ([]byte, error) { | ||
| switch v := input.(type) { | ||
| case string: | ||
| encoded := fmt.Sprintf("%d:%s", len(v), v) |
There was a problem hiding this comment.
Is this length in bytes or in UTF-8 codepoints?
There was a problem hiding this comment.
according to the "spec", just string length
There was a problem hiding this comment.
Disturbingly ambiguous!
There was a problem hiding this comment.
Maybe the format doesn't support UTF-8 at all...
dids/diddht/bencode/bencode.go
Outdated
|
|
||
| return []byte(encoded), nil | ||
| case int, int8, int16, int32, int64, uint, uint8, uint16, uint32, uint64: | ||
| encoded := fmt.Sprintf("%s%d%s", string(IntegerPrefix), v, string(EndSuffix)) |
There was a problem hiding this comment.
You can use %c for characters if you prefer.
There was a problem hiding this comment.
ah yeah much cleaner! done: b5bec44
| // Marshal encodes the given input into a Bencode formatted byte array. | ||
| // More information about Bencode can be found at: | ||
| // https://wiki.theory.org/BitTorrentSpecification#Bencoding | ||
| func Marshal(input any) ([]byte, error) { |
There was a problem hiding this comment.
I assume this is not supposed to be able to encode structs? Might be good to mention that, as typically in Go, Marshal/Unmarshal can.
If you do want to do that, you'll have to use reflection, which you can probably just get away with in the default block.
There was a problem hiding this comment.
for the purpose we need it, no, doesn't need to do structs
There was a problem hiding this comment.
good call. no need for encoding structs right now so went ahead and added a comment d23f683
though would be cool to support that so i'll create an issue
| return nil, err | ||
| } | ||
|
|
||
| b = append(b, encoded...) |
There was a problem hiding this comment.
I'm not sure how efficient you want this to be, but if you pass a singlebytes.Buffer around it will likely be significantly faster because you won't be allocating lots of tiny byte slices, instead you'll be writing to a single buffer.
If performance isn't a real concern than ignore.
There was a problem hiding this comment.
If you do do this you can then use fmt.Fprintf and fmt.Fscanf` directly on the buffer.
There was a problem hiding this comment.
great idea. performance isn't a big deal right now given that we're only ever marshaling a map with two fields that have primitive values.
regardless, what you're suggesting is a better approach nonetheless. going to create an issue to track your suggestion so someone can tackle it
Co-Authored-By: Alec Thomas <alec@swapoff.org>
| // Marshal encodes the given input into a Bencode formatted byte array. | ||
| // More information about Bencode can be found at: | ||
| // https://wiki.theory.org/BitTorrentSpecification#Bencoding | ||
| func Marshal(input any) ([]byte, error) { |
There was a problem hiding this comment.
for the purpose we need it, no, doesn't need to do structs
Co-Authored-By: Alec Thomas <alec@swapoff.org>
* main: handroll bencode (#46) Fix: Added BEP44 decode to DHT resolve
Overview
First-pass run-through at handrolling
bencodeto replace usage of"github.com/anacrolix/torrent/bencode". No qualms with the lib itself it just happens to be a full fledged bittorrent client packageNote
What's in this PR at the very least fulfills the needs of
did:dhtRationale
The lib we're depending on adds 627 dependencies to our dependency graph. removing it brought the dep count down to 15. I recently had to handroll bencode in kotlin and have to do it again in dart so it was fresh on my mind. Figured i might as well knock it out here before forgetting all about it.
Warning
I'm not sure my implementation is idiomatic go. i looked at the anacrolix implementation and it looks pretty involved. I kept the API surface that we're using identical to the 3rd party dep. my variable names are kaka bc i wrote most of this during meetings. definitely open to renaming stuff if the names are jarring