From 17f616823a562ceb3a008f91e05d43bc56d37cae Mon Sep 17 00:00:00 2001 From: Jake Hamby Date: Fri, 13 Aug 2010 12:09:56 -0700 Subject: Fix character count bug and Javadoc typos in SMS (with test cases) Fix a character count bug I discovered while working on related SMS bugs. Includes a new set of test cases to verify the fix for the buggy calculateLength() methods ("runtest frameworks-telephony"). You can also verify that the counter works properly in the Mms app by typing characters until the boundary is crossed where an additional message part is required. The counter should count down to 0 characters remaining before increasing the message count. Change-Id: I4de68b82dfc53dcae094865798f2c0235a355d43 --- .../java/android/telephony/gsm/SmsMessage.java | 12 +- .../android/internal/telephony/GsmAlphabet.java | 2 +- .../internal/telephony/cdma/SmsMessage.java | 6 +- .../internal/telephony/cdma/sms/BearerData.java | 15 +- .../android/internal/telephony/gsm/SmsMessage.java | 14 +- .../internal/telephony/SmsMessageBodyTest.java | 194 +++++++++++++++++++++ 6 files changed, 220 insertions(+), 23 deletions(-) create mode 100644 telephony/tests/telephonytests/src/com/android/internal/telephony/SmsMessageBodyTest.java (limited to 'telephony') diff --git a/telephony/java/android/telephony/gsm/SmsMessage.java b/telephony/java/android/telephony/gsm/SmsMessage.java index 37ef912..0c63c37 100644 --- a/telephony/java/android/telephony/gsm/SmsMessage.java +++ b/telephony/java/android/telephony/gsm/SmsMessage.java @@ -304,9 +304,9 @@ public class SmsMessage { int septets = GsmAlphabet.countGsmSeptets(messageBody, !use7bitOnly); ret[1] = septets; if (septets > MAX_USER_DATA_SEPTETS) { - ret[0] = (septets / MAX_USER_DATA_SEPTETS_WITH_HEADER) + 1; - ret[2] = MAX_USER_DATA_SEPTETS_WITH_HEADER - - (septets % MAX_USER_DATA_SEPTETS_WITH_HEADER); + ret[0] = (septets + (MAX_USER_DATA_SEPTETS_WITH_HEADER - 1)) / + MAX_USER_DATA_SEPTETS_WITH_HEADER; + ret[2] = (ret[0] * MAX_USER_DATA_SEPTETS_WITH_HEADER) - septets; } else { ret[0] = 1; ret[2] = MAX_USER_DATA_SEPTETS - septets; @@ -318,9 +318,9 @@ public class SmsMessage { ret[1] = messageBody.length(); if (octets > MAX_USER_DATA_BYTES) { // 6 is the size of the user data header - ret[0] = (octets / MAX_USER_DATA_BYTES_WITH_HEADER) + 1; - ret[2] = (MAX_USER_DATA_BYTES_WITH_HEADER - - (octets % MAX_USER_DATA_BYTES_WITH_HEADER))/2; + ret[0] = (octets + (MAX_USER_DATA_BYTES_WITH_HEADER - 1)) / + MAX_USER_DATA_BYTES_WITH_HEADER; + ret[2] = ((ret[0] * MAX_USER_DATA_BYTES_WITH_HEADER) - octets) / 2; } else { ret[0] = 1; ret[2] = (MAX_USER_DATA_BYTES - octets)/2; diff --git a/telephony/java/com/android/internal/telephony/GsmAlphabet.java b/telephony/java/com/android/internal/telephony/GsmAlphabet.java index ebdd220..7edf065 100644 --- a/telephony/java/com/android/internal/telephony/GsmAlphabet.java +++ b/telephony/java/com/android/internal/telephony/GsmAlphabet.java @@ -23,7 +23,7 @@ import android.util.Log; /** * This class implements the character set mapping between - * the GSM SMS 7-bit alphabet specifed in TS 23.038 6.2.1 + * the GSM SMS 7-bit alphabet specified in TS 23.038 6.2.1 * and UTF-16 * * {@hide} diff --git a/telephony/java/com/android/internal/telephony/cdma/SmsMessage.java b/telephony/java/com/android/internal/telephony/cdma/SmsMessage.java index b50502c..0f3b8ff 100755 --- a/telephony/java/com/android/internal/telephony/cdma/SmsMessage.java +++ b/telephony/java/com/android/internal/telephony/cdma/SmsMessage.java @@ -287,7 +287,7 @@ public class SmsMessage extends SmsMessageBase { * @param destAddr Address of the recipient. * @param message String representation of the message payload. * @param statusReportRequested Indicates whether a report is requested for this message. - * @param headerData Array containing the data for the User Data Header, preceded + * @param smsHeader Array containing the data for the User Data Header, preceded * by the Element Identifiers. * @return a SubmitPdu containing the encoded SC * address, if applicable, and the encoded message. @@ -355,7 +355,7 @@ public class SmsMessage extends SmsMessageBase { * Get an SMS-SUBMIT PDU for a data message to a destination address & port * * @param destAddr the address of the destination for the message - * @param userDara the data for the message + * @param userData the data for the message * @param statusReportRequested Indicates whether a report is requested for this message. * @return a SubmitPdu containing the encoded SC * address, if applicable, and the encoded message. @@ -446,7 +446,7 @@ public class SmsMessage extends SmsMessageBase { */ public static TextEncodingDetails calculateLength(CharSequence messageBody, boolean use7bitOnly) { - return BearerData.calcTextEncodingDetails(messageBody.toString(), use7bitOnly); + return BearerData.calcTextEncodingDetails(messageBody, use7bitOnly); } /** diff --git a/telephony/java/com/android/internal/telephony/cdma/sms/BearerData.java b/telephony/java/com/android/internal/telephony/cdma/sms/BearerData.java index c7032ac..ab79fe9 100644 --- a/telephony/java/com/android/internal/telephony/cdma/sms/BearerData.java +++ b/telephony/java/com/android/internal/telephony/cdma/sms/BearerData.java @@ -405,7 +405,8 @@ public final class BearerData { /** * Calculate the message text encoding length, fragmentation, and other details. * - * @param force ignore (but still count) illegal characters if true + * @param msg message text + * @param force7BitEncoding ignore (but still count) illegal characters if true * @return septet count, or -1 on failure */ public static TextEncodingDetails calcTextEncodingDetails(CharSequence msg, @@ -427,9 +428,10 @@ public final class BearerData { ted.codeUnitCount = msg.length(); int octets = ted.codeUnitCount * 2; if (octets > MAX_USER_DATA_BYTES) { - ted.msgCount = (octets / MAX_USER_DATA_BYTES_WITH_HEADER) + 1; - ted.codeUnitsRemaining = (MAX_USER_DATA_BYTES_WITH_HEADER - - (octets % MAX_USER_DATA_BYTES_WITH_HEADER))/2; + ted.msgCount = (octets + (MAX_USER_DATA_BYTES_WITH_HEADER - 1)) / + MAX_USER_DATA_BYTES_WITH_HEADER; + ted.codeUnitsRemaining = ((ted.msgCount * + MAX_USER_DATA_BYTES_WITH_HEADER) - octets) / 2; } else { ted.msgCount = 1; ted.codeUnitsRemaining = (MAX_USER_DATA_BYTES - octets)/2; @@ -802,9 +804,8 @@ public final class BearerData { * Create serialized representation for BearerData object. * (See 3GPP2 C.R1001-F, v1.0, section 4.5 for layout details) * - * @param bearerData an instance of BearerData. - * - * @return data byta array of raw encoded SMS bearer data. + * @param bData an instance of BearerData. + * @return byte array of raw encoded SMS bearer data. */ public static byte[] encode(BearerData bData) { bData.hasUserDataHeader = ((bData.userData != null) && diff --git a/telephony/java/com/android/internal/telephony/gsm/SmsMessage.java b/telephony/java/com/android/internal/telephony/gsm/SmsMessage.java index 12c6b88..50dd402 100644 --- a/telephony/java/com/android/internal/telephony/gsm/SmsMessage.java +++ b/telephony/java/com/android/internal/telephony/gsm/SmsMessage.java @@ -800,9 +800,10 @@ public class SmsMessage extends SmsMessageBase{ int septets = GsmAlphabet.countGsmSeptets(msgBody, !use7bitOnly); ted.codeUnitCount = septets; if (septets > MAX_USER_DATA_SEPTETS) { - ted.msgCount = (septets / MAX_USER_DATA_SEPTETS_WITH_HEADER) + 1; - ted.codeUnitsRemaining = MAX_USER_DATA_SEPTETS_WITH_HEADER - - (septets % MAX_USER_DATA_SEPTETS_WITH_HEADER); + ted.msgCount = (septets + (MAX_USER_DATA_SEPTETS_WITH_HEADER - 1)) / + MAX_USER_DATA_SEPTETS_WITH_HEADER; + ted.codeUnitsRemaining = (ted.msgCount * + MAX_USER_DATA_SEPTETS_WITH_HEADER) - septets; } else { ted.msgCount = 1; ted.codeUnitsRemaining = MAX_USER_DATA_SEPTETS - septets; @@ -812,9 +813,10 @@ public class SmsMessage extends SmsMessageBase{ int octets = msgBody.length() * 2; ted.codeUnitCount = msgBody.length(); if (octets > MAX_USER_DATA_BYTES) { - ted.msgCount = (octets / MAX_USER_DATA_BYTES_WITH_HEADER) + 1; - ted.codeUnitsRemaining = (MAX_USER_DATA_BYTES_WITH_HEADER - - (octets % MAX_USER_DATA_BYTES_WITH_HEADER))/2; + ted.msgCount = (octets + (MAX_USER_DATA_BYTES_WITH_HEADER - 1)) / + MAX_USER_DATA_BYTES_WITH_HEADER; + ted.codeUnitsRemaining = ((ted.msgCount * + MAX_USER_DATA_BYTES_WITH_HEADER) - octets) / 2; } else { ted.msgCount = 1; ted.codeUnitsRemaining = (MAX_USER_DATA_BYTES - octets)/2; diff --git a/telephony/tests/telephonytests/src/com/android/internal/telephony/SmsMessageBodyTest.java b/telephony/tests/telephonytests/src/com/android/internal/telephony/SmsMessageBodyTest.java new file mode 100644 index 0000000..b214887 --- /dev/null +++ b/telephony/tests/telephonytests/src/com/android/internal/telephony/SmsMessageBodyTest.java @@ -0,0 +1,194 @@ +/* + * Copyright (C) 2010 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.internal.telephony; + +import android.telephony.SmsMessage; +import android.telephony.TelephonyManager; +import android.test.AndroidTestCase; +import android.test.suitebuilder.annotation.SmallTest; + +import static android.telephony.SmsMessage.MAX_USER_DATA_SEPTETS; + +public class SmsMessageBodyTest extends AndroidTestCase { + + private static final String sAsciiChars = "@$_ !\"#%&'()*+,-./0123456789" + + ":;<=>?ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz\n\r"; + private static final String sGsmBasicChars = "\u00a3\u00a5\u00e8\u00e9" + + "\u00f9\u00ec\u00f2\u00c7\u00d8\u00f8\u00c5\u00e5\u0394\u03a6" + + "\u0393\u039b\u03a9\u03a0\u03a8\u03a3\u0398\u00c6\u00e6" + + "\u00df\u00c9\u00a4\u00a1\u00c4\u00d6\u00d1\u00dc\u00a7\u00bf" + + "\u00e4\u00f6\u00f1\u00fc\u00e0"; + private static final String sGsmExtendedAsciiChars = "{|}\\[~]^\f"; + private static final String sGsmExtendedEuroSymbol = "\u20ac"; + private static final String sUnicodeChars = "\u4e00\u4e01\u4e02\u4e03" + + "\u4e04\u4e05\u4e06\u4e07\u4e08\u4e09\u4e0a\u4e0b\u4e0c\u4e0d" + + "\u4e0e\u4e0f\u3041\u3042\u3043\u3044\u3045\u3046\u3047\u3048" + + "\u30a1\u30a2\u30a3\u30a4\u30a5\u30a6\u30a7\u30a8" + + "\uff10\uff11\uff12\uff13\uff14\uff15\uff16\uff17\uff18" + + "\uff70\uff71\uff72\uff73\uff74\uff75\uff76\uff77\uff78" + + "\u0400\u0401\u0402\u0403\u0404\u0405\u0406\u0407\u0408" + + "\u00a2\u00a9\u00ae\u2122"; + + private static final int sTestLengthCount = 12; + + private static final int[] sSeptetTestLengths = + { 0, 1, 2, 80, 159, 160, 161, 240, 305, 306, 307, 320}; + + private static final int[] sUnicodeTestLengths = + { 0, 1, 2, 35, 69, 70, 71, 100, 133, 134, 135, 160}; + + private static final int[] sTestMsgCounts = + { 1, 1, 1, 1, 1, 1, 2, 2, 2, 2, 3, 3}; + + private static final int[] sSeptetUnitsRemaining = + {160, 159, 158, 80, 1, 0, 145, 66, 1, 0, 152, 139}; + + private static final int[] sUnicodeUnitsRemaining = + { 70, 69, 68, 35, 1, 0, 63, 34, 1, 0, 66, 41}; + + + @SmallTest + public void testCalcLengthAscii() throws Exception { + StringBuilder sb = new StringBuilder(320); + int[] values = {0, 0, 0, SmsMessage.ENCODING_7BIT}; + int startPos = 0; + int asciiCharsLen = sAsciiChars.length(); + + for (int i = 0; i < sTestLengthCount; i++) { + int len = sSeptetTestLengths[i]; + assertTrue(sb.length() <= len); + + while (sb.length() < len) { + int addCount = len - sb.length(); + int endPos = (asciiCharsLen - startPos > addCount) ? + (startPos + addCount) : asciiCharsLen; + sb.append(sAsciiChars, startPos, endPos); + startPos = (endPos == asciiCharsLen) ? 0 : endPos; + } + assertEquals(len, sb.length()); + + String testStr = sb.toString(); + values[0] = sTestMsgCounts[i]; + values[1] = len; + values[2] = sSeptetUnitsRemaining[i]; + + callGsmLengthMethods(testStr, false, values); + callGsmLengthMethods(testStr, true, values); + callCdmaLengthMethods(testStr, false, values); + callCdmaLengthMethods(testStr, true, values); + } + } + + @SmallTest + public void testCalcLength7bitGsm() throws Exception { + // TODO + } + + @SmallTest + public void testCalcLength7bitGsmExtended() throws Exception { + // TODO + } + + @SmallTest + public void testCalcLengthUnicode() throws Exception { + StringBuilder sb = new StringBuilder(160); + int[] values = {0, 0, 0, SmsMessage.ENCODING_16BIT}; + int[] values7bit = {1, 0, 0, SmsMessage.ENCODING_7BIT}; + int startPos = 0; + int unicodeCharsLen = sUnicodeChars.length(); + + // start with length 1: empty string uses ENCODING_7BIT + for (int i = 1; i < sTestLengthCount; i++) { + int len = sUnicodeTestLengths[i]; + assertTrue(sb.length() <= len); + + while (sb.length() < len) { + int addCount = len - sb.length(); + int endPos = (unicodeCharsLen - startPos > addCount) ? + (startPos + addCount) : unicodeCharsLen; + sb.append(sUnicodeChars, startPos, endPos); + startPos = (endPos == unicodeCharsLen) ? 0 : endPos; + } + assertEquals(len, sb.length()); + + String testStr = sb.toString(); + values[0] = sTestMsgCounts[i]; + values[1] = len; + values[2] = sUnicodeUnitsRemaining[i]; + values7bit[1] = len; + values7bit[2] = MAX_USER_DATA_SEPTETS - len; + + callGsmLengthMethods(testStr, false, values); + callCdmaLengthMethods(testStr, false, values); + callGsmLengthMethods(testStr, true, values7bit); + callCdmaLengthMethods(testStr, true, values7bit); + } + } + + private void callGsmLengthMethods(CharSequence msgBody, boolean use7bitOnly, + int[] expectedValues) + { + // deprecated GSM-specific method + int[] values = android.telephony.gsm.SmsMessage.calculateLength(msgBody, use7bitOnly); + assertEquals("msgCount", expectedValues[0], values[0]); + assertEquals("codeUnitCount", expectedValues[1], values[1]); + assertEquals("codeUnitsRemaining", expectedValues[2], values[2]); + assertEquals("codeUnitSize", expectedValues[3], values[3]); + + int activePhone = TelephonyManager.getDefault().getPhoneType(); + if (TelephonyManager.PHONE_TYPE_GSM == activePhone) { + values = android.telephony.SmsMessage.calculateLength(msgBody, use7bitOnly); + assertEquals("msgCount", expectedValues[0], values[0]); + assertEquals("codeUnitCount", expectedValues[1], values[1]); + assertEquals("codeUnitsRemaining", expectedValues[2], values[2]); + assertEquals("codeUnitSize", expectedValues[3], values[3]); + } + + SmsMessageBase.TextEncodingDetails ted = + com.android.internal.telephony.gsm.SmsMessage.calculateLength(msgBody, use7bitOnly); + assertEquals("msgCount", expectedValues[0], ted.msgCount); + assertEquals("codeUnitCount", expectedValues[1], ted.codeUnitCount); + assertEquals("codeUnitsRemaining", expectedValues[2], ted.codeUnitsRemaining); + assertEquals("codeUnitSize", expectedValues[3], ted.codeUnitSize); + } + + private void callCdmaLengthMethods(CharSequence msgBody, boolean use7bitOnly, + int[] expectedValues) + { + int activePhone = TelephonyManager.getDefault().getPhoneType(); + if (TelephonyManager.PHONE_TYPE_CDMA == activePhone) { + int[] values = android.telephony.SmsMessage.calculateLength(msgBody, use7bitOnly); + assertEquals("msgCount", expectedValues[0], values[0]); + assertEquals("codeUnitCount", expectedValues[1], values[1]); + assertEquals("codeUnitsRemaining", expectedValues[2], values[2]); + assertEquals("codeUnitSize", expectedValues[3], values[3]); + } + + SmsMessageBase.TextEncodingDetails ted = + com.android.internal.telephony.cdma.SmsMessage.calculateLength(msgBody, use7bitOnly); + assertEquals("msgCount", expectedValues[0], ted.msgCount); + assertEquals("codeUnitCount", expectedValues[1], ted.codeUnitCount); + assertEquals("codeUnitsRemaining", expectedValues[2], ted.codeUnitsRemaining); + assertEquals("codeUnitSize", expectedValues[3], ted.codeUnitSize); + + ted = com.android.internal.telephony.cdma.sms.BearerData.calcTextEncodingDetails(msgBody, use7bitOnly); + assertEquals("msgCount", expectedValues[0], ted.msgCount); + assertEquals("codeUnitCount", expectedValues[1], ted.codeUnitCount); + assertEquals("codeUnitsRemaining", expectedValues[2], ted.codeUnitsRemaining); + assertEquals("codeUnitSize", expectedValues[3], ted.codeUnitSize); + } +} -- cgit v1.1