clean up cdma sms creation and parsing

related to issue http://b/issue?id=1782245

- fixes 7bit ASCII encode and decode (previous completely broken)

- also consolidates encoding of user data, and changed to match
  the conventions of the new data coding -- previously likely
  broken for several cases

4 files changed, 406 insertions, 283 deletions

diff --git a/telephony/java/com/android/internal/telephony/cdma/SmsMessage.java b/telephony/java/com/android/internal/telephony/cdma/SmsMessage.java
index 677d609..343a22e 100644
--- a/telephony/java/com/android/internal/telephony/cdma/SmsMessage.java
+++ b/telephony/java/com/android/internal/telephony/cdma/SmsMessage.java
@@ -37,9 +37,16 @@ import java.io.ByteArrayOutputStream;
 import java.io.DataInputStream;
 import java.io.DataOutputStream;
 import java.io.IOException;
-import java.io.UnsupportedEncodingException;
 import java.util.Random;
 
+/**
+ * TODO(cleanup): these constants are disturbing... are they not just
+ * different interpretations on one number?  And if we did not have
+ * terrible class name overlap, they would not need to be directly
+ * imported like this.  The class in this file could just as well be
+ * named CdmaSmsMessage, could it not?
+ */
+
 import static android.telephony.SmsMessage.MAX_USER_DATA_BYTES;
 import static android.telephony.SmsMessage.MAX_USER_DATA_BYTES_WITH_HEADER;
 import static android.telephony.SmsMessage.MAX_USER_DATA_SEPTETS;
@@ -47,6 +54,14 @@ import static android.telephony.SmsMessage.MAX_USER_DATA_SEPTETS_WITH_HEADER;
 import static android.telephony.SmsMessage.MessageClass;
 
 /**
+ * TODO(cleanup): internally returning null in many places makes
+ * debugging very hard (among many other reasons) and should be made
+ * more meaningful (replaced with execptions for example).  Null
+ * returns should only occur at the very outside of the module/class
+ * scope.
+ */
+
+/**
  * A Short Message Service message.
  *
  */
@@ -264,8 +279,8 @@ public class SmsMessage extends SmsMessageBase {
     /**
      * Get an SMS-SUBMIT PDU for a destination address and a message
      *
-     * @param scAddress             Service Centre address.  Null means use default.
-     * @param destinationAddress    Address of the recipient.
+     * @param scAddr                Service Centre address.  Null means use default.
+     * @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
@@ -275,89 +290,35 @@ public class SmsMessage extends SmsMessageBase {
      *         Returns null on encode error.
      * @hide
      */
-    public static SubmitPdu getSubmitPdu(String scAddress,
-            String destinationAddress, String message,
+    public static SubmitPdu getSubmitPdu(String scAddr,
+            String destAddr, String message,
             boolean statusReportRequested, byte[] headerData) {
+        /**
+         * TODO(cleanup): why does this method take an scAddr input
+         * and do nothing with it?  GSM allows us to specify a SC (eg,
+         * when responding to an SMS that explicitly requests the
+         * response is sent to a specific SC), or pass null to use the
+         * default value.  Is there no similar notion in CDMA? Or do
+         * we just not have it hooked up?
+         */
 
-        SmsMessage sms = new SmsMessage();
-        SubmitPdu ret = new SubmitPdu();
-        UserData uData = new UserData();
-        SmsHeader smsHeader;
-        byte[] data;
-
-        // Perform null parameter checks.
-        if (message == null || destinationAddress == null) {
+        if (message == null || destAddr == null) {
             return null;
         }
 
-        // ** Set UserData + SmsHeader **
-        try {
-            // First, try encoding it as 7-bit ASCII
-            // User Data (and length)
-
-            uData.payload = message.getBytes();
-
-            if (uData.payload.length > MAX_USER_DATA_SEPTETS) {
-                // Message too long
-                return null;
-            }
-
-            // desired TP-Data-Coding-Scheme
-            uData.msgEncoding = UserData.ENCODING_7BIT_ASCII;
-
-            // sms header
-            if(headerData != null) {
-                smsHeader = SmsHeader.parse(headerData);
-                uData.userDataHeader = smsHeader;
-            } else {
-                // no user data header available!
-            }
-
-            data  = sms.getEnvelope(destinationAddress, statusReportRequested, uData,
-                    (headerData != null), (null == headerData));
-
-        } catch (Exception ex) {
-            Log.e(LOG_TAG, "getSubmitPdu: 7 bit ASCII encoding in cdma.SMSMesage failed: "
-                    + ex.getMessage());
-            Log.w(LOG_TAG, "getSubmitPdu: The message will be sent as UCS-2 encoded message.");
-            byte[] textPart;
-            // Encoding to the 7-bit alphabet failed. Let's see if we can
-            // send it as a UCS-2 encoded message
-
-            try {
-                textPart = message.getBytes("utf-16be");
-            } catch (UnsupportedEncodingException uex) {
-                Log.e(LOG_TAG, "Implausible UnsupportedEncodingException ", uex);
-                return null;
-            }
-
-            uData.payload = textPart;
-
-            if (uData.payload.length > MAX_USER_DATA_BYTES) {
-                // Message too long
-                return null;
-            }
-
-            // TP-Data-Coding-Scheme
-            uData.msgEncoding = UserData.ENCODING_UNICODE_16;
-
-            // sms header
-            if(headerData != null) {
-                smsHeader = SmsHeader.parse(headerData);
-                uData.userDataHeader = smsHeader;
-            } else {
-                // no user data header available!
-            }
-
-            data = sms.getEnvelope(destinationAddress, statusReportRequested, uData,
-                (headerData != null), (null == headerData));
+        UserData uData = new UserData();
+        uData.payloadStr = message;
+        if(headerData != null) {
+            /**
+             * TODO(cleanup): we force the outside to deal with _all_
+             * of the raw details of properly constructing serialized
+             * headers, unserialze here, and then promptly reserialze
+             * during encoding -- rather undesirable.
+             */
+            uData.userDataHeader = SmsHeader.parse(headerData);
         }
 
-        if (null == data) return null;
-
-        ret.encodedMessage = data;
-        ret.encodedScAddress = null;
-        return ret;
+        return privateGetSubmitPdu(destAddr, statusReportRequested, uData, (headerData == null));
     }
 
 
@@ -388,42 +349,37 @@ public class SmsMessage extends SmsMessageBase {
      *         Returns null on encode error.
      */
     public static SubmitPdu getSubmitPdu(String scAddress,
-            String destinationAddress, short destinationPort, byte[] data,
+            String destAddr, short destinationPort, byte[] data,
             boolean statusReportRequested) {
 
-        SmsMessage sms = new SmsMessage();
-        SubmitPdu ret = new SubmitPdu();
-        UserData uData = new UserData();
-        SmsHeader smsHeader = new SmsHeader();
-
-        if (data.length > (MAX_USER_DATA_BYTES - 7 /* UDH size */)) {
-            Log.e(LOG_TAG, "SMS data message may only contain "
-                    + (MAX_USER_DATA_BYTES - 7) + " bytes");
-            return null;
-        }
+        /**
+         * TODO(cleanup): if we had properly exposed SmsHeader
+         * information, this mess of many getSubmitPdu public
+         * interface methods that currently pollute the api could have
+         * been much more cleanly collapsed into one.
+         */
 
+        /**
+         * TODO(cleanup): header serialization should be put somewhere
+         * canonical to allow proper debugging and reuse.
+         */
         byte[] destPort = new byte[4];
         destPort[0] = (byte) ((destinationPort >> 8) & 0xFF); // MSB of destination port
         destPort[1] = (byte) (destinationPort & 0xFF); // LSB of destination port
         destPort[2] = 0x00; // MSB of originating port
         destPort[3] = 0x00; // LSB of originating port
+        SmsHeader smsHeader = new SmsHeader();
         smsHeader.add(
                 new SmsHeader.Element(SmsHeader.APPLICATION_PORT_ADDRESSING_16_BIT, destPort));
-
         smsHeader.nbrOfHeaders = smsHeader.getElements().size();
-        uData.userDataHeader = smsHeader;
 
-        // TP-Data-Coding-Scheme
-        // No class, 8 bit data
+        UserData uData = new UserData();
+        uData.userDataHeader = smsHeader;
         uData.msgEncoding = UserData.ENCODING_OCTET;
+        uData.msgEncodingSet = true;
         uData.payload = data;
 
-        byte[] msgData = sms.getEnvelope(destinationAddress, statusReportRequested, uData,
-                true, true);
-
-        ret.encodedMessage = msgData;
-        ret.encodedScAddress = null;
-        return ret;
+        return privateGetSubmitPdu(destAddr, statusReportRequested, uData, true);
     }
 
     static class PduParser {
@@ -635,6 +591,11 @@ public class SmsMessage extends SmsMessageBase {
     }
 
     /**
+     * TODO(cleanup): why are there two nearly identical functions
+     * below?  More rubbish...
+     */
+
+    /**
      * Parses a SMS-DELIVER message. (mobile-terminated only)
      * See 3GPP2 C.S0015-B, v2, 4.4.1
      */
@@ -677,7 +638,6 @@ public class SmsMessage extends SmsMessageBase {
         }
 
         parseUserData(mBearerData.userData);
-
     }
 
     /**
@@ -711,84 +671,28 @@ public class SmsMessage extends SmsMessageBase {
         }
     }
 
-    /**
-     * Creates BearerData and Envelope from parameters for a Submit SMS.
-     * @return byte stream for SubmitPdu.
-     */
-    private byte[] getEnvelope(String destinationAddress, boolean statusReportRequested,
-            UserData userData, boolean hasHeaders, boolean useNewId) {
-
-        BearerData mBearerData = new BearerData();
-        SmsEnvelope env = new SmsEnvelope();
-        CdmaSmsAddress mSmsAddress = new CdmaSmsAddress();
-
-        // ** set SmsAddress **
-        mSmsAddress.digitMode = CdmaSmsAddress.DIGIT_MODE_8BIT_CHAR;
-        try {
-            mSmsAddress.origBytes = destinationAddress.getBytes("UTF-8");
-        } catch (Exception e) {
-            Log.e(LOG_TAG, "doGetSubmitPdu: conversion of destinationAddress from string to byte[]"
-                    + " failed: " + e.getMessage());
-            return null;
-        }
-        mSmsAddress.numberOfDigits = (byte)mSmsAddress.origBytes.length;
-        mSmsAddress.numberMode = CdmaSmsAddress.NUMBER_MODE_NOT_DATA_NETWORK;
+    private static CdmaSmsAddress parseCdmaSmsAddr(String addrStr) {
         // see C.S0015-B, v2.0, 3.4.3.3
-        mSmsAddress.numberPlan = CdmaSmsAddress.NUMBERING_PLAN_ISDN_TELEPHONY;
-        mSmsAddress.ton = CdmaSmsAddress.TON_INTERNATIONAL_OR_IP;
-
-        // ** set BearerData **
-        mBearerData.userData = userData;
-        mBearerData.messageType = BearerData.MESSAGE_TYPE_SUBMIT;
-
-        if (useNewId) {
-            setNextMessageId();
-        }
-        mBearerData.messageId = nextMessageId;
-
-        // Set the reply options (See C.S0015-B, v2.0, 4.5.11)
-        if(statusReportRequested) {
-            mBearerData.deliveryAckReq = true;
-        } else {
-            mBearerData.deliveryAckReq = false;
-        }
-        // Currently settings applications do not support this
-        mBearerData.userAckReq = false;
-        mBearerData.readAckReq = false;
-        mBearerData.reportReq = false;
-
-        // number of messages: not needed for encoding!
-
-        // indicate whether a user data header is available
-        mBearerData.hasUserDataHeader = hasHeaders;
-
-        // ** encode BearerData **
-        byte[] encodedBearerData = null;
+        CdmaSmsAddress addr = new CdmaSmsAddress();
+        addr.digitMode = CdmaSmsAddress.DIGIT_MODE_8BIT_CHAR;
         try {
-            encodedBearerData = BearerData.encode(mBearerData);
-        } catch (Exception e) {
-            Log.e(LOG_TAG, "doGetSubmitPdu: EncodeCdmaSMS function in JNI interface failed: "
-                    + e.getMessage());
+            addr.origBytes = addrStr.getBytes("UTF-8");
+        } catch  (java.io.UnsupportedEncodingException ex) {
+            Log.e(LOG_TAG, "CDMA address parsing failed: " + ex);
             return null;
         }
-
-        // ** SmsEnvelope **
-        env.messageType = SmsEnvelope.MESSAGE_TYPE_POINT_TO_POINT;
-        env.teleService = SmsEnvelope.TELESERVICE_WMT;
-        env.destAddress = mSmsAddress;
-        env.bearerReply = RETURN_ACK;
-        env.bearerData = encodedBearerData;
-        mEnvelope = env;
-
-        // get byte array output stream from SmsAddress object and SmsEnvelope member.
-        return serialize(mSmsAddress);
+        addr.numberOfDigits = (byte)addr.origBytes.length;
+        addr.numberMode = CdmaSmsAddress.NUMBER_MODE_NOT_DATA_NETWORK;
+        addr.numberPlan = CdmaSmsAddress.NUMBERING_PLAN_ISDN_TELEPHONY;
+        addr.ton = CdmaSmsAddress.TON_INTERNATIONAL_OR_IP;
+        return addr;
     }
 
     /**
      * Set the nextMessageId to a random value between 0 and 65536
      * See C.S0015-B, v2.0, 4.3.1.5
      */
-    private void setNextMessageId() {
+    private static void setNextMessageId() {
         // Message ID, modulo 65536
         if(firstSMS) {
             Random generator = new Random();
@@ -800,38 +704,80 @@ public class SmsMessage extends SmsMessageBase {
     }
 
     /**
-     * Creates ByteArrayOutputStream from CdmaSmsAddress and SmsEnvelope objects
-     *
-     * @param address CdmaSmsAddress object
-     * @return ByteArrayOutputStream
+     * Creates BearerData and Envelope from parameters for a Submit SMS.
+     * @return byte stream for SubmitPdu.
      */
-    private byte[] serialize(CdmaSmsAddress destAddress) {
-        SmsEnvelope env = mEnvelope;
-        ByteArrayOutputStream baos = new ByteArrayOutputStream(100);
-        DataOutputStream dos = new DataOutputStream(new BufferedOutputStream(baos));
+    private static SubmitPdu privateGetSubmitPdu(String destAddrStr, boolean statusReportRequested,
+            UserData userData, boolean useNewId) {
+
+        /**
+         * TODO(cleanup): give this function a more meaningful name.
+         */
+
+        CdmaSmsAddress destAddr = parseCdmaSmsAddr(destAddrStr);
+        if (destAddr == null) return null;
+
+        BearerData bearerData = new BearerData();
+        bearerData.messageType = BearerData.MESSAGE_TYPE_SUBMIT;
+
+        if (useNewId) setNextMessageId();
+        bearerData.messageId = nextMessageId;
+
+        bearerData.deliveryAckReq = statusReportRequested;
+        bearerData.userAckReq = false;
+        bearerData.readAckReq = false;
+        bearerData.reportReq = false;
+
+        bearerData.userData = userData;
+        bearerData.hasUserDataHeader = (userData.userDataHeader != null);
+
+        byte[] encodedBearerData = BearerData.encode(bearerData);
+        if (encodedBearerData == null) return null;
+
+        SmsEnvelope envelope = new SmsEnvelope();
+        envelope.messageType = SmsEnvelope.MESSAGE_TYPE_POINT_TO_POINT;
+        envelope.teleService = SmsEnvelope.TELESERVICE_WMT;
+        envelope.destAddress = destAddr;
+        envelope.bearerReply = RETURN_ACK;
+        envelope.bearerData = encodedBearerData;
+
+        /**
+         * TODO(cleanup): envelope looks to be a pointless class, get
+         * rid of it.  Also -- most of the envelope fields set here
+         * are ignored, why?
+         */
 
         try {
-            dos.writeInt(env.teleService);
+            /**
+             * TODO(cleanup): reference a spec and get rid of the ugly comments
+             */
+            ByteArrayOutputStream baos = new ByteArrayOutputStream(100);
+            DataOutputStream dos = new DataOutputStream(baos);
+            dos.writeInt(envelope.teleService);
             dos.writeInt(0); //servicePresent
             dos.writeInt(0); //serviceCategory
-            dos.write(destAddress.digitMode);
-            dos.write(destAddress.numberMode);
-            dos.write(destAddress.ton); // number_type
-            dos.write(destAddress.numberPlan);
-            dos.write(destAddress.numberOfDigits);
-            dos.write(destAddress.origBytes, 0, destAddress.origBytes.length); // digits
+            dos.write(destAddr.digitMode);
+            dos.write(destAddr.numberMode);
+            dos.write(destAddr.ton); // number_type
+            dos.write(destAddr.numberPlan);
+            dos.write(destAddr.numberOfDigits);
+            dos.write(destAddr.origBytes, 0, destAddr.origBytes.length); // digits
             // Subaddress is not supported.
             dos.write(0); //subaddressType
             dos.write(0); //subaddr_odd
             dos.write(0); //subaddr_nbr_of_digits
-            dos.write(env.bearerData.length);
-            dos.write(env.bearerData, 0, env.bearerData.length);
+            dos.write(encodedBearerData.length);
+            dos.write(encodedBearerData, 0, encodedBearerData.length);
             dos.close();
-            return baos.toByteArray();
+
+            SubmitPdu pdu = new SubmitPdu();
+            pdu.encodedMessage = baos.toByteArray();
+            pdu.encodedScAddress = null;
+            return pdu;
         } catch(IOException ex) {
-            Log.e(LOG_TAG, "serialize: conversion from object to data output stream failed: " + ex);
-            return null;
+            Log.e(LOG_TAG, "creating SubmitPdu failed: " + ex);
         }
+        return null;
     }
 
     /**
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 b5952a1..e64d022 100644
--- a/telephony/java/com/android/internal/telephony/cdma/sms/BearerData.java
+++ b/telephony/java/com/android/internal/telephony/cdma/sms/BearerData.java
@@ -18,6 +18,8 @@ package com.android.internal.telephony.cdma.sms;
 
 import android.util.Log;
 
+import android.telephony.SmsMessage;
+
 import com.android.internal.telephony.GsmAlphabet;
 import com.android.internal.telephony.SmsHeader;
 import com.android.internal.telephony.cdma.sms.UserData;
@@ -26,10 +28,9 @@ import com.android.internal.util.HexDump;
 import com.android.internal.util.BitwiseInputStream;
 import com.android.internal.util.BitwiseOutputStream;
 
+
 /**
- * XXX
- *
- *
+ * An object to encode and decode CDMA SMS bearer data.
  */
 public final class BearerData{
     private final static String LOG_TAG = "SMS";
@@ -256,7 +257,7 @@ public final class BearerData{
         builder.append("  displayMode: " + (displayModeSet ? displayMode : "not set") + "\n");
         builder.append("  language: " + (languageIndicatorSet ? language : "not set") + "\n");
         builder.append("  errorClass: " + (messageStatusSet ? errorClass : "not set") + "\n");
-        builder.append("  messageStatus: " + (messageStatusSet ? messageStatus : "not set") + "\n");
+        builder.append("  msgStatus: " + (messageStatusSet ? messageStatus : "not set") + "\n");
         builder.append("  hasUserDataHeader: " + hasUserDataHeader + "\n");
         builder.append("  timeStamp: " + timeStamp + "\n");
         builder.append("  userAckReq: " + userAckReq + "\n");
@@ -280,9 +281,118 @@ public final class BearerData{
         outStream.skip(3);
     }
 
+    private static byte[] encode7bitAscii(String msg)
+        throws CodingException
+    {
+        try {
+            BitwiseOutputStream outStream = new BitwiseOutputStream(msg.length());
+            byte[] expandedData = msg.getBytes("US-ASCII");
+            for (int i = 0; i < expandedData.length; i++) {
+                int charCode = expandedData[i];
+                // Test ourselves for ASCII membership, since Java seems not to care.
+                if ((charCode < UserData.PRINTABLE_ASCII_MIN_INDEX) ||
+                        (charCode > UserData.PRINTABLE_ASCII_MAX_INDEX)) {
+                    throw new CodingException("illegal ASCII code (" + charCode + ")");
+                }
+                outStream.write(7, expandedData[i]);
+            }
+            return outStream.toByteArray();
+        } catch  (java.io.UnsupportedEncodingException ex) {
+            throw new CodingException("7bit ASCII encode failed: " + ex);
+        } catch  (BitwiseOutputStream.AccessException ex) {
+            throw new CodingException("7bit ASCII encode failed: " + ex);
+        }
+    }
+
+    private static byte[] encodeUtf16(String msg)
+        throws CodingException
+    {
+        try {
+            return msg.getBytes("utf-16be"); // XXX(do not submit) -- make sure decode matches
+        } catch (java.io.UnsupportedEncodingException ex) {
+            throw new CodingException("UTF-16 encode failed: " + ex);
+        }
+    }
+
+    private static byte[] encode7bitGsm(String msg)
+        throws CodingException
+    {
+        try {
+            /**
+             * TODO(cleanup): find some way to do this without the copy.
+             */
+            byte []fullData = GsmAlphabet.stringToGsm7BitPacked(msg);
+            byte []data = new byte[fullData.length - 1];
+            for (int i = 0; i < data.length; i++) {
+                data[i] = fullData[i + 1];
+            }
+            return data;
+        } catch (com.android.internal.telephony.EncodeException ex) {
+            throw new CodingException("7bit GSM encode failed: " + ex);
+        }
+    }
+
+    private static void encodeUserDataPayload(UserData uData)
+        throws CodingException
+    {
+        if (uData.msgEncodingSet) {
+            if (uData.msgEncoding == UserData.ENCODING_OCTET) {
+                if (uData.payload == null) {
+                    Log.e(LOG_TAG, "user data with octet encoding but null payload");
+                    // TODO(code_review): reasonable for fail case? or maybe bail on encoding?
+                    uData.payload = new byte[0];
+                }
+            } else {
+                if (uData.payloadStr == null) {
+                    Log.e(LOG_TAG, "non-octet user data with null payloadStr");
+                    // TODO(code_review): reasonable for fail case? or maybe bail on encoding?
+                    uData.payloadStr = "";
+                }
+                if (uData.msgEncoding == UserData.ENCODING_GSM_7BIT_ALPHABET) {
+                    uData.payload = encode7bitGsm(uData.payloadStr);
+                } else if (uData.msgEncoding == UserData.ENCODING_7BIT_ASCII) {
+                    uData.payload = encode7bitAscii(uData.payloadStr);
+                } else if (uData.msgEncoding == UserData.ENCODING_UNICODE_16) {
+                    uData.payload = encodeUtf16(uData.payloadStr);
+                } else {
+                    throw new CodingException("unsupported user data encoding (" +
+                                              uData.msgEncoding + ")");
+                }
+                uData.numFields = uData.payloadStr.length();
+            }
+        } else {
+            if (uData.payloadStr == null) {
+                Log.e(LOG_TAG, "user data with null payloadStr");
+                // TODO(code_review): reasonable for fail case? or maybe bail on encoding?
+                uData.payloadStr = "";
+            }
+            try {
+                uData.payload = encode7bitAscii(uData.payloadStr);
+                uData.msgEncoding = UserData.ENCODING_7BIT_ASCII;
+            } catch (CodingException ex) {
+                uData.payload = encodeUtf16(uData.payloadStr);
+                uData.msgEncoding = UserData.ENCODING_UNICODE_16;
+            }
+            uData.msgEncodingSet = true;
+            uData.numFields = uData.payloadStr.length();
+        }
+        if (uData.payload.length > SmsMessage.MAX_USER_DATA_BYTES) {
+            throw new CodingException("encoded user data too large (" + uData.payload.length +
+                                      " > " + SmsMessage.MAX_USER_DATA_BYTES + " bytes)");
+        }
+    }
+
     private static void encodeUserData(BearerData bData, BitwiseOutputStream outStream)
-        throws BitwiseOutputStream.AccessException
+        throws BitwiseOutputStream.AccessException, CodingException
     {
+        encodeUserDataPayload(bData.userData);
+        /**
+         * XXX/TODO: figure out what the right answer is WRT padding bits
+         *
+         *   userData.paddingBits = (userData.payload.length * 8) - (userData.numFields * 7);
+         *   userData.paddingBits = 0; // XXX this seems better, but why?
+         *
+         */
         int dataBits = (bData.userData.payload.length * 8) - bData.userData.paddingBits;
         byte[] headerData = null;
         if (bData.hasUserDataHeader) {
@@ -523,6 +633,7 @@ public final class BearerData{
         byte paramBytes = inStream.read(8);
         bData.userData = new UserData();
         bData.userData.msgEncoding = inStream.read(5);
+        bData.userData.msgEncodingSet = true;
         bData.userData.msgType = 0;
         int consumedBits = 5;
         if ((bData.userData.msgEncoding == UserData.ENCODING_IS91_EXTENDED_PROTOCOL) ||
@@ -536,26 +647,29 @@ public final class BearerData{
         bData.userData.payload = inStream.readByteArray(dataBits);
     }
 
-    private static String decodePayloadStr(byte[] data, int offset, int numFields, String format)
+    private static String decodeUtf16(byte[] data, int offset, int numFields)
         throws CodingException
     {
         try {
-            return new String(data, offset, numFields, format);
+            return new String(data, offset, numFields * 2, "utf-16be");
         } catch (java.io.UnsupportedEncodingException ex) {
-            throw new CodingException("invalid ASCII user data code");
+            throw new CodingException("UTF-16 decode failed: " + ex);
         }
     }
 
-    private static String decodeIa5(byte[] data, int offset, int numFields) {
+    private static String decodeIa5(byte[] data, int offset, int numFields)
+        throws CodingException
+    {
         try {
+            offset *= 8;
             StringBuffer strBuf = new StringBuffer(numFields);
             BitwiseInputStream inStream = new BitwiseInputStream(data);
-            inStream.skip(offset);
-            int wantedBits = numFields * 7;
+            int wantedBits = (offset * 8) + (numFields * 7);
             if (inStream.available() < wantedBits) {
                 throw new CodingException("insufficient data (wanted " + wantedBits +
                                           " bits, but only have " + inStream.available() + ")");
             }
+            inStream.skip(offset);
             for (int i = 0; i < numFields; i++) {
                 int charCode = inStream.read(7);
                 if ((charCode < UserData.IA5_MAP_BASE_INDEX) ||
@@ -566,11 +680,42 @@ public final class BearerData{
             }
             return strBuf.toString();
         } catch (BitwiseInputStream.AccessException ex) {
-            Log.e(LOG_TAG, "UserData AI5 decode failed: " + ex);
-        } catch (CodingException ex) {
-            Log.e(LOG_TAG, "UserData AI5 decode failed: " + ex);
+            throw new CodingException("AI5 decode failed: " + ex);
         }
-        return null;
+    }
+
+    private static String decode7bitAscii(byte[] data, int offset, int numFields)
+        throws CodingException
+    {
+        try {
+            offset *= 8;
+            BitwiseInputStream inStream = new BitwiseInputStream(data);
+            int wantedBits = offset + (numFields * 7);
+            if (inStream.available() < wantedBits) {
+                throw new CodingException("insufficient data (wanted " + wantedBits +
+                                          " bits, but only have " + inStream.available() + ")");
+            }
+            inStream.skip(offset);
+            byte[] expandedData = new byte[numFields];
+            for (int i = 0; i < numFields; i++) {
+                expandedData[i] = inStream.read(7);
+            }
+            return new String(expandedData, 0, numFields, "US-ASCII");
+        } catch (java.io.UnsupportedEncodingException ex) {
+            throw new CodingException("7bit ASCII decode failed: " + ex);
+        } catch (BitwiseInputStream.AccessException ex) {
+            throw new CodingException("7bit ASCII decode failed: " + ex);
+        }
+    }
+
+    private static String decode7bitGsm(byte[] data, int offset, int numFields)
+        throws CodingException
+    {
+        String result = GsmAlphabet.gsm7BitPackedToString(data, offset, numFields);
+        if (result == null) {
+            throw new CodingException("7bit GSM decoding failed");
+        }
+        return result;
     }
 
     private static void decodeUserDataPayload(UserData userData, boolean hasUserDataHeader)
@@ -578,28 +723,26 @@ public final class BearerData{
     {
         int offset = 0;
         if (hasUserDataHeader) {
-            int UdhLen = userData.payload[0];
-            byte[] headerData = new byte[UdhLen];
-            System.arraycopy(userData.payload, 1, headerData, 0, UdhLen);
+            int udhLen = userData.payload[0];
+            offset += udhLen;
+            byte[] headerData = new byte[udhLen];
+            System.arraycopy(userData.payload, 1, headerData, 0, udhLen);
             userData.userDataHeader = SmsHeader.parse(headerData);
         }
         switch (userData.msgEncoding) {
         case UserData.ENCODING_OCTET:
             break;
         case UserData.ENCODING_7BIT_ASCII:
-            userData.payloadStr = decodePayloadStr(userData.payload, offset,
-                                                   userData.numFields, "US-ASCII");
+            userData.payloadStr = decode7bitAscii(userData.payload, offset, userData.numFields);
             break;
         case UserData.ENCODING_IA5:
             userData.payloadStr = decodeIa5(userData.payload, offset, userData.numFields);
             break;
         case UserData.ENCODING_UNICODE_16:
-            userData.payloadStr = decodePayloadStr(userData.payload, offset,
-                                                   userData.numFields * 2, "UTF-16");
+            userData.payloadStr = decodeUtf16(userData.payload, offset, userData.numFields);
             break;
         case UserData.ENCODING_GSM_7BIT_ALPHABET:
-            userData.payloadStr = GsmAlphabet.gsm7BitPackedToString(userData.payload,
-                                                                    offset, userData.numFields);
+            userData.payloadStr = decode7bitGsm(userData.payload, offset, userData.numFields);
             break;
         default:
             throw new CodingException("unsupported user data encoding ("
diff --git a/telephony/java/com/android/internal/telephony/cdma/sms/UserData.java b/telephony/java/com/android/internal/telephony/cdma/sms/UserData.java
index f916089..02e94ad 100644
--- a/telephony/java/com/android/internal/telephony/cdma/sms/UserData.java
+++ b/telephony/java/com/android/internal/telephony/cdma/sms/UserData.java
@@ -19,7 +19,7 @@ package com.android.internal.telephony.cdma.sms;
 import com.android.internal.telephony.SmsHeader;
 import com.android.internal.util.HexDump;
 
-public class UserData{
+public class UserData {
 
     /**
      * User data encoding types.
@@ -50,6 +50,12 @@ public class UserData{
         'p', 'q', 'r', 's', 't', 'u', 'v', 'w', 'x', 'y', 'z', '{', '|', '}', '~'};
 
     /**
+     * Only elements between these indices in the ASCII table are printable.
+     */
+    public static final int PRINTABLE_ASCII_MIN_INDEX = 0x20;
+    public static final int PRINTABLE_ASCII_MAX_INDEX = 0x7F;
+
+    /**
      * Mapping for IA5 values less than 32 are flow control signals
      * and not used here.
      */
@@ -65,6 +71,7 @@ public class UserData{
      * Contains the data encoding type for the SMS message
      */
     public int msgEncoding;
+    public boolean msgEncodingSet = false;
 
     // XXX needed when encoding is IS91 or DCS (not supported yet):
     public int msgType;
@@ -87,7 +94,7 @@ public class UserData{
     public String toString() {
         StringBuilder builder = new StringBuilder();
         builder.append("UserData:\n");
-        builder.append("  msgEncoding: " + msgEncoding + "\n");
+        builder.append("  msgEncoding: " + (msgEncodingSet ? msgEncoding : "not set") + "\n");
         builder.append("  msgType: " + msgType + "\n");
         builder.append("  paddingBits: " + paddingBits + "\n");
         builder.append("  numFields: " + (int)numFields + "\n");
diff --git a/tests/AndroidTests/src/com/android/unit_tests/CdmaSmsTest.java b/tests/AndroidTests/src/com/android/unit_tests/CdmaSmsTest.java
index 9ff80c7..271a2cb 100644
--- a/tests/AndroidTests/src/com/android/unit_tests/CdmaSmsTest.java
+++ b/tests/AndroidTests/src/com/android/unit_tests/CdmaSmsTest.java
@@ -35,23 +35,6 @@ import android.util.Log;
 public class CdmaSmsTest extends AndroidTestCase {
     private final static String LOG_TAG = "Cdma_Sms_Test";
 
-    private static UserData makeUserData(String msg) {
-        UserData userData = new UserData();
-        byte[] payload;
-        try {
-            payload = GsmAlphabet.stringToGsm7BitPacked(msg);
-            userData.payload = new byte[payload.length - 1];
-            for (int i = 0; i < userData.payload.length; i++) userData.payload[i] = payload[i + 1];
-            userData.numFields = payload[0];
-            userData.paddingBits = (userData.payload.length * 8) - (userData.numFields * 7);
-            userData.paddingBits = 0; // XXX this is better, wtf?
-            userData.msgEncoding = UserData.ENCODING_GSM_7BIT_ALPHABET;
-        } catch (com.android.internal.telephony.EncodeException ex) {
-            assertEquals(1, 0);
-        }
-        return userData;
-    }
-
     @SmallTest
     public void testStandardSms() throws Exception {
         String pdu = "00031040900112488ea794e074d69e1b7392c270326cde9e98";
@@ -60,50 +43,82 @@ public class CdmaSmsTest extends AndroidTestCase {
     }
 
     @SmallTest
-    public void testStandardSmsFeedback() throws Exception {
+    public void testUserData7bitAscii() throws Exception {
+        String pdu = "0003100160010610262d5ab500";
+        BearerData bearerData = BearerData.decode(HexDump.hexStringToByteArray(pdu));
+        assertEquals("bjjj", bearerData.userData.payloadStr);
+    }
+
+    @SmallTest
+    public void testUserData7bitAsciiFeedback() throws Exception {
         BearerData bearerData = new BearerData();
         bearerData.messageType = BearerData.MESSAGE_TYPE_DELIVER;
         bearerData.messageId = 0;
         bearerData.hasUserDataHeader = false;
-        String payloadStr = "Test standard SMS";
-        bearerData.userData = makeUserData(payloadStr);
+        UserData userData = new UserData();
+        userData.payloadStr = "Test standard SMS";
+        userData.msgEncoding = UserData.ENCODING_7BIT_ASCII;
+        userData.msgEncodingSet = true;
+        bearerData.userData = userData;
+        byte []encodedSms = BearerData.encode(bearerData);
+        BearerData revBearerData = BearerData.decode(encodedSms);
+        assertEquals(BearerData.MESSAGE_TYPE_DELIVER, revBearerData.messageType);
+        assertEquals(0, revBearerData.messageId);
+        assertEquals(false, revBearerData.hasUserDataHeader);
+        assertEquals(userData.msgEncoding, revBearerData.userData.msgEncoding);
+        assertEquals(userData.payloadStr.length(), revBearerData.userData.numFields);
+        assertEquals(userData.payloadStr, revBearerData.userData.payloadStr);
+    }
+
+    @SmallTest
+    public void testUserData7bitGsmFeedback() throws Exception {
+        BearerData bearerData = new BearerData();
+        bearerData.messageType = BearerData.MESSAGE_TYPE_DELIVER;
+        bearerData.messageId = 0;
+        bearerData.hasUserDataHeader = false;
+        UserData userData = new UserData();
+        userData.payloadStr = "Test standard SMS";
+        userData.msgEncoding = UserData.ENCODING_GSM_7BIT_ALPHABET;
+        userData.msgEncodingSet = true;
+        bearerData.userData = userData;
         byte []encodedSms = BearerData.encode(bearerData);
         BearerData revBearerData = BearerData.decode(encodedSms);
         assertEquals(BearerData.MESSAGE_TYPE_DELIVER, revBearerData.messageType);
         assertEquals(0, revBearerData.messageId);
         assertEquals(false, revBearerData.hasUserDataHeader);
-        assertEquals(UserData.ENCODING_GSM_7BIT_ALPHABET, revBearerData.userData.msgEncoding);
-        assertEquals(payloadStr.length(), revBearerData.userData.numFields);
-        assertEquals(payloadStr, revBearerData.userData.payloadStr);
+        assertEquals(userData.msgEncoding, revBearerData.userData.msgEncoding);
+        assertEquals(userData.payloadStr.length(), revBearerData.userData.numFields);
+        assertEquals(userData.payloadStr, revBearerData.userData.payloadStr);
     }
 
     @SmallTest
-    public void testAltUserDataFeedback() throws Exception {
-        try {
-            BearerData bearerData = new BearerData();
-            bearerData.messageType = BearerData.MESSAGE_TYPE_DELIVER;
-            bearerData.messageId = 0;
-            bearerData.hasUserDataHeader = false;
-            UserData userData = new UserData();
-            String str1 = "test ascii user data encoding";
-            userData.payload = str1.getBytes("US-ASCII");
-            userData.numFields = str1.length();
-            userData.paddingBits = 0;
-            userData.msgEncoding = UserData.ENCODING_7BIT_ASCII;
-            bearerData.userData = userData;
-            byte []encodedSms = BearerData.encode(bearerData);
-            BearerData revBearerData = BearerData.decode(encodedSms);
-            assertEquals(str1, revBearerData.userData.payloadStr);
-            String str2 = "\u0160u\u1E5B\u0301r\u1ECFg\uD835\uDC1At\u00E9\u4E002\u3042";
-            userData.payload = str2.getBytes("UTF-16");
-            userData.numFields = str2.length() + 1;
-            userData.msgEncoding = UserData.ENCODING_UNICODE_16;
-            encodedSms = BearerData.encode(bearerData);
-            revBearerData = BearerData.decode(encodedSms);
-            assertEquals(str2, revBearerData.userData.payloadStr);
-        } catch (java.io.UnsupportedEncodingException ex) {
-            throw new RuntimeException("user data encoding error");
-        }
+    public void testUserDataUtf16Feedback() throws Exception {
+        BearerData bearerData = new BearerData();
+        bearerData.messageType = BearerData.MESSAGE_TYPE_DELIVER;
+        bearerData.messageId = 0;
+        bearerData.hasUserDataHeader = false;
+        UserData userData = new UserData();
+        userData.payloadStr = "\u0160u\u1E5B\u0301r\u1ECFg\uD835\uDC1At\u00E9\u4E002\u3042";
+        userData.msgEncoding = UserData.ENCODING_UNICODE_16;
+        userData.msgEncodingSet = true;
+        bearerData.userData = userData;
+        byte []encodedSms = BearerData.encode(bearerData);
+        BearerData revBearerData = BearerData.decode(encodedSms);
+        assertEquals(BearerData.MESSAGE_TYPE_DELIVER, revBearerData.messageType);
+        assertEquals(0, revBearerData.messageId);
+        assertEquals(false, revBearerData.hasUserDataHeader);
+        assertEquals(userData.msgEncoding, revBearerData.userData.msgEncoding);
+        assertEquals(userData.payloadStr.length(), revBearerData.userData.numFields);
+        assertEquals(userData.payloadStr, revBearerData.userData.payloadStr);
+        userData.msgEncoding = UserData.ENCODING_OCTET;
+        userData.msgEncodingSet = false;
+        revBearerData = BearerData.decode(BearerData.encode(bearerData));
+        assertEquals(BearerData.MESSAGE_TYPE_DELIVER, revBearerData.messageType);
+        assertEquals(0, revBearerData.messageId);
+        assertEquals(false, revBearerData.hasUserDataHeader);
+        assertEquals(userData.msgEncoding, revBearerData.userData.msgEncoding);
+        assertEquals(userData.payloadStr.length(), revBearerData.userData.numFields);
+        assertEquals(userData.payloadStr, revBearerData.userData.payloadStr);
     }
 
     @SmallTest
@@ -144,7 +159,9 @@ public class CdmaSmsTest extends AndroidTestCase {
         bearerData.messageType = BearerData.MESSAGE_TYPE_DELIVER;
         bearerData.messageId = 0;
         bearerData.hasUserDataHeader = false;
-        bearerData.userData = makeUserData("test reply option");
+        UserData userData = new UserData();
+        userData.payloadStr = "test reply option";
+        bearerData.userData = userData;
         bearerData.userAckReq = true;
         byte []encodedSms = BearerData.encode(bearerData);
         BearerData revBearerData = BearerData.decode(encodedSms);
@@ -196,7 +213,9 @@ public class CdmaSmsTest extends AndroidTestCase {
         bearerData.messageType = BearerData.MESSAGE_TYPE_DELIVER;
         bearerData.messageId = 0;
         bearerData.hasUserDataHeader = false;
-        bearerData.userData = makeUserData("test message count");
+        UserData userData = new UserData();
+        userData.payloadStr = "test message count";
+        bearerData.userData = userData;
         bearerData.numberOfMessages = 27;
         byte []encodedSms = BearerData.encode(bearerData);
         BearerData revBearerData = BearerData.decode(encodedSms);
@@ -221,7 +240,9 @@ public class CdmaSmsTest extends AndroidTestCase {
         bearerData.messageType = BearerData.MESSAGE_TYPE_DELIVER;
         bearerData.messageId = 0;
         bearerData.hasUserDataHeader = false;
-        bearerData.userData = makeUserData("test callback number");
+        UserData userData = new UserData();
+        userData.payloadStr = "test callback number";
+        bearerData.userData = userData;
         CdmaSmsAddress addr = new CdmaSmsAddress();
         addr.digitMode = CdmaSmsAddress.DIGIT_MODE_8BIT_CHAR;
         addr.ton = CdmaSmsAddress.TON_NATIONAL_OR_EMAIL;
@@ -256,7 +277,9 @@ public class CdmaSmsTest extends AndroidTestCase {
         bearerData.messageType = BearerData.MESSAGE_TYPE_DELIVER;
         bearerData.messageId = 0;
         bearerData.hasUserDataHeader = false;
-        bearerData.userData = makeUserData("test message center timestamp");
+        UserData userData = new UserData();
+        userData.payloadStr = "test message center timestamp";
+        bearerData.userData = userData;
         bearerData.timeStamp = HexDump.hexStringToByteArray("112233445566");
         byte []encodedSms = BearerData.encode(bearerData);
         BearerData revBearerData = BearerData.decode(encodedSms);
@@ -283,13 +306,14 @@ public class CdmaSmsTest extends AndroidTestCase {
         bearerData.messageType = BearerData.MESSAGE_TYPE_DELIVER;
         bearerData.messageId = 0;
         bearerData.hasUserDataHeader = false;
-        String payloadStr = "test privacy indicator";
-        bearerData.userData = makeUserData(payloadStr);
+        UserData userData = new UserData();
+        userData.payloadStr = "test privacy indicator";
+        bearerData.userData = userData;
         bearerData.privacy = BearerData.PRIVACY_SECRET;
         bearerData.privacyIndicatorSet = true;
         byte []encodedSms = BearerData.encode(bearerData);
         BearerData revBearerData = BearerData.decode(encodedSms);
-        assertEquals(revBearerData.userData.payloadStr, payloadStr);
+        assertEquals(revBearerData.userData.payloadStr, userData.payloadStr);
         assertEquals(revBearerData.privacyIndicatorSet, true);
         assertEquals(revBearerData.privacy, BearerData.PRIVACY_SECRET);
         bearerData.privacy = BearerData.PRIVACY_RESTRICTED;
@@ -324,19 +348,20 @@ public class CdmaSmsTest extends AndroidTestCase {
         bearerData.messageType = BearerData.MESSAGE_TYPE_DELIVER;
         bearerData.messageId = 0;
         bearerData.hasUserDataHeader = false;
-        String payloadStr = "test message delivery alert";
-        bearerData.userData = makeUserData(payloadStr);
+        UserData userData = new UserData();
+        userData.payloadStr = "test message delivery alert";
+        bearerData.userData = userData;
         bearerData.alert = BearerData.ALERT_MEDIUM_PRIO;
         bearerData.alertIndicatorSet = true;
         byte []encodedSms = BearerData.encode(bearerData);
         BearerData revBearerData = BearerData.decode(encodedSms);
-        assertEquals(revBearerData.userData.payloadStr, payloadStr);
+        assertEquals(revBearerData.userData.payloadStr, userData.payloadStr);
         assertEquals(revBearerData.alertIndicatorSet, true);
         assertEquals(revBearerData.alert, bearerData.alert);
         bearerData.alert = BearerData.ALERT_HIGH_PRIO;
         encodedSms = BearerData.encode(bearerData);
         revBearerData = BearerData.decode(encodedSms);
-        assertEquals(revBearerData.userData.payloadStr, payloadStr);
+        assertEquals(revBearerData.userData.payloadStr, userData.payloadStr);
         assertEquals(revBearerData.alertIndicatorSet, true);
         assertEquals(revBearerData.alert, bearerData.alert);
     }
@@ -359,19 +384,20 @@ public class CdmaSmsTest extends AndroidTestCase {
         bearerData.messageType = BearerData.MESSAGE_TYPE_DELIVER;
         bearerData.messageId = 0;
         bearerData.hasUserDataHeader = false;
-        String payloadStr = "test language indicator";
-        bearerData.userData = makeUserData(payloadStr);
+        UserData userData = new UserData();
+        userData.payloadStr = "test language indicator";
+        bearerData.userData = userData;
         bearerData.language = BearerData.LANGUAGE_ENGLISH;
         bearerData.languageIndicatorSet = true;
         byte []encodedSms = BearerData.encode(bearerData);
         BearerData revBearerData = BearerData.decode(encodedSms);
-        assertEquals(revBearerData.userData.payloadStr, payloadStr);
+        assertEquals(revBearerData.userData.payloadStr, userData.payloadStr);
         assertEquals(revBearerData.languageIndicatorSet, true);
         assertEquals(revBearerData.language, bearerData.language);
         bearerData.language = BearerData.LANGUAGE_KOREAN;
         encodedSms = BearerData.encode(bearerData);
         revBearerData = BearerData.decode(encodedSms);
-        assertEquals(revBearerData.userData.payloadStr, payloadStr);
+        assertEquals(revBearerData.userData.payloadStr, userData.payloadStr);
         assertEquals(revBearerData.languageIndicatorSet, true);
         assertEquals(revBearerData.language, bearerData.language);
     }
@@ -396,19 +422,20 @@ public class CdmaSmsTest extends AndroidTestCase {
         bearerData.messageType = BearerData.MESSAGE_TYPE_DELIVER;
         bearerData.messageId = 0;
         bearerData.hasUserDataHeader = false;
-        String payloadStr = "test display mode";
-        bearerData.userData = makeUserData(payloadStr);
+        UserData userData = new UserData();
+        userData.payloadStr = "test display mode";
+        bearerData.userData = userData;
         bearerData.displayMode = BearerData.DISPLAY_MODE_IMMEDIATE;
         bearerData.displayModeSet = true;
         byte []encodedSms = BearerData.encode(bearerData);
         BearerData revBearerData = BearerData.decode(encodedSms);
-        assertEquals(revBearerData.userData.payloadStr, payloadStr);
+        assertEquals(revBearerData.userData.payloadStr, userData.payloadStr);
         assertEquals(revBearerData.displayModeSet, true);
         assertEquals(revBearerData.displayMode, bearerData.displayMode);
         bearerData.displayMode = BearerData.DISPLAY_MODE_USER;
         encodedSms = BearerData.encode(bearerData);
         revBearerData = BearerData.decode(encodedSms);
-        assertEquals(revBearerData.userData.payloadStr, payloadStr);
+        assertEquals(revBearerData.userData.payloadStr, userData.payloadStr);
         assertEquals(revBearerData.displayModeSet, true);
         assertEquals(revBearerData.displayMode, bearerData.displayMode);
     }