74847 - PR_ParseTimeString broken - not all timezones supported

Status: RESOLVED WONTFIX

(NSPR :: NSPR, defect)

Description

[NilsE]

OS: RH Linux 6.2
CVS pull 2001-04-05
(probably at least all Unix flavours affected)

Overview Description:

PR_ParseTimeString converts a date string to seconds since Unix Epoch:
http://lxr.mozilla.org/seamonkey/source/nsprpub/pr/src/misc/prtime.c#939
but is missing several timezones. If these are encountered, the returned
value is bogus, but failure is not indicated.

Reproducible: always

Steps to reproduce:

1. Call PR_ParseTimeString with two timestrings, differing only in the
   timezone (say PDT and CEST).  
2. Observe that resulting ints differ in much more that just the 9 hours
   offset.


Detailed Description:

This was being observed when chasing
http://bugzilla.mozilla.org/show_bug.cgi?id=73029 -- PR_ParseTimeString
fails to recognize european timezones CET and CEST. All calculations
based on the bogus result are wrong. No warning was given. This resulted
in lots of assertions and wrong cache behaviour there. The fix there was
to avoid PR_ParseTimeString. But LXR says that PR_ParseTimeString is
referenced in 18 other files of Mozilla, yielding wrong results there as
well. The fact that not all timezones are covered is even documented in
the source, but no action is taken in case the an unknown TZ is
discovered. Silent failure is not acceptable. PR_ParseTimeString must
support all common timezones in the world, possibly by using an external
library.

Actual results: 
return value of PR_ParseTimeString wrong if timezone not supported, but
no indication of failure. The only timezones supported are: PST, PDT,
MST, MDT, CST, CDT, EST, EDT, AST, NST, GMT, BST, MET, EET, JST. In the
real world, there are much more ... ;-)


Expected results:
return value correct for all currently used timezones.


Additional information:

- This bug may also be the source of problems in bug 63720 and bug
65671. They're JavaScript bugs, but in the end NSPR is used there as
well.

- The reason why I set severity to major is that no warnings are being
issued and Netscape devlopers in supported timezones cannot see the
problem reported by users in problematic timezones. And such problems do
exist (see bug 73029). This leads to a class of bugs that are difficult
to track down. (In my case, the cache was expiring most entries
immediately but darin couldn't reproduce the problem because he was in
PDT). Fixing this bug will yield a high gain.

- In bug 73029, I attached a patch for prtime.c to support MEST, CET and
CEST. The brute force fix for this bug will be add all other timezones
in the same manner.

Comment 1

[larryh (gone)]

This is really a deprecated function. It is still present for backward 
compatibility. No enhancements are planned for this function.

Comment 2

[NilsE]

If it's deprecated, why is it still being used in 18 different files in
Seamonkey? If it's just for backward compatibility, why are even new extensions
such as P3P using it? Could it be that you consider it deprecated and maybe even
have a replacement for it, but word hasn't got through to Seamonkey folks? If
so, some evangelizing might be appropriate. If not, I disagree with marking WONTFIX.

Comment 3

[Dan Mosedale (:dmosedale, :dmose)]

It would be helpful to know why this was deprecated, and what existing callers
are supposed to do now.  Could this information at least be added to the
comments in prtime.h, so that folks stumbling on this function via lxr and other
places won't be tempted to use it?

Comment 4

[larryh (gone)]

PR_ParseTimeString() is not documented in the NSPR Reference 
( http://www.mozilla.org/projects/nspr/reference/html/index.html ). It was left 
in the headers and source for backward compatibility. The source for the 
function says that only a limited number of timezones are recognized.

Comment 5

[NilsE]

After reading chapter 14 of the above, it appears to me that NSPR does provide
fcts to convert PRTime (ticks since epoch) into PRExplodedTime (struct
representing year, month, day, ...) and vice versa, but no fctn that converts a
time string into PRExplodedTime (similar to PR_ParseTimeString).

Could it be that you declared PR_ParseTimeString deprecated without providing a
substitute?

Comment 6

[Dan Mosedale (:dmosedale, :dmose)]

larryh: well, the current header file commentary and lack of reference guide
mention doesn't seem to have been sufficient to prevent people from continuing
to use it. It seems as though adding "DEPRECATED" loudly in the header file
would at least be a step in the right direction. Also, can you shed some light
on why it was deprecated?  Thanks!

nils: if you could file a separate bug on what to do about the SeaMonkey
call-sites, that would be great.

Comment 7

[larryh (gone)]

Why was PR_ParseTimeString() deprecated? Good question. Frankly, I don't recall. 
Alas, I recall participating in the discussion, but for the life of me, I can't 
remember the content. ... a bad case of CRS. That's what happens when you get 
old, or something.

Comment 8

[NilsE]

Dan: I filed bug 90631 about removing PR_ParseTimeString() from Seamonkey.

To close this matter, can we agree to shift the focus of this bug to adding 
the documentation that Dan mentioned? I would also suggest to add a note that
there's no official replacement provided.

Comment 9

[larryh (gone)]

See related bugzilla: 90631.
Marking WontFix again.

Comment 10

[Jan Varga [:janv]]

I can't find a replacement for this function ?
Is there a similar function at all ?
I need it for SQL database support I work on
Thanks

Comment 11

[Wan-Teh Chang]

*** Bug 180912 has been marked as a duplicate of this bug. ***