Zend Framework

Extensible sleep / wakeup

Details

  • Type: Improvement Improvement
  • Status: Resolved Resolved
  • Priority: Major Major
  • Resolution: Won't Fix
  • Affects Version/s: 0.8.0
  • Fix Version/s: 1.5.0
  • Component/s: Zend_TimeSync
  • Labels:
    None

Description

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.

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.

  1. ZF-987-patch.rar
    02/Mar/07 12:23 PM
    2 kB
    Andries Seutens
  2. ZF-987-patch-2.rar
    04/Mar/07 4:14 PM
    2 kB
    Andries Seutens

Issue Links

Activity

Hide
Andries Seutens added a comment -

Please consider attached file

ZF-987-patch.rar
Show
Andries Seutens added a comment - Please consider attached file
ZF-987-patch.rar
Hide
Gavin added a comment -

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.

Show
Gavin added a comment - 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.
Hide
Thomas Weidner added a comment -

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.

Show
Thomas Weidner added a comment - 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.
Hide
Andries Seutens added a comment -

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).

Show
Andries Seutens added a comment - 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).
Hide
Andries Seutens added a comment -

New patch

Show
Andries Seutens added a comment - New patch
Hide
Andries Seutens added a comment -

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();
}
Show
Andries Seutens added a comment - 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();
}
Hide
Andries Seutens added a comment -

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.

Show
Andries Seutens added a comment - 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.
Hide
Gavin added a comment -

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.

Show
Gavin added a comment - 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.
Hide
Andries Seutens added a comment -

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:

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...

Show
Andries Seutens added a comment - 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:
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...
Hide
Gavin added a comment -

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

Show
Gavin added a comment - 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
Hide
Andries Seutens added a comment -

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?

Show
Andries Seutens added a comment - 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?
Hide
Thomas Weidner added a comment -

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.

Show
Thomas Weidner added a comment - 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.
Hide
Andries Seutens added a comment -

Hey Thomas,

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

Show
Andries Seutens added a comment - Hey Thomas, Sorry my bad, I didn't want to push anybody. I just want to make sure that this component isn't forgetten.
Hide
Gavin added a comment -

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?

Show
Gavin added a comment - 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?
Hide
Darby Felton added a comment -

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:

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.

Show
Darby Felton added a comment - 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:
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.
Hide
Thomas Weidner added a comment -

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.

Show
Thomas Weidner added a comment - 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.
Hide
Thomas Weidner added a comment -

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.

Show
Thomas Weidner added a comment - 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.
Hide
Thomas Weidner added a comment -

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".

Show
Thomas Weidner added a comment - 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".

People

Vote (0)
Watch (3)

Dates

  • Created:
    Updated:
    Resolved: