-
Notifications
You must be signed in to change notification settings - Fork 19
Refine specification and validation of file upload sizes. #41
Comments
We have a couple of scenarios here:
My thought for the 1 MB limit was mainly for the hub I think, since that could expand up to 2+ MB after it runs through everything. Better than having something be accepted as 2+ MB and start slowing down the processing on other services. I'm wondering if services should generally cap at a higher rate like 3 MB just to account for being beyond 1 MB? @kid-icarus @kirbysayshi @brycebaril @taotetek |
Also feel free to set up issues over here https://github.com/revisitors/revisit.link since that is where the spec is |
The limit can also be considered advisory, or a minimum limit that a service is expected to have. That way if they feel they can confidently process images up to 10 MB they can, but if their service has issues with images that are only 250 KB, maybe they should figure out how to get it up to par. This relies on the protocol letting any node at any time enforce its (>= 1 MB) limit. As long as we're comfortable letting services abort if given something past their own limit, and the hub itself enforces the minimum limit, maybe we don't need to worry too much about this. |
@ednapiranha does the hub enforce the 1MB limit only when accepting the initial image upload, or also when returning the finished play to the user/ws? Seems like the validator enforces a 1MB image limit from the service? Am I reading that properly? |
Yeah it looks my validation assumes input and output at a 1 MB limit (unless you override with a new limit https://github.com/revisitors/node-revisit-validator/blob/master/index.js#L15 The hub doesn't actually use the vaildator for checking input - that's only a pre-merge service to verify nothing out of the base specification is out of order. But yes, the hub only enforces on incoming upload, doesn't verify outgoing. |
At this point the spec states:
'Any content passed through http://hub.revisit.link cannot exceed 1 MB per data URI. But data coming from distributed services may increase to a greater size, so ensure your service endpoint can handle a size averaging between 1-3 MB.'
#36 (comment) states some confusion in the spec regarding a maximum transmission size of the overall payload, and various components of the payload.
What do you think about mocking up different payloads and checking for a 413 status code and the original object as posted to the service returned (noop)? That could be added to validate.revisit.link.
I think the approach of asking all services to simply 413 and noop on a the post of a payload over 2MB is reasonable. It simplifies service development in that a payload will simply be passed through the chain in the event that it at some point it exceeds the maximum allowed sized.
Any thoughts?
The text was updated successfully, but these errors were encountered: