# Serializing exceptions - Thread: https://externals.io/message/85416 - Original archive: https://news-web.php.net/php.internals/85416 - Started by Stanislav Malyshev on 2015-03-23T05:21:49+00:00 - Messages: 33 - Page 1 of 2: messages 1 to 25 (25 per page, add `?page=N` to the URL) - Next page: https://externals.io/message/85416.md?page=2 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #85416 — Stanislav Malyshev — 2015-03-23T05:21:49+00:00 URL: https://externals.io/message/85416#85416 Hi! Looking into some issue, I've discovered that, to my surprise, Exceptions are serializable. Except that it doesn't always work of course (e.g. see http://stackoverflow.com/q/9747813/214196) because exceptions contain backtraces, and those can contain non-serializable objects. So in reality, you never know if you can serialize it or not. So, I wonder - would it be ok to make exceptions not serializable at all? I think that would prevent major WTF factor when people try it and it randomly fails. Thoughts? -- Stas Malyshev smalyshev@gmail.com --- ## #85417 — Juan Basso — 2015-03-23T05:31:34+00:00 URL: https://externals.io/message/85416#85417 · In reply to: #85416 Maybe you can implement the __sleep method and just return the documented attributes (message, code, file, line). Not perfect, but probably more useful than throw an exception, specially if the exception is something that is the attribute of some object that is being serialized. Juan Basso > Hi! > > Looking into some issue, I've discovered that, to my surprise, > Exceptions are serializable. Except that it doesn't always work of > course (e.g. see http://stackoverflow.com/q/9747813/214196) because > exceptions contain backtraces, and those can contain non-serializable > objects. So in reality, you never know if you can serialize it or not. > > So, I wonder - would it be ok to make exceptions not serializable at > all? I think that would prevent major WTF factor when people try it and > it randomly fails. > > Thoughts? > -- > Stas Malyshev > smalyshev@gmail.com --- ## #85418 — Pierre Joye — 2015-03-23T05:57:13+00:00 URL: https://externals.io/message/85416#85418 · In reply to: #85417 > Maybe you can implement the __sleep method and just return the documented > attributes (message, code, file, line). > > Not perfect, but probably more useful than throw an exception, specially if > the exception is something that is the attribute of some object that is > being serialized. Well, can someone explain me why some code would need to serialize exception in the 1st place? Cheers, -- Pierre @pierrejoye | http://www.libgd.org --- ## #85419 — Robert Stoll — 2015-03-23T06:11:46+00:00 URL: https://externals.io/message/85416#85419 · In reply to: #85418 > -----Ursprüngliche Nachricht----- > Von: Pierre Joye [mailto:pierre.php@gmail.com] > Gesendet: Montag, 23. März 2015 06:57 > An: Juan Basso > Cc: Stanislav Malyshev; PHP Internals > Betreff: Re: [PHP-DEV] Serializing exceptions > > > > […] > > Well, can someone explain me why some code would need to serialize exception in the 1st place? > > Cheers, > -- > Pierre > > @pierrejoye | http://www.libgd.org > > -- > Maybe some user defined mechanism for remote exception handling --- ## #85420 — Dennis Birkholz — 2015-03-23T06:21:22+00:00 URL: https://externals.io/message/85416#85420 · In reply to: #85416 Hi, Am 23.03.2015 um 06:21 schrieb Stanislav Malyshev: > Looking into some issue, I've discovered that, to my surprise, > Exceptions are serializable. Except that it doesn't always work of > course (e.g. see http://stackoverflow.com/q/9747813/214196) because > exceptions contain backtraces, and those can contain non-serializable > objects. So in reality, you never know if you can serialize it or not. > > So, I wonder - would it be ok to make exceptions not serializable at > all? I think that would prevent major WTF factor when people try it and > it randomly fails. > > Thoughts? I think we should discuss if (un)serialization is a first-class operation in the language and if so, we should try to make everything serializable. Currently, we introduce more and more unserializable language features (closures, anonymous classes) which makes it more and more dangerous to serialize something that could contain any of these (or any of a number of non-serializable internal classes: mysqli/pdo, etc.) If we don't want serialization as a first-class operation, we should make objects not serializable by default unless they implement serializable or __sleep()/__wakeup() and add an is_serializable() method to check if an object is serializable. But currently serialization gets more and more unreliable/prone to runtime errors. Greets Dennis --- ## #85421 — Stanislav Malyshev — 2015-03-23T06:43:47+00:00 URL: https://externals.io/message/85416#85421 · In reply to: #85420 Hi! > I think we should discuss if (un)serialization is a first-class > operation in the language and if so, we should try to make everything > serializable. Currently, we introduce more and more unserializable I don't think we can, not unless we can serialize PHP code (like Java can have JARs with bytecode). Otherwise, something like closure is inherently not serializable. Moreover, something resource-bound is not serializable either since it depends on something external. > If we don't want serialization as a first-class operation, we should I'm not sure what you mean by "first-class operation". We certainly want most objects be serializable, and they are. But some are not, and would never be. The question is - is Exception one of those? I think yes, but I'd like to hear if somebody says otherwise. > make objects not serializable by default unless they implement > serializable or __sleep()/__wakeup() and add an is_serializable() method > to check if an object is serializable. But currently serialization gets > more and more unreliable/prone to runtime errors. That's because implementers of classes are not careful to account for serialization. However, I don't think we need any drastic changes here. -- Stas Malyshev smalyshev@gmail.com --- ## #85422 — Stanislav Malyshev — 2015-03-23T06:45:02+00:00 URL: https://externals.io/message/85416#85422 · In reply to: #85417 Hi! > Maybe you can implement the __sleep method and just return the > documented attributes (message, code, file, line). That would be an option, but before going there, my question is - does anybody need it, really? -- Stas Malyshev smalyshev@gmail.com --- ## #85423 — Christian Stoller — 2015-03-23T07:41:36+00:00 URL: https://externals.io/message/85416#85423 · In reply to: #85422 From: Stanislav Malyshev [mailto:smalyshev@gmail.com], Sent: Monday, March 23, 2015 7:45 AM > Hi! > > > […] > >That would be an option, but before going there, my question is - does >anybody need it, really? > Hi. Symfony2 stores an `AuthenticationException` in the session, when login credentials are invalid. I think, exceptions should be serializable. One could strip the objects from the backtrace and document it like that. Christian --- ## #85424 — Rowan Collins — 2015-03-23T11:36:05+00:00 URL: https://externals.io/message/85416#85424 · In reply to: #85416 Stanislav Malyshev wrote on 23/03/2015 05:21: > Hi! > > Looking into some issue, I've discovered that, to my surprise, > Exceptions are serializable. Except that it doesn't always work of > course (e.g. see http://stackoverflow.com/q/9747813/214196) because > exceptions contain backtraces, and those can contain non-serializable > objects. So in reality, you never know if you can serialize it or not. > > So, I wonder - would it be ok to make exceptions not serializable at > all? I think that would prevent major WTF factor when people try it and > it randomly fails. > > Thoughts? I came upon exactly this situation some time ago, and posted to the list about it: http://marc.info/?l=php-internals&m=138118333112741&w=2 The options I presented there are: 1) Mark all exceptions as non-serializable, as you suggest, removing the surprise. 2) Don't capture args in exceptions backtraces, with the additional benefit of destructors firing more predictably. 3) Strip args out of the backtrace when serializing. 4) Have a function to strip the backtrace ready to serialize, and prevent serialization if it hasn't been called. Option 1 is the simplest, but just as many have questioned the need to serialize exceptions, I would question the need for their backtraces to contain every argument passed to every function in the stack, so would tend towards option 2. AFAIK, no unserializable information is stored outside the 'args' element of the backtrace items. Regards, -- Rowan Collins [IMSoP] --- ## #87313 — Stas Malyshev — 2015-07-27T07:08:51+00:00 URL: https://externals.io/message/85416#87313 · In reply to: #85416 Hi! Looking into some issue, I've discovered that, to my surprise, > Exceptions are serializable. Except that it doesn't always work of > course (e.g. see http://stackoverflow.com/q/9747813/214196) because > exceptions contain backtraces, and those can contain non-serializable > objects. So in reality, you never know if you can serialize it or not. > > So, I wonder - would it be ok to make exceptions not serializable at > all? I think that would prevent major WTF factor when people try it and > it randomly fails. > Since discussion on this did not lead to a definite conclusion, but I did not hear from anybody that they need serialized exceptions, and we keep getting bug reports about exception serialization and various issues triggered by it, I propose this change: https://github.com/php/php-src/pull/1442 Since it is kind of BC break (even though I assume it breaks something that needed not to be allowed in the first place) I'd like to merge it in 7.0.0. Please object if you think this should not be done and explain why. Otherwise I intend to merge it after a suitable wait and after adding necessary tests/docs. Thanks, -- Stas Malyshev smalyshev@gmail.com --- ## #87318 — Rowan Collins — 2015-07-27T14:12:37+00:00 URL: https://externals.io/message/85416#87318 · In reply to: #87313 Stas Malyshev wrote on 27/07/2015 08:08: > Since discussion on this did not lead to a definite conclusion, but I did > not hear from anybody that they need serialized exceptions, and we keep > getting bug reports about exception serialization and various issues > triggered by it, I propose this change: > https://github.com/php/php-src/pull/1442 I agree that this is better than the status quo - although I would still prefer stripping out the 'args' from the exception backtrace, which has other benefits for predictable destructors etc. Out of interest, is there any way to over-ride a denied serialization like this, using __sleep or Serializable? At the moment, you can create serializable sub-classes of exception, and retain a summarised copy of the backtrace: http://3v4l.org/IHPI5 It's not a big deal either way - in the rare cases where serialization of exception data is necessary, you could just write a wrapper to extract the public details - but I'm interested if my current hack will be adaptable if this change is applied. Regards, -- Rowan Collins [IMSoP] --- ## #87339 — Nikita Popov — 2015-07-28T13:07:54+00:00 URL: https://externals.io/message/85416#87339 · In reply to: #87313 > Hi! > > Looking into some issue, I've discovered that, to my surprise, > > […] > > Since discussion on this did not lead to a definite conclusion, but I did > not hear from anybody that they need serialized exceptions, and we keep > getting bug reports about exception serialization and various issues > triggered by it, I propose this change: > https://github.com/php/php-src/pull/1442 > > Since it is kind of BC break (even though I assume it breaks something that > needed not to be allowed in the first place) I'd like to merge it in 7.0.0. > Please object if you think this should not be done and explain why. > Otherwise I intend to merge it after a suitable wait and after adding > necessary tests/docs. > -1 on this. If there is no technical problem with serializing the Exception class itself, it should be possible to serialize it. It can always happen that an object contains some not-serializable member, this is nothing specific to exceptions. I don't see the point of this change. Also, Christian Stoller's mail pointed out that Symfony uses serialized exceptions. Nikita --- ## #87344 — Rowan Collins — 2015-07-28T15:50:04+00:00 URL: https://externals.io/message/85416#87344 · In reply to: #87339 Nikita Popov wrote on 28/07/2015 14:07: > -1 on this. If there is no technical problem with serializing the Exception > class itself, it should be possible to serialize it. It can always happen > that an object contains some not-serializable member, this is nothing > specific to exceptions. I don't see the point of this change. > > Also, Christian Stoller's mail pointed out that Symfony uses serialized > exceptions. The problem is that exceptions *unpredictably* contain non-serializable members, because it depends on what's happened at arbitrary points in the backtrace. There is nothing a user catching an exception can do to predict or prevent it happening, unless (as I have repeatedly suggested) there is some way to strip the 'args' from the backtrace items. --- ## #87345 — Stanislav Malyshev — 2015-07-28T16:40:46+00:00 URL: https://externals.io/message/85416#87345 · In reply to: #87339 Hi! > -1 on this. If there is no technical problem with serializing the > Exception class itself, it should be possible to serialize it. It can > always happen that an object contains some not-serializable member, this > is nothing specific to exceptions. I don't see the point of this change. The point is exactly that Exception contains non-serializable members that makes successfully serializing and restoring them impossible. You could get back something that looks like an Exception, but not the original one - namely, you can not store and restore backtraces. > Also, Christian Stoller's mail pointed out that Symfony uses serialized > exceptions. I must have missed it - what Symphony uses serialized exceptions for? -- Stas Malyshev smalyshev@gmail.com --- ## #87349 — Marco Pivetta — 2015-07-28T20:34:06+00:00 URL: https://externals.io/message/85416#87349 · In reply to: #87345 This sort of change would be a major BC break for 8.x or similar. I also don't see security implications, tbh. > Hi! > > > […] > > The point is exactly that Exception contains non-serializable members > that makes successfully serializing and restoring them impossible. You > could get back something that looks like an Exception, but not the > original one - namely, you can not store and restore backtraces. > > > […] > > I must have missed it - what Symphony uses serialized exceptions for? > > -- > Stas Malyshev > smalyshev@gmail.com --- ## #87351 — Rowan Collins — 2015-07-28T20:44:19+00:00 URL: https://externals.io/message/85416#87351 · In reply to: #87349 >This sort of change would be a major BC break for 8.x or similar. I think that was the point of trying to squeeze it into 7.0 --- ## #87352 — Stanislav Malyshev — 2015-07-28T20:46:26+00:00 URL: https://externals.io/message/85416#87352 · In reply to: #87349 Hi! > This sort of change would be a major BC break for 8.x or similar. How is it a major BC break? You make it sound like serializing exceptions is something no application can do without. I have yet to see a single case where it's useful (yes, I've read the Symphony comment but I'm not sure why they're doing it and if it's indeed something that should be done and not an ugly hack like unserializing fake internal objects). > I also don't see security implications, tbh. I don't want to discuss it in detail yet, but check out currently open or recently fixed security issues and see how many of them relate to serialized exceptions and consequences of that. -- Stas Malyshev smalyshev@gmail.com --- ## #87353 — Marco Pivetta — 2015-07-28T20:46:38+00:00 URL: https://externals.io/message/85416#87353 · In reply to: #87351 New BC breaks in Beta? Meh. > > > […] > > I think that was the point of trying to squeeze it into 7.0 --- ## #87354 — Stanislav Malyshev — 2015-07-28T20:50:56+00:00 URL: https://externals.io/message/85416#87354 · In reply to: #87353 Hi! > New BC breaks in Beta? Why not? What's the problem with it? Beta is to identify issues with current code, serialized exceptions is an issue (as in, they don't work and lead to security problems and generally pointless). Dragging this known problem around for years makes no sense. -- Stas Malyshev smalyshev@gmail.com --- ## #87450 — Ferenc Kovacs — 2015-07-31T19:21:21+00:00 URL: https://externals.io/message/85416#87450 · In reply to: #87345 On Tue, Jul 28, 2015 at 6:40 PM, Stanislav Malyshev wrote: > Hi! > > > […] > > The point is exactly that Exception contains non-serializable members > that makes successfully serializing and restoring them impossible. You > could get back something that looks like an Exception, but not the > original one - namely, you can not store and restore backtraces. > > Personally I feel the restoring them impossible argument weak, consider that we allow stuff like serializing resources without even a notice. Based on my own experiences where I had to fix multiple apps when we introduced the unserializable Closure (mostly error logger and debugging tools) which got passed as argument in the backtrace I would prefer if we could remove that restriction. -- Ferenc Kovács @Tyr43l - http://tyrael.hu --- ## #87453 — Stanislav Malyshev — 2015-07-31T19:56:30+00:00 URL: https://externals.io/message/85416#87453 · In reply to: #87450 Hi! > Personally I feel the restoring them impossible argument weak, consider > that we allow stuff like serializing resources without even a notice. Not sure what you mean by that. If you try to serialize resource, you just get an integer. Not ideal, as a remanant of the times in PHP where the approach was "if it doesn't make sense, do whatever and hope the user is ok with that", but certainly it's not "serializing resources". It's "ignoring resources when serializing and producing integers instead". Replacing Exceptions with integers probably won't work that well :) > Based on my own experiences where I had to fix multiple apps when we > introduced the unserializable Closure (mostly error logger and debugging > tools) which got passed as argument in the backtrace I would prefer if > we could remove that restriction. I don't see how we can really remove the underlying problem - Exceptions contain backtraces, which means serializing them tries to serialize a ton of stuff that may be not only not serializable but outright dangerous to carry around - such as keys, passwords, etc. Given how many problems we have had with serialization of complex objects lately, and given that I still see absolutely no practical use of actually serializing exceptions I would rather remove it and reduce the vulnerable surface than keep dealing with dozens of issues that continue to pop up from that. BTW what you mean by "unserializable Closure"? As far as I know you can not serialize Closure. -- Stas Malyshev smalyshev@gmail.com --- ## #87457 — Stephen Coakley — 2015-07-31T20:14:53+00:00 URL: https://externals.io/message/85416#87457 · In reply to: #87352 > Hi! > > > […] > > How is it a major BC break? You make it sound like serializing > exceptions is something no application can do without. I have yet to see > a single case where it's useful (yes, I've read the Symphony comment but > I'm not sure why they're doing it and if it's indeed something that > should be done and not an ugly hack like unserializing fake internal > objects). > > > […] > > I don't want to discuss it in detail yet, but check out currently open > or recently fixed security issues and see how many of them relate to > serialized exceptions and consequences of that. > -- > Stas Malyshev > smalyshev@gmail.com > Serializing exceptions can be useful in parallel code using multiple processes or threads. I have been working on a concurrency library for a week or two and I serialize exceptions (excluding stack trace arguments) to send them back to the calling process to aid in debugging process failures. I agree there aren't too many use cases, but there are a few. Of course, exceptions aren't *consistently* serializable, which is still a problem that should be resolved in some way. -- Stephen Coakley --- ## #87458 — Stephen Coakley — 2015-07-31T20:22:46+00:00 URL: https://externals.io/message/85416#87458 · In reply to: #87313 > Hi! > > Looking into some issue, I've discovered that, to my surprise, > > […] > > Since discussion on this did not lead to a definite conclusion, but I did > not hear from anybody that they need serialized exceptions, and we keep > getting bug reports about exception serialization and various issues > triggered by it, I propose this change: > https://github.com/php/php-src/pull/1442 > > Since it is kind of BC break (even though I assume it breaks something that > needed not to be allowed in the first place) I'd like to merge it in 7.0.0. > Please object if you think this should not be done and explain why. > Otherwise I intend to merge it after a suitable wait and after adding > necessary tests/docs. > > Thanks, > I'm serializing exceptions in a current project, and I would much prefer losing `args` (the only part not always serializable) from the trace than not being able to serialize the exception at all. Making exceptions not serializable will just result in another userland wrapper, like a SuperException, that lets you serialize and unserialize with eval()'s. I think allowing the serialize to drop args in the trace and just include a warning in the docs about the serialization being lossy. My 2 cents. -- Stephen Coakley --- ## #87459 — Anatol Belski — 2015-07-31T20:55:14+00:00 URL: https://externals.io/message/85416#87459 · In reply to: #87354 Hi Stas, > -----Original Message----- > From: Stanislav Malyshev [mailto:smalyshev@gmail.com] > Sent: Tuesday, July 28, 2015 10:51 PM > To: Marco Pivetta ; Rowan Collins > > Cc: Nikita Popov ; PHP Internals List > > Subject: Re: [PHP-DEV] Re: Serializing exceptions > > Hi! > > > […] > > Why not? What's the problem with it? Beta is to identify issues with current > code, serialized exceptions is an issue (as in, they don't work and lead to security > problems and generally pointless). Dragging this known problem around for > years makes no sense. I was looking through and seems that situation is not that bad. Reading http://fabien.potencier.org/php-serialization-stack-traces-and-exceptions.html it might be not that meaningful to break symphony. Having upset users is logic as long as we don't offer a solution, while the functionality itself seems to make sense. And after all serialization looks useful in some cases (logging at least). Maybe we should go more fine tuning way, maybe ignoring the thing that cannot be serialized. Say, serializing only a white list in this case? Regards Anatol --- ## #87460 — Ferenc Kovacs — 2015-07-31T21:23:03+00:00 URL: https://externals.io/message/85416#87460 · In reply to: #87453 On Fri, Jul 31, 2015 at 9:56 PM, Stanislav Malyshev wrote: > Hi! > > > […] > > Not sure what you mean by that. If you try to serialize resource, you > just get an integer. Not ideal, as a remanant of the times in PHP where > the approach was "if it doesn't make sense, do whatever and hope the > user is ok with that", but certainly it's not "serializing resources". > your argument was that we shouldn't allow serialization of such items where after the unserliazion you won't get the same thing back. this is where my point about we allow serializing resources just fine was aimed at. > It's "ignoring resources when serializing and producing integers > instead". Replacing Exceptions with integers probably won't work that > well :) > nobody suggested replacing Exceptions with integers, but serializing Closure to "Closure #1" or similar (albeit we could even do a better job, similarly how java8 supports lambda serialization). > > > […] > > I don't see how we can really remove the underlying problem - Exceptions > contain backtraces, which means serializing them tries to serialize a > ton of stuff that may be not only not serializable but outright > dangerous to carry around - such as keys, passwords, etc. `var_export()` shares the same problem, should we then also try to remove __toString() from closures? or try for looking for passwords or credit card data in the variable? I think that your argument is really streched. I'm not saying that we should give more rope for the developers to hang themselves but the current situations sucks (you can serialize basically anything but Closure) and I think that your suggested way of solving the problem would create a medium sized BC break for little gain as it would only fix a specific scenario (people like in your SO link in the opening post would still try to `debug_backtrace()` from their logger/debugger which would trigger the same problem that the Closure arg can't be serialized). > Given how many > problems we have had with serialization of complex objects lately, and > given that I still see absolutely no practical use of actually > serializing exceptions I would rather remove it and reduce the > vulnerable surface than keep dealing with dozens of issues that continue > to pop up from that. > that's true, but you would only remove a single attack vector via removing an otherwise useful feature those issues would be still present, still reported, we should still fix then, even if you can't trigger them via an exception logger but a session unserialized. > BTW what you mean by "unserializable Closure"? As far as I know you can > not serialize Closure. > I mean exactly that and btw. that was the original problem in the Stackoverflow post you cited in your opening post ( http://stackoverflow.com/questions/9747813/storing-a-php-exception-object-in-a-database ). -- Ferenc Kovács @Tyr43l - http://tyrael.hu --- Next page: https://externals.io/message/85416.md?page=2