Re: Fix for bug #63437

From: Date: Thu, 14 Mar 2013 11:42:25 +0000
Subject: Re: Fix for bug #63437
References: 1 2 3 4 5 6 7 8  Groups: php.internals 
Request: Send a blank email to [email protected] to get a copy of this message
On Wed, 13 Mar 2013, Anatol Belski wrote:

> On Mon, 2013-03-11 at 11:42 +0000, Derick Rethans wrote: 
> > 
> > On Mon, 11 Mar 2013, Anatol Belski wrote:
> > > 
> > > What is the way you had in the mind to achieve the 
> > > string<->integer conversions?
> > 
> > atoll() (or atoq()).
> 
> Please take a look at the reworked patch in #53437
> 
> https://bugs.php.net/patch-display.php?bug_id=53437&patch=date_patch_var3.patch&revision=latest
> 
> Serializing 64 bit integers as strings.

That bit looks ok, I have a few comments though:

+	PHP_DATE_INTERVAL_READ_PROPERTY("special_type", special.type, unsigned int);
+	if ((*intobj)->diff->special.amount > 12 || (*intobj)->diff->special.amount <
-12) {
+		php_error(E_ERROR, "Invalid serialization data for DateInterval object
(special_amount)");
+	}

Checking for the values here, means that the library (timelib) isn't 
in control of the values that are allowed anymore. I wonder whether the 
deserialisation should bother checking some of those special/extra 
types/values. The only important thing is to not have things crash 
really.

And secondly, where are the test cases?

cheers,
Derick

-- 
http://derickrethans.nl | http://xdebug.org
Like Xdebug? Consider a donation: http://xdebug.org/donate.php
twitter: @derickr and @xdebug
Posted with an email client that doesn't mangle email: alpine


Thread (15 messages)

« previous php.internals (#66610) next »