Gocnak / Botnak

A Java-based IRC chat client with focus on Twitch.tv streams.
https://gocnak.github.io/Botnak
MIT License
67 stars 28 forks source link

Need a sanity check on nameface image sizes #91

Closed RaeveNoir closed 9 years ago

RaeveNoir commented 9 years ago

This image is 24752 x 13520, 51.5MB: http://www.spitzer.caltech.edu/uploaded_files/images/0006/3149/ssc2008-11a13.jpg

This breaks the botnak, at least for a few minutes. You may also note that it breaks most browsers as well, which might be closer to some other reason it causes issue.

I now have half a galaxy as a nameface, hooray!

NotAFile commented 9 years ago

If you decide to do threading maybe add a timeout, to stop people from saturating the streamers connection with a couple of large images. Should not be a issue for people playing offline, but nonetheless.

mikesmiffy128 commented 9 years ago

That or and just give up downloading after some number of megabytes.

NotAFile commented 9 years ago

that would be harder, as it would require changing the imageIO stuff, and would not protect against decompression bombs.

mikesmiffy128 commented 9 years ago

You don't have to change ImageIO, just add a custom Stream / Reader and wrap the input stream. Decompression bombs are why I changed "or" to "and."

Also, making downloads asynchronous is a good idea, but please no more threads please. Use NIO or something to handle concurrent IO stuff. Currently Botnak uses something like 200MB or RAM. That's not that much but streaming with x264 while running a modern game and having any significant background programs open can already chew up a fair bit and not everyone has a super-high end machine. I don't know if there are other possible optimizations, but not creating a new thread for absolutely everything might be one.

dr-kegel commented 9 years ago

This still leaves the problem of decompression bombs. 10000 x 10000 white png is 308 Kb or 83 Kb in as a .gif. (created with mspaint).

This should work against decompression bombs:

try(ImageInputStream in = ImageIO.createImageInputStream(resourceFile)){
    final Iterator<ImageReader> readers = ImageIO.getImageReaders(in);
    if (readers.hasNext()) {
        ImageReader reader = readers.next();
        try {
            reader.setInput(in);
            return new Dimension(reader.getWidth(0), reader.getHeight(0));
        } finally {
            reader.dispose();
        }
    }
} 

Source: StackOverflow

Sry: accidentally clicked the wrong button. reopened. :(

Gocnak commented 9 years ago

Botnak uses something like 200 MB of RAM.

This is heap size. I've run two different memory profile debuggers that tell me Botnak only really uses upwards of 100 MB of RAM, and that's after spamming full sounds and whatnot, where Botnak (in task manager) would have about 1GB of RAM being used.

zkltw8j

Creating a singleton thread isn't how I would go about it either, I would probably make an ExecutorService in the IRCBot class (see the Heartbeat class and MessageQueue class for examples). Not only will it handle the Image downloads on this but also the API Requests that the bot needs to do as well. These handle creating and closing the threads after the runnable has run its course. But NIO looks interesting if you can provide an example of how it's used.

As for this issue, I'll be testing with what kegel posted. Another solution I was looking at was sending the "HEAD" request property to a HttpConnection.

mikesmiffy128 commented 9 years ago

In that case, maybe I just need to pass Java -Xmx... though actually if the RAM use is so much more than the heap size there is probably some other overhead. My guess would be that every thread having a stack doesn't really help.

For HTTP requests you might actually want to look into AsyncHttpClient, which is event based and handles threads for you. It also has a fluent API for configuring all the headers and getting response codes and all that stuff. It has a couple of transitive dependencies though so you'd probably then want to start using a proper build tool like Gradle or I guess Maven but really why would you use Maven?

dr-kegel commented 9 years ago

@Gocnak: HEAD does not help, because it can be faked. It might be more difficult to fake head than to upload decompression bombs, but it is still possible.

But NIO looks interesting if you can provide an example of how it's used.

Ten months ago I wrote an irc client (well I started, but I abandoned it) with NIO & event based. The class for communication with the server is here: https://github.com/dr-kegel/kircbot/blob/master/src/kircbot/ConnectionClient.java It is currently not usable, if I remember correctly parsing messages was not even implemented. Maybe it helps with nio.

(Hopefully pressing the right button this time m( )

mikesmiffy128 commented 9 years ago

If we're talking about doing IRC with NIO, you could play about with JerkLib. It'll probably need substantial modification and it's pretty badly documented, and the licensing is ambiguous, but it could act as a starting point or guide or something. I actually wanted to start modifying Botnak to use it quite a while ago but I gave up after a while because I couldn't be bothered and there wasn't a point.

EDIT: there's also irc-api, which is pretty decent but really bloated and overcomplicated if I correctly remember my experiences trying to integrate it with Twitch.

Gocnak commented 9 years ago

So the cap is going to be 5000 pixels wide and high. Any objections (bigger/smaller and why) let me know in here.

NotAFile commented 9 years ago

if it is configurable, 5000 is totally fine. If not, I think 5000 still might be to much for some streamers machines, and I'm having a hard time thinking of reasons why anyone would want to upload a 250 megapixel image.

Is the file size limited the in any way? would be a shame if botnak spends 3mins downloading an Image, and then realizes it is to big.

Gocnak commented 9 years ago

It checks size before downloading it. It'll be configurable, don't worry.