search

OriginProtocol / origin

Monorepo for our developer tools and decentralized marketplace application
https://www.originprotocol.com/developers
MIT License
652 stars 196 forks source link

Message Object Size #487

Open micahalcorn opened 6 years ago

micahalcorn commented 6 years ago

As recommended by @crazybuster, we should validate the size of the message object before storing it in OrbitDB since we are now supporting the inclusion of image data. I'm assuming that this should happen at the DApp layer since this is where we are doing the same for listings?

franckc commented 6 years ago

@tomlinton I believe on the IPFS node used for listings, upload limit is set to 2MB. Is it the same on the messaging IPFS node ?

franckc commented 6 years ago

Sort of related to #489

tomlinton commented 6 years ago

It is the same for both at the moment I believe, it is enforced by the nginx proxy running in front of the IPFS nodes. The values might need to be changed to make them more suitable (at least for listings). The size limit needs to be enforced at the nginx proxy level because it will be easily circumvented in the DApp, but the DApp should also enforce it to provide better feedback/user experience.

I'm not totally across this so I might be wrong. I'm going to be looking into it over the next few days.

micahalcorn commented 5 years ago

This has become more of a priority because naive actors will upload images that will lock up their own browsers and those of their recipients. A bad actor could easily spam and ruin every messaging-enabled DApp user's experience.

tomlinton commented 5 years ago

Because nginx limits the size you can't drop anything larger than 2mb into a message or it drops the connection. The user experience isn't great because it looks like the image was successfully sent and there are no warnings about any failures, but at least a malicious actor shouldn't be able to spam huge images.

Given that, do we still need the p0 tag? There are a lot of improvements to be made here, but I'm not sure it is still blocking.

DanielVF commented 5 years ago

It's easy to increase nginx's limits.

tomlinton commented 5 years ago

For someone who wants to send a bunch of huge images? As far as I know there is no way to bypass the nginx setting.

DanielVF commented 5 years ago

We are talking about the maximum POST upload size in nginx?

client_max_body_size?

https://nginx.org/en/docs/http/ngx_http_core_module.html#client_max_body_size

DanielVF commented 5 years ago

That said, I think a 2mb limit is a good sane limit for image uploads.

tomlinton commented 5 years ago

You are right, it is easy to bypass. So it should only help with accidental cases.

micahalcorn commented 5 years ago

@tomlinton @DanielVF I'll remove the p0 from this and create a separate issue for the case where someone circumvents the persistence layer and injects larger data.

wanderingstan commented 5 years ago

On the UX side, we really should be resizing images in the browser, so even naive users don't have to think about file sizes. E.g. they could upload a 10GB image and have it automatically scaled down to under 2MB.

We already do this for the avatar image (via the npm package we're using).

See the ancient issue #44 , especially HTML5 image uploading

joshfraser commented 5 years ago

+1 for in-browser resizing