Opened 2 years ago
Closed 2 years ago
#17932 closed bug (fixed)
strptime is broken (not matching patterns it should be matching)
Reported by: | fatigatti | Owned by: | nobody |
---|---|---|---|
Priority: | normal | Milestone: | |
Component: | System/POSIX | Version: | R1/Development |
Keywords: | strptime POSIX | Cc: | |
Blocked By: | Blocking: | ||
Platform: | All |
Description
While trying to fix a bug in WebPositive (cookies being deleted) I ran into this through BNetworkCookie and BHttpTime. Steps to reproduce:
1a) Make sure your locale is english in all the relevant fields (date and time formats specifically, but change all to english if unsure).
1b) Alternatively, change the "example" variable in the source file attached with the words used in your locale (change Mon to Lun and Jan to Ene if you're using spanish, for example).
2) Compile and run it. Expected result is matching the format and filling the tm struct correctly (tested on Xubuntu LTS but should be the same for any system implementing POSIX strptime correctly). Haiku's implementation fails to match.
Attachments (1)
Change History (8)
by , 2 years ago
comment:1 by , 2 years ago
The problem in strptime.c is in the code block starting at line 34 which is currently
switch (*f++) { case 'a': case 'A': dest = &tm->tm_wday; min = ABDAY_1; range = 7; goto symbolic_range; case 'b': case 'B': case 'h': dest = &tm->tm_mon; min = ABMON_1; range = 12; goto symbolic_range;
and at line 185
symbolic_range: for (i=2*range-1; i>=0; i--) { ex = nl_langinfo(min+i); len = strlen(ex); if (strncasecmp(s, ex, len)) continue; s += len; *dest = i % range; break; } if (i<0) return 0; goto update;
The range is doubled so that both abbreviated and full strings can be tested. Unfortunately in Haiku the abbreviated strings come after the full strings. The loop tests from the last possible string to the first and unfortunately there is no string returned by nl_langinfo(ABMON_1+2*range-1) it errors and aborts the loop.7
When using %a (ABDAY_1) it also potentially fails, it will check against Mon,...Sun,January,February,March,April,May,June,July
The min values should be set to DAY_1 or MON_1
comment:2 by , 2 years ago
Indeed, glibc/musl define these constants in the opposite ordering to FreeBSD and ourselves here. Guess we need some #ifdefs in this code with comments.
comment:3 by , 2 years ago
I might give this a try since it seems easy enough for me. I understand the order in what we define the constants is untouchable because it may break a lot of things, so the ifdefs would go in this code (strptime.c) and they will check for the Haiku platform and change the code accordingly. Should we push that change upstream then?
We should probably check strftime as well for proper behaviour. Should we consider changing the order of the constants then?
Sorry about the questions, I'm trying to understand what would be the best approach before attempting anything.
comment:4 by , 2 years ago
No, we should not upstream anything, just add some #ifdef HAIKU that it will be clear enough we need to keep when merging newer versions from upstream.
comment:5 by , 2 years ago
Thanks for the quick reply. I'll give it a go as soon as I have some time then!
comment:6 by , 2 years ago
I sent a patch at https://review.haiku-os.org/c/haiku/+/5635
Tried it with the small test program and it's matching! Big thanks to both of you for discovering the problem and giving the solution.
comment:7 by , 2 years ago
Milestone: | Unscheduled |
---|---|
Resolution: | → fixed |
Status: | new → closed |
Changed merged; thanks for the contribution!
Simple strptime test