squidowl / halloy

IRC application written in Rust
GNU General Public License v3.0
1.21k stars 43 forks source link

DCC Send fails on filenames with whitespace #318

Open dahlend opened 3 months ago

dahlend commented 3 months ago

These lines linked below, if the filename contains a space such as: "DCC SEND Example file name.txt (127.0.0.1)" The iterator spits out "Example" as the filename, then attempts to parse the rest as an IP.

https://github.com/squidowl/halloy/blob/698eef88b17832881987f2da16faad7e4469a71d/data/src/dcc.rs#L69-L70

Also on line 11 I'm unsure but you may want to throw a .trim() to strip white space before the iterator call?

I may take a stab at putting a PR in for this, but I'm a little unclear of all the surrounding logic still. No promises.

Good stuff though, nicely written!

casperstorm commented 3 months ago

@dahlend feel free to take a stab at it! We will assist you if you run into any issues. Thanks for the bug report, great to catch this before we ship it.

tarkah commented 3 months ago

Thanks for the report @dahlend. Yeah I didn't attempt to address this initially. Halloy sanitizes file names before trying to send them, so I assume this is something sent from another client which doesn't (unfortunate).

Considering in your example the filename isn't quoted, then it's going to come down to collecting the iterator into a vec (unless we have an exact sized iterator) to see how many items it has and take appropriate action off that info.

After checking size, if it's > 4 or 5 we know the filename probably has spaces. We should instead process the items in reverse order, so size, port, host then the rest is filename.

If it's a reverse send request, we may have the token at the very end and port is 0, so we will need to account for that.

dahlend commented 3 months ago

These are bots I am interfacing with which send files like this (and have a single space char before the " DCC SEND", which is why i mention trim()). Gotta love parsing text from the internet.

@tarkah Yeah that was roughly the outline I had also considered.

I am not sure I have the cycles for a bit.

tarkah commented 3 months ago

So the space you're seeing before DCC is probably the 0x01 byte which denotes this is a ctcp message, which is how we know to parse the PRIVMSG as such. We discard this so we're good on this front 👍

I can take a look but it might be a little bit before I get to it as well.

ViktorEvil commented 3 months ago

I have had issues with bots sending files and nothing arriving my end as well, some bots send files fine though

casperstorm commented 3 months ago

I have had issues with bots sending files and nothing arriving my end as well, some bots send files fine though

Is this a different issue than filenames with whitespace? If so feel free to open a new issue with some information around the issue :)