Skip to content

Commit 00e1ec3

Browse files
Kaiedespevans
authored andcommitted
Fix DateFormatter TimeZone Setter
Because of the use of 'timeZone' instead of 'newValue' in the setter, setting the timeZone of a DateFormatter doesn't actually work. Instead, it winds up re-setting the old value, which happens to be whatever was originally set, usually what we get from copying the default down in CoreFoundation. Also adds a unit test to help prevent breaks like this again. (cherry picked from commit 9f3e050) Fix Off-By-One Error With System TimeZone This one is nasty, since it creates problems for DateFormatter, while appearing to behave correctly. (Mostly) By copying 1 too many bytes, we wind up with the null terminator in the CFString. This breaks DateFormatter in an annoying way that the system TimeZone is always considered to be GMT by the formatter. You can ask for the zone’s abbreviation, and it will be correct. The name will appear to be correct, except the length is 1 longer than it should be. All sorts of fun. But the moment you ask DateFormatter to give you a string (or parse one), it assumes the TimeZone is GMT and ruins your day. Included is a test case which can detect this. The downside is that it doesn’t detect the problem if GMT is already the current TimeZone. It is possible a second, more targeted test should also be added here? This also breaks getting the description on the NSTimeZone/CFTimeZone, but not the TimeZone because of the wrapping at play, because of the terminator that gets inserted into the string description. (cherry picked from commit 827e842) Cleanup SystemTimeZone tests Cleans up the DateFormatter test, improves the information about how it doesn’t catch issues when the system time zone is already GMT, and adds a more focused test for the SystemTimeZone name issue that can catch it when GMT is the system time zone. (cherry picked from commit 2219bd1) Make UTC Explicit in test_dateDifferenceComponents The test assumes that it is running as GMT/UTC. Because of the TimeZone.current fixes, the test is no longer guaranteed to be using GMT/UTC, and will use the system TimeZone and breaks when in PDT for example. By making the UTC assumption explicit, we remove one more variable on this test. (cherry picked from commit 5b1bfc9) Disable test_expectedTimeZone Case 1 This is hitting an issue on GMT time zones where it shows up as UTC when formatted. (cherry picked from commit e826252)
1 parent e959b9f commit 00e1ec3

File tree

5 files changed

+91
-13
lines changed

5 files changed

+91
-13
lines changed

CoreFoundation/NumberDate.subproj/CFTimeZone.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -793,7 +793,7 @@ static CFTimeZoneRef __CFTimeZoneCreateSystem(void) {
793793
size_t zoneInfoDirLen = CFStringGetLength(__tzZoneInfo);
794794
if (strncmp(linkbuf, tzZoneInfo, zoneInfoDirLen) == 0) {
795795
name = CFStringCreateWithBytes(kCFAllocatorSystemDefault, (uint8_t *)linkbuf + zoneInfoDirLen,
796-
strlen(linkbuf) - zoneInfoDirLen + 1, kCFStringEncodingUTF8, false);
796+
strlen(linkbuf) - zoneInfoDirLen, kCFStringEncodingUTF8, false);
797797
} else {
798798
name = CFStringCreateWithBytes(kCFAllocatorSystemDefault, (uint8_t *)linkbuf, strlen(linkbuf), kCFStringEncodingUTF8, false);
799799
}

Foundation/DateFormatter.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -182,13 +182,13 @@ open class DateFormatter : Formatter {
182182
/*@NSCopying*/ internal var _timeZone: TimeZone? { willSet { _reset() } }
183183
open var timeZone: TimeZone! {
184184
get {
185-
guard let timeZone = _timeZone else {
185+
guard let tz = _timeZone else {
186186
return (CFDateFormatterCopyProperty(_cfObject, kCFDateFormatterTimeZone) as! NSTimeZone)._swiftObject
187187
}
188-
return timeZone
188+
return tz
189189
}
190190
set {
191-
_timeZone = timeZone
191+
_timeZone = newValue
192192
}
193193
}
194194

TestFoundation/TestCalendar.swift

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,16 @@ class TestNSDateComponents: XCTestCase {
238238
// 2286-11-20 17:46:41
239239
let date4 = Date(timeIntervalSince1970: 10_000_000_001)
240240

241-
let diff1 = Calendar.current.dateComponents([.month, .year, .day], from: date1, to: date2)
241+
// The date components below assume UTC/GMT time zone.
242+
guard let timeZone = TimeZone(abbreviation: "UTC") else {
243+
XCTFail("Unable to create UTC TimeZone for Test")
244+
return
245+
}
246+
247+
var calendar = Calendar.current
248+
calendar.timeZone = timeZone
249+
250+
let diff1 = calendar.dateComponents([.month, .year, .day], from: date1, to: date2)
242251
XCTAssertEqual(diff1.year, 1)
243252
XCTAssertEqual(diff1.month, 5)
244253
XCTAssertEqual(diff1.isLeapMonth, false)
@@ -257,35 +266,35 @@ class TestNSDateComponents: XCTestCase {
257266
XCTAssertNil(diff1.calendar)
258267
XCTAssertNil(diff1.timeZone)
259268

260-
let diff2 = Calendar.current.dateComponents([.weekOfMonth], from: date2, to: date1)
269+
let diff2 = calendar.dateComponents([.weekOfMonth], from: date2, to: date1)
261270
XCTAssertEqual(diff2.weekOfMonth, -76)
262271
XCTAssertEqual(diff2.isLeapMonth, false)
263272

264-
let diff3 = Calendar.current.dateComponents([.weekday], from: date2, to: date1)
273+
let diff3 = calendar.dateComponents([.weekday], from: date2, to: date1)
265274
XCTAssertEqual(diff3.weekday, -536)
266275
XCTAssertEqual(diff3.isLeapMonth, false)
267276

268-
let diff4 = Calendar.current.dateComponents([.weekday, .weekOfMonth], from: date1, to: date2)
277+
let diff4 = calendar.dateComponents([.weekday, .weekOfMonth], from: date1, to: date2)
269278
XCTAssertEqual(diff4.weekday, 4)
270279
XCTAssertEqual(diff4.weekOfMonth, 76)
271280
XCTAssertEqual(diff4.isLeapMonth, false)
272281

273-
let diff5 = Calendar.current.dateComponents([.weekday, .weekOfYear], from: date1, to: date2)
282+
let diff5 = calendar.dateComponents([.weekday, .weekOfYear], from: date1, to: date2)
274283
XCTAssertEqual(diff5.weekday, 4)
275284
XCTAssertEqual(diff5.weekOfYear, 76)
276285
XCTAssertEqual(diff5.isLeapMonth, false)
277286

278-
let diff6 = Calendar.current.dateComponents([.month, .weekOfMonth], from: date1, to: date2)
287+
let diff6 = calendar.dateComponents([.month, .weekOfMonth], from: date1, to: date2)
279288
XCTAssertEqual(diff6.month, 17)
280289
XCTAssertEqual(diff6.weekOfMonth, 2)
281290
XCTAssertEqual(diff6.isLeapMonth, false)
282291

283-
let diff7 = Calendar.current.dateComponents([.weekOfYear, .weekOfMonth], from: date2, to: date1)
292+
let diff7 = calendar.dateComponents([.weekOfYear, .weekOfMonth], from: date2, to: date1)
284293
XCTAssertEqual(diff7.weekOfYear, -76)
285294
XCTAssertEqual(diff7.weekOfMonth, 0)
286295
XCTAssertEqual(diff7.isLeapMonth, false)
287296

288-
let diff8 = Calendar.current.dateComponents([.era, .quarter, .year, .month, .day, .hour, .minute, .second, .nanosecond, .calendar, .timeZone], from: date2, to: date3)
297+
let diff8 = calendar.dateComponents([.era, .quarter, .year, .month, .day, .hour, .minute, .second, .nanosecond, .calendar, .timeZone], from: date2, to: date3)
289298
XCTAssertEqual(diff8.era, 0)
290299
XCTAssertEqual(diff8.year, 315)
291300
XCTAssertEqual(diff8.quarter, 0)
@@ -299,7 +308,7 @@ class TestNSDateComponents: XCTestCase {
299308
XCTAssertNil(diff8.calendar)
300309
XCTAssertNil(diff8.timeZone)
301310

302-
let diff9 = Calendar.current.dateComponents([.era, .quarter, .year, .month, .day, .hour, .minute, .second, .nanosecond, .calendar, .timeZone], from: date4, to: date3)
311+
let diff9 = calendar.dateComponents([.era, .quarter, .year, .month, .day, .hour, .minute, .second, .nanosecond, .calendar, .timeZone], from: date4, to: date3)
303312
XCTAssertEqual(diff9.era, 0)
304313
XCTAssertEqual(diff9.year, 0)
305314
XCTAssertEqual(diff9.quarter, 0)

TestFoundation/TestDateFormatter.swift

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ class TestDateFormatter: XCTestCase {
2424
("test_dateFormatString", test_dateFormatString),
2525
("test_setLocaleToNil", test_setLocaleToNil),
2626
("test_setTimeZoneToNil", test_setTimeZoneToNil),
27+
("test_setTimeZone", test_setTimeZone),
28+
("test_expectedTimeZone", test_expectedTimeZone),
2729
("test_dateFrom", test_dateFrom),
2830
]
2931
}
@@ -358,6 +360,57 @@ class TestDateFormatter: XCTestCase {
358360
XCTAssertEqual(f.timeZone, NSTimeZone.system)
359361
}
360362

363+
func test_setTimeZone() {
364+
// Test two different time zones. Should ensure that if one
365+
// happens to be TimeZone.current, we still get a valid test.
366+
let newYork = TimeZone(identifier: "America/New_York")!
367+
let losAngeles = TimeZone(identifier: "America/Los_Angeles")!
368+
369+
XCTAssertNotEqual(newYork, losAngeles)
370+
371+
// Case 1: New York
372+
let f = DateFormatter()
373+
f.timeZone = newYork
374+
XCTAssertEqual(f.timeZone, newYork)
375+
376+
// Case 2: Los Angeles
377+
f.timeZone = losAngeles
378+
XCTAssertEqual(f.timeZone, losAngeles)
379+
}
380+
381+
func test_expectedTimeZone() {
382+
let newYork = TimeZone(identifier: "America/New_York")!
383+
let losAngeles = TimeZone(identifier: "America/Los_Angeles")!
384+
385+
XCTAssertNotEqual(newYork, losAngeles)
386+
387+
let now = Date()
388+
389+
let f = DateFormatter()
390+
f.dateFormat = "z"
391+
f.locale = Locale(identifier: "en_US_POSIX")
392+
393+
// Case 1: TimeZone.current
394+
// This case can catch some issues that cause TimeZone.current to be
395+
// treated like GMT, but it doesn't work if TimeZone.current is GMT.
396+
// If you do find an issue like this caused by this first case,
397+
// it would benefit from a more specific test that fails when
398+
// TimeZone.current is GMT as well.
399+
// (ex. TestTimeZone.test_systemTimeZoneName)
400+
401+
// Disabled because of: https://bugs.swift.org/browse/SR-8994
402+
// f.timeZone = TimeZone.current
403+
// XCTAssertEqual(f.string(from: now), TimeZone.current.abbreviation())
404+
405+
// Case 2: New York
406+
f.timeZone = newYork
407+
XCTAssertEqual(f.string(from: now), newYork.abbreviation())
408+
409+
// Case 3: Los Angeles
410+
f.timeZone = losAngeles
411+
XCTAssertEqual(f.string(from: now), losAngeles.abbreviation())
412+
}
413+
361414
func test_dateFrom() throws {
362415
let formatter = DateFormatter()
363416
formatter.timeZone = TimeZone(identifier: "UTC")

TestFoundation/TestTimeZone.swift

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77
// See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
88
//
99

10+
import CoreFoundation
11+
1012
class TestTimeZone: XCTestCase {
1113

1214
static var allTests: [(String, (TestTimeZone) -> () throws -> Void)] {
@@ -29,6 +31,7 @@ class TestTimeZone: XCTestCase {
2931

3032
("test_customMirror", test_tz_customMirror),
3133
("test_knownTimeZones", test_knownTimeZones),
34+
("test_systemTimeZoneName", test_systemTimeZoneName),
3235
]
3336
}
3437

@@ -198,4 +201,17 @@ class TestTimeZone: XCTestCase {
198201
XCTAssertNotNil(TimeZone(identifier: tz), "Cant instantiate valid timeZone: \(tz)")
199202
}
200203
}
204+
205+
func test_systemTimeZoneName() {
206+
// Ensure that the system time zone creates names the same way as creating them with an identifier.
207+
// If it isn't the same, bugs in DateFormat can result, but in this specific case, the bad length
208+
// is only visible to CoreFoundation APIs, and the Swift versions hide it, making it hard to detect.
209+
let timeZone = CFTimeZoneCopySystem()
210+
let timeZoneName = CFTimeZoneGetName(timeZone)
211+
212+
let createdTimeZone = TimeZone(identifier: TimeZone.current.identifier)!
213+
214+
XCTAssertEqual(CFStringGetLength(timeZoneName), TimeZone.current.identifier.count)
215+
XCTAssertEqual(CFStringGetLength(timeZoneName), createdTimeZone.identifier.count)
216+
}
201217
}

0 commit comments

Comments
 (0)