Submit log pictures

[following5]

Submitting log pictures via OKAPI would be a great enhancement.

The are some differences in image postprocessing by OCPL and OCDE, with the OCDE processing afaik being more refined, e.g. not shrinking pictures which have a small file size, and resolving EXIF rotation if the Imagick PHP extension is available. I think it should be fine to adopt the OCDE implementation for all nodes.

Maximum image dimensions and file size threshold need to be configured, as well as picture upload path and maybe allowed file types.

[wrygiel]

+1

How external developers will know about restrictions? Extra fields in apisrv/installation?

[following5]

They need not to know about size limits for the stored images, because pictures will automatically be shrunk. The file formats accepted by OKAPI can be defined to be the same for all nodes (in the simplest case, just JPEG), so this also is no issue.

Another limit are the Megapixels of the uploaded original image, because they consume lots of memory while rotating/shrinking. This is difficult to handle, because the memory usage depends on the PHP graphics library implementation, which may be updated between any two PHP script executions. OC.de just puts a conservative limit on the file size; the shrink/rotate script may still crash e.g. for a 40 MP blue-sky picture. (I am not aware that his ever happend in the one and a half year since implementation.) It needs some thought and some tests to decide how to handle this in OKAPI.

[wrygiel]

Yes, dimentions (and formats) are not so important, but size is. If we don't restrict it in any way, this will ultimately lead to developers sending us 30+ MB files (which will then be shrunk to 100 kB).

[following5]

Ok. OCDE has a settings for this (currently 6 MB), OCPL a constant of 3.5 MB.

[following5]

Btw, the pixel dimensions of an image can be retrieved without processing the whole file; it is in the header. As the temporarily uploaded file size doesn't matter and the real limit is PHP memory, a megapixel limit would be the most effective soution on the server side. Also for client apps which directly access the device's camera and can chose an MP limit upfront. But for anything else, the file size limit will be easier to handle ...

[wrygiel]

It's also easier on the user's bandwidth.

How about adding a set of rcmd_* and max_* fields to the apisrv/installation method?

[following5]

I don't know what rcmd_ is for. Adding these site properties to apisrv/installation should be fine.

What about these methods:

logs/add_image to add an image to a log (title, image data, spoiler flag) logs/edit_image to change the title and/or spoiler flag of an image logs/remove_image to remove an image from a log

All three operations currently are only allowed for the log submitter. There is an approved feature request at OC.de to allow cache owners to set the spoiler flag, too. We may also add an optional images parameter to logs/submit, to directly include images

Later there could be similar methods for the log entries (edit_log, remove_log).

[wrygiel]

So far, looks fine.

In regard to editing - see http://opencaching.pl/okapi/services/caches/save_personal_notes.html - in particular the "old_value" and "new_value" parameters. I have used them to avoid overwriting data by mistake. You might want to similar parameters too, espiecially it you intend to allow editing log messages.

I don't know what rcmd_ is for.

E.g. "recommended image width - there's a big chance we'll downsize your image if you submit one which doesn't fit", etc.

[following5]

I will try to implement this asap, it is very important for logging "Safari caches" at OC.de. However this may still take some weeks or months, as there are some even more important issues at OCDE.

[wrygiel]

This is a major feature, and there are lots of border cases to consider. I also guess we will need talk a little more about the method interfaces. Let's make a separate branch for this. Perhaps it's best to start with xml documentation files first, and add php implementations later on?

[following5]

okay

[following5]

On any log picture transaction, cache_logs.last_modified must be updated, which triggers an update of cache_logs.okapi_syncbase.

[following5]

On any log picture transaction, cache_logs.last_modified must be updated, which triggers an update of cache_logs.okapi_syncbase.

Actually, OCDE code does this already by triggers. In OCPL code it is not clear yet how this wil be implemented (see OCPL issue #341), but OKAPI may simply update cache_logs.okapi_syncbase on picture changes.

[following5]

For OCPL, OKAPI also must update cache_logs.picturescount when adding or removing log pictures (which updates okapi_syncbase).

[following5]

I have opened a new branch feature/logimages for this, and proposed an interface definition. Can you have a look at it?

Alternatively, the new methods could be placed in a new "services/images" category, and reused in the future for editing geocache images. The edit and remove API should be identical; just on submitting it needs to be determined if it's to be added to a geocache or a log entry.

[wrygiel]

I have submitted some proposals of mine.

The biggest change is the handling of the new and old values of the text fields. I have updated the docs to fit the flow used in the save_personal_notes previously. It makes it harder to developers to use, but also makes sure that all of them display the current caption correctly, before allowing the user to change it. What do you think?

Alternatively, the new methods could be placed in a new "services/images" category, and reused in the future for editing geocache images. The edit and remove API should be identical; just on submitting it needs to be determined if it's to be added to a geocache or a log entry.

Huh, it's an interesting idea... But I tend to believe it would be safer to keep them in separate methods, because these are separate database entities, and may have separate properties in the future. We can still make one inherit some (or all) of the other's parameters (as we do in search methods).

[wrygiel]

and may have separate properties in the future

One of these will be the log_id :)

[wrygiel]

PS: Because captions are usually very short and not really important, I would also be fine with dropping the old_value/new_value verification scheme. This verification seems quite important in cases like log entry messages or personal notes, but perhaps could be dropped in case of the captions. I will leave this decision to you.

[following5]

But I tend to believe it would be safer to keep them in separate methods, because these are separate database entities, and may have separate properties in the future.

The OC sites store them all in one table, but I agree that there can be differences. Actually there is already a difference, that I missed: geocache images have an additional flag 'display separately from description'.

I have dropped the old/new caption thing.

The position option that you added is currently not supported by any OC site. OCPL has an ordering field pictures.seq, but that's only used for geocache images. It is always = 1 for log images! But I like that idea and can try to implement it for OCDE.

[following5]

I just noticed that OCPL has constraints for both the long side and the short side of images. And the shrinking code is mad: It will first shrink to the max long side (if necessary), and then shrink (again) to the max short side (if necessary). Wtf ...

I don't think that we need to copy this long/short limits but can stay with the long-side limit. It's already complicated enough, because at both OCDE and OCPL there is an additional recommended max file size, which triggers shrinking (before the dimension limits are tested).

If you think otherwise, please let me know.

[following5]

It's even more complicated: Shrinking is triggered only if both ar exceeded, the rcmd max filesize and the rcmd max dimensions. This has the nice effect that panoramic pics can exceed the dimension limit - I use that often myself.

But I think we may simplify that and just set two limits:

• maximum accepted upload file size; larger images are rejected,
• maximum storable square dimension (width * height); larger images are shrunk.

This will produce some odd image dimensions instead of e.g. 1024 * Y, or X * 768, but who cares?

[wrygiel]

I think we can implement our own way of doing this.

Accepting all X*Y<LIMIT wouldn't be always okay (e.g. if X=1 then Y could be huge). But the "recommended" in rcmd_max_long_side doesn't need to imply "absolutely required for not shrinking". We could say that OKAPI may still skip shrinking if this limit is exceeded and some other (undocumented) heuristics are satisfied (e.g. when panoramic pics are detected).

[wrygiel]

Or, we can use your "maximum area" and say the OKAPI may still shrink your image if some other (undocumented) heuristics are satisfied (e.g. X=1). Both approaches seem okay to me.

[following5]

Accepting all X*Y<LIMIT wouldn't be always okay (e.g. if X=1 then Y could be huge).

All OC sites allow that. :-S

I will try the square dimensions thing, because it gives developers a good grip on what will (very probably) be accepted.

Btw, it's fun to do the 'logs/add' implementation. :) Never implemented a new OKAPI method before.

[wrygiel]

All OC sites allow that. :-S

We also can allow that for starters ;) I'm just saying this can change in the future, and we should let the developers know that in some way (e.g. by simply saying "OKAPI may still shrink it if it wants").

[following5]

All OC sites allow that. :-S

We also can allow that for starters ;)

You misunderstood the smiley. ;) I was startled that this is possible.

[following5]

I am not sure what the image_max_upload_size limit means. As far as I understand it, the image parameter is just an ordinary (base64) string and not a file upload, which would pop up in the PHP $_FILES array. For the latter one, image_max_upload_size might reflect the PHP upload_max_filesize setting. But for an ordinary parameter, there is only the post_max_size setting which limits the size of the whole post request.

So I think image_max_upload_size may be calculated as ini_get('post_max_size') minus some estimated upper limit for the size of the rest of the query.

Also, an upload total pixel count limit would be nice, to avoid crashing the image processing due to lack of memory. But this limit would be either very dynamic or restrictive (because memory would be wasted by a conservative setting). Therefore I prefer to have this limit undocumented.

[following5]

After some more tests and thoughts: I thing that image_max_upload_size does not make sense at all. It would make sense only if it was enforced by the web server, i.e. by setting apache's LimitRequestBody. But usually this limit is not set, and also there is no way to query it from a PHP script. So we would need a redundant OKAPI setting for that, which could go out of sync with the Apache setting.

The OC code has an upload limit, but that is inserted into upload forms and can be checked by the browser. OKAPI image uploads usually will not originate from browser forms.

And finally, if an image has already been transfered, it does not make sense that the 'add' methods enforces a size limit. Why throw away good data? Instead, it makes sense that OKAPI checks if the image's pixel count is processable - see above.

It's up to the developer and the user what upload bandwith they are willing to use. For the OC nodes, the image transfers are peanuts, even if all image files are 100 MB in size.

Therefore, for now I am going to remove image_max_upload_size from the 'installation' method. If you find some useful meaning for this setting that I missed, I may be re-added.

[wrygiel]

For the OC nodes, the image transfers are peanuts, even if all image files are 100 MB in size.

How sure are you about this? If we completely drop this parameter, there's no way back. It might be a better idea to keep it, just for the sake of having a possibility of enforcing it in the future.

[following5]

For the OC nodes, the image transfers are peanuts, even if all image files are 100 MB in size.

How sure are you about this?

For running an OC site, you need a server with full root acces. Such servers come with huge free traffic or flatrates today in developed countries. Ok, if someone wants to run an OC site in Senegal or Afghanistan, this may be an issue. ;)

It might be a better idea to keep it, just for the sake of having a possibility of enforcing it in the future.

What possibility of enforcing? Is there any way how a PHP script can co2ntrol the size of POST request that the webserver accepts?

[wrygiel]

What possibility of enforcing? Is there any way how a PHP script can co2ntrol the size of POST request that the webserver accepts?

By "forcing" I mean "forcing the developer to compress the image properly if he wants to publish it via OKAPI".

This won't protect OC from DoS attacks, of course. But it protects OC from bad developers who eat server resources without having any good reason to. I don't like lazy developers. They're actions influence both users and OC servers.

Such servers come with huge free traffic

Network traffic is just one of the resources to think about. The first resource to suffer would probably be the number of Apache connections. Or disk space for storing temporary files. E.g. when 10 users are uploading 10 images each, all in parallel, over a poor network connection, and each of these images is 25MB big, then the server will need to allocate 10*10*25 MB for them.

[following5]

And how will you force the developer to do that? By rejecting the image after upload, so that there is a "disciplinary" effect? Or by changing the web server settings (which can be out of control of the OKAPI project?)

There can be applications where no means for shrinking images are available, therefore I see it as a nice feature to accept even huge images. So for OC.de I would avoid such a limit. The traffic doesn't matter, the number of connections neither (there are just a dozen image uploads per day), and the purely theoretical 2,5 GB of temp space would be peanuts, too.

[wrygiel]

Currently OKAPI can do that only via the first method (the one you call a disciplinary effect).

So for OC.de I would avoid such a limit.

Okay, but I'm not so sure about OCPL (and possibly others). Currently OCPL is struggling with disk space issues.

[wrygiel]

@opencaching/opencaching-pl-lead-programmers - please comment.

• Will you be okay with some external OKAPI applications submitting uncompressed camera uploads?

or

• Would you prefer OKAPI to enforce upload size limits (so that external developers are forced to shrink camera images client-side, before uploading them).

[following5]

Currently OKAPI can do that only via the first method (the one you call a disciplinary effect).

Ok. This was the way I implemented it first, then I discarded it because I thought: Is bogus to discard successfully uploaded images. But I am fine with re-adding it, so that OCPL can discard large images (> X megabytes) instead of downscaling them.

Note to the OCPL developers: We are talking about the size of temporary data while uploading. Before storing to images/upload, OKAPI will shrink all images that are too large, i.e. enforce a configurable numer-of-pixels limit.

[wrygiel]

It might seem weird, but it's entirely sufficient for our purposes :)

[wrygiel]

Just now I have realized that we're NOT using multipart/form-data encoding, so the clients will not be able to send "really big" files anyway. I can now understand why you found some of my previous arguments weird, and why you're parsing php.ini variables! (didn't catch that yesterday)