Skip to content

Commit 5ef4942

Browse files
enh-googleraphlinus
enh-google
authored and
raphlinus
committed
Fix Time.parse and Time.parse3339 crashing bugs. DO NOT MERGE
Two reported by users, the other spotted by inspection. Bug: http://code.google.com/p/android/issues/detail?id=16002 Bug: http://code.google.com/p/android/issues/detail?id=22225 Change-Id: I86fe022fda4af68e5a6fb9dc5dd2abdb75e9d966 This was committed to master, cherry-picking to jb-mr1-dev
1 parent 37ee534 commit 5ef4942

File tree

2 files changed

+96
-99
lines changed

2 files changed

+96
-99
lines changed

‎core/java/android/text/format/Time.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -437,6 +437,9 @@ public String format(String format) {
437437
* @throws android.util.TimeFormatException if s cannot be parsed.
438438
*/
439439
public boolean parse(String s) {
440+
if (s == null) {
441+
throw new NullPointerException("time string is null");
442+
}
440443
if (nativeParse(s)) {
441444
timezone = TIMEZONE_UTC;
442445
return true;

‎core/jni/android_text_format_Time.cpp‎

Lines changed: 93 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#include "jni.h"
2424
#include "utils/misc.h"
2525
#include "android_runtime/AndroidRuntime.h"
26+
#include "ScopedStringChars.h"
2627
#include "TimeUtils.h"
2728
#include <nativehelper/JNIHelp.h>
2829
#include <cutils/tztime.h>
@@ -71,11 +72,10 @@ static inline bool java2time(JNIEnv* env, Time* t, jobject o)
7172
t->t.tm_gmtoff = env->GetLongField(o, g_gmtoffField);
7273
bool allDay = env->GetBooleanField(o, g_allDayField);
7374
if (allDay &&
74-
((t->t.tm_sec !=0) || (t->t.tm_min != 0) || (t->t.tm_hour != 0))) {
75-
char msg[100];
76-
sprintf(msg, "allDay is true but sec, min, hour are not 0.");
77-
jniThrowException(env, "java/lang/IllegalArgumentException", msg);
78-
return false;
75+
((t->t.tm_sec !=0) || (t->t.tm_min != 0) || (t->t.tm_hour != 0))) {
76+
jniThrowException(env, "java/lang/IllegalArgumentException",
77+
"allDay is true but sec, min, hour are not 0.");
78+
return false;
7979
}
8080
return true;
8181
}
@@ -308,7 +308,7 @@ static jstring android_text_format_Time_format(JNIEnv* env, jobject This,
308308
static jstring android_text_format_Time_toString(JNIEnv* env, jobject This)
309309
{
310310
Time t;
311-
if (!java2time(env, &t, This)) return env->NewStringUTF("");;
311+
if (!java2time(env, &t, This)) return env->NewStringUTF("");
312312
ACQUIRE_TIMEZONE(This, t)
313313

314314
String8 r = t.toString();
@@ -360,32 +360,30 @@ static void android_text_format_Time_set(JNIEnv* env, jobject This, jlong millis
360360
// ============================================================================
361361
// Just do this here because it's not worth recreating the strings
362362

363-
static int get_char(JNIEnv* env, const jchar *s, int spos, int mul,
364-
bool *thrown)
363+
static int get_char(JNIEnv* env, const ScopedStringChars& s, int spos, int mul,
364+
bool* thrown)
365365
{
366366
jchar c = s[spos];
367367
if (c >= '0' && c <= '9') {
368368
return (c - '0') * mul;
369369
} else {
370370
if (!*thrown) {
371-
char msg[100];
372-
sprintf(msg, "Parse error at pos=%d", spos);
373-
jniThrowException(env, "android/util/TimeFormatException", msg);
371+
jniThrowExceptionFmt(env, "android/util/TimeFormatException",
372+
"Parse error at pos=%d", spos);
374373
*thrown = true;
375374
}
376375
return 0;
377376
}
378377
}
379378

380-
static bool check_char(JNIEnv* env, const jchar *s, int spos, jchar expected)
379+
static bool check_char(JNIEnv* env, const ScopedStringChars& s, int spos, jchar expected)
381380
{
382381
jchar c = s[spos];
383382
if (c != expected) {
384-
char msg[100];
385-
sprintf(msg, "Unexpected character 0x%02x at pos=%d. Expected %c.", c, spos,
386-
expected);
387-
jniThrowException(env, "android/util/TimeFormatException", msg);
388-
return false;
383+
jniThrowExceptionFmt(env, "android/util/TimeFormatException",
384+
"Unexpected character 0x%02x at pos=%d. Expected %c.",
385+
c, spos, expected);
386+
return false;
389387
}
390388
return true;
391389
}
@@ -394,20 +392,19 @@ static bool check_char(JNIEnv* env, const jchar *s, int spos, jchar expected)
394392
static jboolean android_text_format_Time_parse(JNIEnv* env, jobject This, jstring strObj)
395393
{
396394
jsize len = env->GetStringLength(strObj);
397-
const jchar *s = env->GetStringChars(strObj, NULL);
395+
if (len < 8) {
396+
jniThrowException(env, "android/util/TimeFormatException",
397+
"String too short -- expected at least 8 characters.");
398+
return false;
399+
}
398400

399-
bool thrown = false;
400-
int n;
401401
jboolean inUtc = false;
402402

403-
if (len < 8) {
404-
char msg[100];
405-
sprintf(msg, "String too short -- expected at least 8 characters.");
406-
jniThrowException(env, "android/util/TimeFormatException", msg);
407-
return false;
408-
}
403+
ScopedStringChars s(env, strObj);
409404

410405
// year
406+
int n;
407+
bool thrown = false;
411408
n = get_char(env, s, 0, 1000, &thrown);
412409
n += get_char(env, s, 1, 100, &thrown);
413410
n += get_char(env, s, 2, 10, &thrown);
@@ -454,7 +451,7 @@ static jboolean android_text_format_Time_parse(JNIEnv* env, jobject This, jstrin
454451
if (len > 15) {
455452
// Z
456453
if (!check_char(env, s, 15, 'Z')) return false;
457-
inUtc = true;
454+
inUtc = true;
458455
}
459456
} else {
460457
env->SetBooleanField(This, g_allDayField, JNI_TRUE);
@@ -467,8 +464,7 @@ static jboolean android_text_format_Time_parse(JNIEnv* env, jobject This, jstrin
467464
env->SetIntField(This, g_ydayField, 0);
468465
env->SetIntField(This, g_isdstField, -1);
469466
env->SetLongField(This, g_gmtoffField, 0);
470-
471-
env->ReleaseStringChars(strObj, s);
467+
472468
return inUtc;
473469
}
474470

@@ -477,19 +473,19 @@ static jboolean android_text_format_Time_parse3339(JNIEnv* env,
477473
jstring strObj)
478474
{
479475
jsize len = env->GetStringLength(strObj);
480-
const jchar *s = env->GetStringChars(strObj, NULL);
481-
482-
bool thrown = false;
483-
int n;
484-
jboolean inUtc = false;
485-
486476
if (len < 10) {
487477
jniThrowException(env, "android/util/TimeFormatException",
488-
"Time input is too short; must be at least 10 characters");
478+
"String too short --- expected at least 10 characters.");
489479
return false;
490480
}
491481

482+
jboolean inUtc = false;
483+
484+
ScopedStringChars s(env, strObj);
485+
492486
// year
487+
int n;
488+
bool thrown = false;
493489
n = get_char(env, s, 0, 1000, &thrown);
494490
n += get_char(env, s, 1, 100, &thrown);
495491
n += get_char(env, s, 2, 10, &thrown);
@@ -520,28 +516,28 @@ static jboolean android_text_format_Time_parse3339(JNIEnv* env,
520516
// T
521517
if (!check_char(env, s, 10, 'T')) return false;
522518

523-
env->SetBooleanField(This, g_allDayField, JNI_FALSE);
519+
env->SetBooleanField(This, g_allDayField, JNI_FALSE);
524520
// hour
525521
n = get_char(env, s, 11, 10, &thrown);
526522
n += get_char(env, s, 12, 1, &thrown);
527523
if (thrown) return false;
528-
int hour = n;
524+
int hour = n;
529525
// env->SetIntField(This, g_hourField, n);
530-
531-
// :
532-
if (!check_char(env, s, 13, ':')) return false;
533526

534-
// minute
527+
// :
528+
if (!check_char(env, s, 13, ':')) return false;
529+
530+
// minute
535531
n = get_char(env, s, 14, 10, &thrown);
536532
n += get_char(env, s, 15, 1, &thrown);
537533
if (thrown) return false;
538-
int minute = n;
534+
int minute = n;
539535
// env->SetIntField(This, g_minField, n);
540536

541-
// :
542-
if (!check_char(env, s, 16, ':')) return false;
537+
// :
538+
if (!check_char(env, s, 16, ':')) return false;
543539

544-
// second
540+
// second
545541
n = get_char(env, s, 17, 10, &thrown);
546542
n += get_char(env, s, 18, 1, &thrown);
547543
if (thrown) return false;
@@ -561,64 +557,63 @@ static jboolean android_text_format_Time_parse3339(JNIEnv* env,
561557
if (len > tz_index) {
562558
char c = s[tz_index];
563559

564-
// NOTE: the offset is meant to be subtracted to get from local time
565-
// to UTC. we therefore use 1 for '-' and -1 for '+'.
566-
switch (c) {
567-
case 'Z':
568-
// Zulu time -- UTC
569-
offset = 0;
570-
break;
571-
case '-':
560+
// NOTE: the offset is meant to be subtracted to get from local time
561+
// to UTC. we therefore use 1 for '-' and -1 for '+'.
562+
switch (c) {
563+
case 'Z':
564+
// Zulu time -- UTC
565+
offset = 0;
566+
break;
567+
case '-':
572568
offset = 1;
573-
break;
574-
case '+':
569+
break;
570+
case '+':
575571
offset = -1;
576-
break;
577-
default:
578-
char msg[100];
579-
sprintf(msg, "Unexpected character 0x%02x at position %d. Expected + or -",
580-
c, tz_index);
581-
jniThrowException(env, "android/util/TimeFormatException", msg);
582-
return false;
583-
}
572+
break;
573+
default:
574+
jniThrowExceptionFmt(env, "android/util/TimeFormatException",
575+
"Unexpected character 0x%02x at position %d. Expected + or -",
576+
c, tz_index);
577+
return false;
578+
}
584579
inUtc = true;
585580

586-
if (offset != 0) {
587-
if (len < tz_index + 6) {
588-
char msg[100];
589-
sprintf(msg, "Unexpected length; should be %d characters", tz_index + 6);
590-
jniThrowException(env, "android/util/TimeFormatException", msg);
591-
return false;
592-
}
593-
594-
// hour
595-
n = get_char(env, s, tz_index + 1, 10, &thrown);
596-
n += get_char(env, s, tz_index + 2, 1, &thrown);
597-
if (thrown) return false;
598-
n *= offset;
599-
hour += n;
600-
601-
// :
602-
if (!check_char(env, s, tz_index + 3, ':')) return false;
603-
604-
// minute
605-
n = get_char(env, s, tz_index + 4, 10, &thrown);
606-
n += get_char(env, s, tz_index + 5, 1, &thrown);
607-
if (thrown) return false;
608-
n *= offset;
609-
minute += n;
610-
}
611-
}
612-
env->SetIntField(This, g_hourField, hour);
581+
if (offset != 0) {
582+
if (len < tz_index + 6) {
583+
jniThrowExceptionFmt(env, "android/util/TimeFormatException",
584+
"Unexpected length; should be %d characters",
585+
tz_index + 6);
586+
return false;
587+
}
588+
589+
// hour
590+
n = get_char(env, s, tz_index + 1, 10, &thrown);
591+
n += get_char(env, s, tz_index + 2, 1, &thrown);
592+
if (thrown) return false;
593+
n *= offset;
594+
hour += n;
595+
596+
// :
597+
if (!check_char(env, s, tz_index + 3, ':')) return false;
598+
599+
// minute
600+
n = get_char(env, s, tz_index + 4, 10, &thrown);
601+
n += get_char(env, s, tz_index + 5, 1, &thrown);
602+
if (thrown) return false;
603+
n *= offset;
604+
minute += n;
605+
}
606+
}
607+
env->SetIntField(This, g_hourField, hour);
613608
env->SetIntField(This, g_minField, minute);
614609

615-
if (offset != 0) {
616-
// we need to normalize after applying the hour and minute offsets
617-
android_text_format_Time_normalize(env, This, false /* use isdst */);
618-
// The timezone is set to UTC in the calling Java code.
619-
}
610+
if (offset != 0) {
611+
// we need to normalize after applying the hour and minute offsets
612+
android_text_format_Time_normalize(env, This, false /* use isdst */);
613+
// The timezone is set to UTC in the calling Java code.
614+
}
620615
} else {
621-
env->SetBooleanField(This, g_allDayField, JNI_TRUE);
616+
env->SetBooleanField(This, g_allDayField, JNI_TRUE);
622617
env->SetIntField(This, g_hourField, 0);
623618
env->SetIntField(This, g_minField, 0);
624619
env->SetIntField(This, g_secField, 0);
@@ -628,8 +623,7 @@ static jboolean android_text_format_Time_parse3339(JNIEnv* env,
628623
env->SetIntField(This, g_ydayField, 0);
629624
env->SetIntField(This, g_isdstField, -1);
630625
env->SetLongField(This, g_gmtoffField, 0);
631-
632-
env->ReleaseStringChars(strObj, s);
626+
633627
return inUtc;
634628
}
635629

0 commit comments

Comments
 (0)