Closed
Bug 794927
Opened 12 years ago
Closed 7 years ago
Unsupported format strings in Date().toLocaleFormat abort on Windows
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
DUPLICATE
of bug 818634
People
(Reporter: benjamin, Unassigned)
References
Details
(Keywords: csectype-dos, Whiteboard: [js:p2])
On Windows, calling Date().toLocaleFormat('%R') crashes. %R is a shortcut for %H:%M in the Linux strftime, but is not supported on Windows and I think any unsupported % character causes the CRT to abort on Windows. See bug 794662 for some stacks where this hit B2G.
Comment 1•12 years ago
|
||
See also bug 806231 about %e causing crashes.
Updated•12 years ago
|
Whiteboard: [js:p2]
Comment 2•12 years ago
|
||
I reported on this over in bug 681704. Weave hits this in metrofx constantly, crashing the browser. I've seen this on two observer events - weave:service:login:start and weave:service:login:finish.
Updated•12 years ago
|
Whiteboard: [js:p2] → [js:p2][metro-mvp]
Comment 3•12 years ago
|
||
Sync doesn't use %R:
service.js
1155: let dateStr = new Date().toLocaleFormat(LOG_DATE_FORMAT);
stages/enginesync.js
180: let dateStr = new Date().toLocaleFormat(LOG_DATE_FORMAT);
constants.js
179:LOG_DATE_FORMAT: "%Y-%m-%d %H:%M:%S",
Comment 4•12 years ago
|
||
Ohhhh, you're running old XUL code.
mobile/xul/chrome/content/sync.js
479: let syncDate = new Date(lastSync).toLocaleFormat("%a %R");
Try copying desktop instead:
browser/base/content/browser-syncui.js
296: let lastSyncDate = new Date(lastSync).toLocaleFormat("%a %H:%M");
You might find more things like this; we haven't maintained XUL Sync code for about 15 months.
Comment 5•12 years ago
|
||
I will happily r+ the change from %R to %H:%M if you want to land it on Elm, Jim.
Comment 6•12 years ago
|
||
(In reply to Richard Newman [:rnewman] from comment #5)
> I will happily r+ the change from %R to %H:%M if you want to land it on Elm,
> Jim.
Working on it now. Thanks for the f(In reply to Richard Newman [:rnewman] from comment #5)
> I will happily r+ the change from %R to %H:%M if you want to land it on Elm,
> Jim.
Thanks, spun off in bug 827767.
Updated•12 years ago
|
Summary: Date().toLocaleFormat('%R') crashes → Unsupported format strings in Date().toLocaleFormat abort on Windows
Whiteboard: [js:p2][metro-mvp] → [js:p2]
Comment 7•12 years ago
|
||
Invalid format strings abort too:
alert((new Date).toLocaleFormat("%"));
What's the reason for aborting? Seems like it should be a warning, at worst.
Comment 8•12 years ago
|
||
This is getting passed directly to the Windows CRT which is doing the aborting. I don't think this is actually intentional on the JS side, just a lack of sanitization on the input.
Comment 9•12 years ago
|
||
http://mxr.mozilla.org/mozilla-central/source/js/src/prmjtime.cpp#499
_CrtSetReportMode(_CRT_ASSERT, 0);
Added in bug 395836. I guess the default is to crash/throw, and we changed it to a debug-only assert?
Comment 10•12 years ago
|
||
Oh. Apparently we shot ourselves in the foot here in bug 587982:
http://mxr.mozilla.org/mozilla-central/source/toolkit/xre/nsAppRunner.cpp#4310
http://mxr.mozilla.org/mozilla-central/source/toolkit/xre/nsAppRunner.cpp#1352
We made CRT assertions intentionally mozalloc_abort on Windows. I guess you could slip a _CrtSetReportHook into prmjtime as well.
These bother me on a fundamental level, since these are all *process global*. I don't know of another way to handle this aside from not using Microsoft's implementation of strftime.
Comment 11•12 years ago
|
||
Is this just a question of whether _CRT_ASSERT should abort or not?
Comment 12•12 years ago
|
||
I don't know about in the general case, but in this specific case we pass the format string directly from JavaScript down to strftime, so it's trivially easy to make this abort from content JS (in a debug build).
Assignee | ||
Updated•10 years ago
|
Assignee: general → nobody
Comment 13•7 years ago
|
||
toLocaleFormat was removed in bug 818634.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → DUPLICATE
You need to log in
before you can comment on or make changes to this bug.
Description
•