| Summary: | Crash in jk-status on Windows (when producing HTML output) | ||
|---|---|---|---|
| Product: | Tomcat Connectors | Reporter: | Rainer Jung <rainer.jung> |
| Component: | Common | Assignee: | Tomcat Developers Mailing List <dev> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | chinoygupta |
| Priority: | P2 | ||
| Version: | 1.2.41 | ||
| Target Milestone: | --- | ||
| Hardware: | All | ||
| OS: | All | ||
|
Description
Rainer Jung
2015-08-26 18:42:34 UTC
Looking at the manual pages on a Linux and BSD system, it does appear that the %z format specifier is indeed an "extension" to C90. The Linux manual page specifically identifies it as a "Single UNIX Specification" extension:
"
%z The +hhmm or -hhmm numeric timezone (that is, the hour and
minute offset from UTC). (SU)
"
The "(SU)" notation indicates that it's a "Single Unix Specification" extension. So it's not surprising that it's not supported on Microsoft Windows.
Having the time zone information in a timestamp is pretty important. C90 specifies the "%Z" format specifier. Can we use that instead?
What is the root cause of the crash? stfrtime takes a buffer-size argument and should not overrun it. Is this really a bug in the win32 POSIX library?
%z (lower case) was first added (standardized) in "IEEE Std 1003.1, 2004 Edition" aka SUSv3 (after including issue 6). See http://pubs.opengroup.org/onlinepubs/009695399/functions/strftime.html %Z (upper case) was what we used before. As I wrote, it was changed in order to fix BZ 54177. Concerning the importantce of the tz info, I dont fully agree. This is "just" jk-status output. So it would be nice to have but rating all the stuff that jk-status shows, IMHO the current tz is one of the least important things. Looking at the available APIs on Windows I tend to drop the tz info for that platform. BZ 54177 suggests we should stick to numeric representation (at least to XML safe stuff) and I invite you to look at the apr file "time/win32/time.c" how tm_gmtoff is calculated in apr_time_exp_lt(). That tm_gmtoff is what httpd uses to print the tz offset in its access log. At least for me that's not a road I'd like to go "only" for tz. Any other suggestions are welcome, but short term it seems best to remove and stay on the safe side. About the root cause of the crash: httpd+mod_jk should not crash due to buffer overrun and in a simple standalone test it does not and simply returns without formatting the time stamp. In the debugger, I can see that it crashes in strftime and only if "%z" is used and also not when "%z" is the only thing to format. In addition one can see, that the buffer would not be big enough to accommodate the full formatted timestamp. Why it crashes in the module and not in the standalone example is not known to me. I don't have PDB files for all components, so it is a bit hard to debug further. But the crash definitely happens in strftime() and is definitely related to "%z" in combination with other format chars in the pattern and a too small buffer. And we definitely don't want to %z on Windows because its output (long textual name of time zone) is not what we want. Fixed in r1698316, will be part of 1.2.42. I found a way to get the timezone offset even on Windows without too much hassle. Not very nice code though. Tested in combination with httpd 2.4 on Windows (only for my local timezone). Not sure, whether there is still a problem on Netware. According to http://www.novell.com/documentation/developer/libc/libc_vol2/data/sdk1810.html, %z should work when using libc. According to http://www.novell.com/documentation/developer/clib/prog_enu/data/sdk1810.html, %z should not be available when using clib. Not sure, which variant of Netware - libc or clib - we actually support. Clsing, but can be reopened, if we need a fix on Netware. Is 1.2.42 release planned in near future or should we integrate this fix now? If you can easily integrate now, I suggest you do so. There are currently only two post 1.2.41 fixes, this one here and one about atomics on some niche platforms (not Windows, not Linux 64 Bit). This one here affects all Windows users, but one of the frequently used providers for Windows binaries (ApacheLounge) has now included the fix in his 1.2.41 build. So it seems there is no high pressure for a release. I worked a bit on our own Windows build system, so I hope we can finally provide good quality binaries for Windows for 1.2.42. I want to update the docs and probably have a look at some of the old outstanding ISAPI redirector bugs. So personally I don't plan a release very soon (not this month, maybe much later depending on what happens in the meantime). Thanks for the prompt reply. Based on your comment, it might be some time before the next release comes out. So I will look into integrating it now only. |