imagemagick crashes

d4gurasu

"Suggesting that someone does not upload huge images is our typical advice, so much so that we have an article dedicated to such issues:"

Right, so much so there are clearly many people who's original preference is to use larger images and who would benefit by some solution like this.

I saw your referenced i.php lines before you edited that out.
That seems useful. It's not set by default, it's not in the "security" tab and there's a warning on the option making it so one would be inclined not to set it. I don't quite understand the warning about memory usage yet though. I bet many public zp servers are vulnerable to this problem to some extent. Your demo is.

For serializing I'm thinking now along the lines of waiting to get a semaphor or mutex lock in i.php. That would allow the on-the-fly cacher to still work. I don't know how that plays with timeout issues, but not worse than jamming up the system with too many processes. OK, so technically, that's IPC.

d4gurasu

well this makes things a thousand times easier,
some commenter on the php manual page:

"On the Apache webserver, ... any semaphores got and acquired by a script and not released and removed will still be automatically cleaned up by the PHP interpreter each time the script terminates."

but unless there's slightly more to it, it might still not cleanup after kill -9 or crashes and if other servers don't clean up that's a problem. Of course this is the pain with locks.

and this is bad, from the php manual:
"Support for this functions are not enabled by default. To enable System V semaphore support compile PHP with the option --enable-sysvsem ."

of course I have no idea how or if this works on windows etc.

kagutsuchi

I deleted the `i.php` lines because they did not do what I thought they did. That option would not prevent flooding requests to `i.php`. I'm not sure if this is an issue for GD, but for Imagick, `MAGICK_THROTTLE` could counter this.

System V semaphores are not available on Windows. For PHP, you will be far better off using a file lock. Check out `fopen` with the `x` option. Keep in mind that, as you said, you will have to implement cleaning up stale locks in case the process is killed. (This is actually fairly trivial if you write the PID to the lock file.)

d4gurasu

trivial unless you want control of number of allowed processes, then maybe slightly harder, but still probably not too bad.
Is the x option atomic? The manual doesn't exactly say it is.

kagutsuchi

Yes. The underlying `O_EXCL` flag to the C lib guarantees atomicity.

d4gurasu

Thanks. I'm not really impressed with the flooding solution. It seems like it will not reduce the number of httpd processes that one user can sit on at a time, maybe won't help memory much (I'll try and see), they can still use cpu to the max you ever want it used for image creation at least, but for much longer, and can flood out precaching requests from others who are also governed by the same communal (from the sounds of it) limit, and they can still fill up hard drive space nearly forever until other precache requests cannot be fulfilled for lack of space.

I think it's a serious issue but comes with the territory of allowing large user demand-able resource allocation I guess. I should start a new thread but I'm not sure I should have mentioned in a thread at all. I think the right solution is to have an option to dissallow all external access to i.php, and that should be the default. When the option is set it should also be true that no themes etc pass through limitlessly arbitrary user requests to the internal interface.

Of course if the user is not allowed to make arbitrary requests, then the list of requests they can make is predetermined (even if in a complex way) and precachable. There could also be higher-permission users (bloggers or whatever) who are allowed to make arbitrary requests though.

You actually had me mostly convinced that the secure image processor option should work. I thought the fix might just be to make it more default/noticable/less-discouraged. I think it's intent was to protect content though, not system resources, but I still thought it would work.

d4gurasu

Wow.. this works awesomely .. and with about 10 lines of code, probably 5 that actually do anyting. Once the processes aren't piling up on each other, it even works pretty fast. And it could be used with OTF either enabled or disabled, but I left it enabled. Too bad the devs made me wade through all this just so I could learn enough to figure out where to put 10 philosophically obvious lines of code. In less time than you all speant arguing and writing blog posts about how to not use big images, you could have just fixed this.

Of course I rewrote the whole architecture of zp in 10 lines (but I did implement IPC) </grin>

btw I didn't use fopen. It's atomic, but writing the PID into the file is not atomic and while that can be dealt with. I took an easier way, but there's an obvious better way than that or mine with slightly more work.

Now to worry about that security threat.. well this already helps it quite a bit at least.

acrylian

I didn't follow the discussion in details but if you managed something useful, please share the code…:-)

d4gurasu

I'll sumarize for now...

Only let 1 (or configurable number) instances of the image processor run at a time.

The end. (but that was the request, and it's what works)

I'll have more time again later.