ZF-987: Extensible sleep / wakeup

Description

h3. Problem:

Zend_TimeSync is potentially dangerous. Although it works nicely, and does what it claims, there is a big risk of developers accidentally running time queries (SNTP/NTP) on public time servers for every web page request.

h3. Proposed Solution:

In order to minimize this risk, Zend_TimeSync should default to a safe behavior. After the i18n team discussed this issue at length, Andries proposed adding magic sleep/wakeup methods to Zend_TimeSync, so that the component can maintain persistent state. The state is needed for Zend_TimeSync to have sufficient intelligence to make an informed decision about whether to use a cached time offset, or poll some time servers (and which ones) to update the offset, without "abusing" public time servers.

Initially, the implementations for __sleep/__wakeup could use Zend_Cache, but developers ought to have the ability to overload these and use other mechanisms to persist the state of Zend_TimeSync.

Comments

Please consider attached file

```

Just an idea ... what if init() accepted a single argument that is an instance of Zend_Cache?

Then developers would use: $sync = new Zend_TimeSync(); $sync->init(Zend_Cache::factory($frontend, $backend, $frontendOptions, $backendOptions));

Hmmm .. I see one problem. It should be possible to use Zend_TimeSync without loading all the classes/code needed to query time servers, when a cached information is available and current. The constructor for Zend_TimeSync currently processes its arguments and loads Zend_TimeSync_*, etc. However, if a recently cached result exists, it should be possible to obtain access to offset-adjusted Zend_Date instance objects and the offset itself without loading all the code for interacting with time servers.

For example:


$offset = Zend_TimeSync::getOffset(Zend_Cache::factory($frontend, $backend, $frontendOptions, $backendOptions));
// now use $offset when inserting/updating a timestamp into a row in a DB
$now = time() - $offset;
$sql = "UPDATE order SET last_viewed_timestamp = $now";

This use case was presented as one of the reasons justifying the proposal.

If you think we should also support:


$timeSync = new Zend_TimeSync($target, $alias, Zend_Cache::factory($frontend, $backend, $frontendOptions, $backendOptions));

then developers could manage multiple Zend_TimeSync instance objects having possibly different offsets (e.g. resulting from querying different sets of time servers). However, this seems more like an edge case, than the other use case above.

From Bill's recent email to fw-general about "feature freeze", the window of time available for finishing Zend_TimeSync before ZF 1.0 is almost closed. However, I have every expectation that Zend_TimeSync could be included afterwards, if it is not included in 1.0.

Loading from cache without subclasses should be possible.

And it should be possible to empty the cache to initialize a new timesync request, instead of creating a new timesync.

Related to not inclusion for ZF1.0... This would of course also mean that the related issues for Zend_Date will not be included.

Of course I would like to have it included within 1.0... but maybe it's better not to hurry, to make it really as secure as we can imagine. For this class it's better to have it almost perfect in our eyes, than importing problems into the framework.

Gavin, regarding the idea that init() should accepted a single argument that is an instance of Zend_Cache... I have also thought about that, but if a developer wishes to use his own caching mechanism, it will not be as simple as it is now.

Eg, consider that the developer's caching component uses different method names. How would Zend_TimeSync know which methods to call? That's the reason why I have provided separated methods, so the developer could easily override these (if there's really need for it).

New patch

Use case for patch {ZF-987-patch}:


require_once 'Zend/TimeSync.php';
require_once 'Zend/Cache.php';

$example = array(
    'windows'   => 'ntp://time.windows.com',
    'localhost' => 'ntp://127.0.0.1:123',
);

$frontendOptions = array(
   'lifeTime' => 7200,
   'automaticSerialization' => true
);

$backendOptions = array(
    'cacheDir' => './tmp/'
);


$servers = new Zend_TimeSync($example);
$servers->init('Core', 'File', $frontendOptions, $backendOptions);

try {
    $date = $servers->getDate();
    $info = $servers->getInfo();
    
    echo $date->getIso();
} catch (Zend_TimeSync_Exception $e) {
    echo $e->getMessage();
}

important notice I have asked Darby to delete my two last posts, because it messed up the formatting, so i will re-post in a single post.

Gavin, regarding Zend_TimeSync without loading all the classes/code needed to query time servers, when a cached information is available... there's one problem with the current approach, which is that Zend_TimeSync_* will be required, or php will return a __PHP_Incomplete_Class when quering a cached result.

Consider the following use case, where i only try to include the required code if there is no cache available:


require_once 'Zend/TimeSync.php';
require_once 'Zend/Cache.php';

$frontendOptions = array(
   'lifeTime' => 7200,
   'automaticSerialization' => true
);

$backendOptions = array(
    'cacheDir' => './tmp/'
);

$servers = new Zend_TimeSync();
$servers->init('Core', 'File', $frontendOptions, $backendOptions);

if (!$servers->isCached()) {
    $example = array(
        'windows'   => 'ntp://time.windows.com',
        'localhost' => 'ntp://127.0.0.1:123',
    );
    
    $servers->addServer($example);
}

try {
    $date = $servers->getDate();
    $info = $servers->getInfo();
    
    echo $date->getIso();
} catch (Zend_TimeSync_Exception $e) {
    echo $e->getMessage();
}

will throw an exception, because when i load a cached result, and the Ntp or Sntp classes (depends on which server returned a valid result) are not included, it will result in a __PHP_Incomplete_Class.

A solution exists by factoring the code differently. For example, a lightweight container class could be used for the caching, to avoid the coupling with the Ntp and Sntp classes. Alternatively, persisting the data in a simple keyed array could also work. Since most of the TimeSync, Sntp, and Ntp code only needs to execute once every few minutes (at most), whatever code is run each request should be minimized, even if that results in some extra code added to the code that runs once every few minutes.

Even though we are discussing some aspects of optimization above, I believe we need to discuss this in order to make sure the design of the final API accomodates the most common use cases.

I have been talking to Thomas about this, and he has good reasons to avoid loading an array into Zend_Date ... I'll quote Thomas here:

bq. Accepting an array as input for offset means that all user could have their own time to be calculated with... which would break ALL languages, as time() is always strict coupled with the OS...

Using a class won't prevent the problem Thomas is referring to. What stops a developer from simply creating their own subclass that loads from an array, and then using an instance of that subclass as "input for offset" to "load" into Zend_Date? Some developers will want to store the offset using something other than Zend_Cache, but I don't think we should try to implement an alternative. Instead, I suggested two approaches above, both of which can be implemented without passing arrays between Zend_TimeSync* and Zend_Date.

In any case, I'm not too worried about the implementation details at this time. Instead, let us concentrate on the architecture and design by creating a way for developers to easily use the values saved by Zend_TimeSync without loading all the code/overhead of the NTP/SNTP protocol handling classes. Can we improve upon the possible example use case in my first comment above? I always like to see alternatives before proceeding :)

I would like to activly continue developing this class.... I have tried what Gavin suggested (with a response object, or with array storage). But this takes away the advantage of using other methods from the protocol classes. Zend_TimeSync provides a facade to Ntp and Sntp, so it is required anyways... How do you plan on using a class's methods if the class isnt loaded? I am clueless at this time, but would like to continue. Any thoughts, guys?

I think we should delay the further progress in this issue until after 1.0 We are all in stress for getting 1.0 finished.

And as Zend_Timesync will not be included in 1.0 there is no problem to delay it some weeks.

Hey Thomas,

Sorry my bad, I didn't want to push anybody. I just want to make sure that this component isn't forgetten.

I agree with Thomas regarding delay. Regardless, much of the value of this component comes from simply having read access to the data computed by this component. Reading the most useful portions of this data from within developer's code does not require code that understands the NTP/SNTP protocols.

For example, in the very simplest form, this component can provide an offset between the local system's time()/microtime() and a time computed using NTP/SNTP. Thus, the most common use case for this component might be satisfied by simply returning a single cached value. No need to load lots of code to return this single value. Obviously, there are other pieces of data and information provided by this component. The decisions about what should be available cached will require careful consideration. What would you suggest?

I'm not sure that building an internal caching mechanism is necessary. This component may be considered analogous to the web service consumer components, such as Zend_Service_Amazon, in that these components would be used primarily to fetch information from remote service providers. If we require internal caching for Zend_TimeSync, then I think we should require the same from other components like Zend_Service_Amazon. But does this make sense? I think no.

Consider the original problem statement:

bq. Zend_TimeSync is potentially dangerous. Although it works nicely, and does what it claims, there is a big risk of developers accidentally running time queries (SNTP/NTP) on public time servers for every web page request.

The same risk of making remote service requests for every web page request applies to the web services components, also. Indeed, the same risk appears within PHP itself - {{fopen()}} with {{allow_url_fopen}} enabled, the curl extension, socket functions, and much more. But these items do not have magic internal caching.

Instead, I tend to think that we should simply ensure that caching the results from Zend_TimeSync is easily possible with the API. Leave it to the user to be responsible for its use and to decide what is acceptable.

I would recommend resolving this issue with a simple addition to the documentation, maybe urging users to use the components responsibly, but, more importantly, show an example of how to cache the results from Zend_TimeSync.

I already thought about this one.

We can not solve all possible problems with an automatic approach without having the user being aware of some things. As this is originally my component and I am not sure if andries has time and effort to bring this further I will look in fixing the docu for about 1.1 or 1.2.

There are some small issues with Zend_TimeSync which should also be solved before releasing it into the core. So we will solve this with documentation and probably have the class included with latest 1.2.

I will take over this work, and write / add the documentation after Release 1.0. There is some other work for the coupling to Zend_Date which will be done the same time.

Related to Darbys response from june and the actual usage of the framework especially Zend_Cache I have come to the same solution.

It is not be necessary to include a own caching mechanism. Therefor this issue is closed as "won't fix".