Tidy up app::response().
authorTom Sepez <tsepez@chromium.org>
Tue, 29 Jul 2014 21:01:21 +0000 (14:01 -0700)
committerTom Sepez <tsepez@chromium.org>
Tue, 29 Jul 2014 21:01:21 +0000 (14:01 -0700)
Follow-up from https://codereview.chromium.org/424883002/

- Remove some stray whitespace.
- Fix "else after return".
- Remove unused swResponse local.
- Treat unexpectedly large responses as errors.

BUG=
R=jun_fang@foxitsoftware.com

Review URL: https://codereview.chromium.org/423953002

fpdfsdk/include/fpdfformfill.h
fpdfsdk/src/javascript/app.cpp

index 50a2dff..b69a8f6 100644 (file)
@@ -72,11 +72,10 @@ typedef struct _IPDF_JsPlatform
        *                       None
        */
        void (*app_beep)(struct _IPDF_JsPlatform* pThis,  int nType);
-       
-       
+
        /** 
        * Method: app_response
-       *                       Displays a dialog box containing a question and an entry field for the user to reply to the question.  
+       *                       Displays a dialog box containing a question and an entry field for the user to reply to the question.
        * Interface Version:
        *                       1
        * Implementation Required:
@@ -86,23 +85,20 @@ typedef struct _IPDF_JsPlatform
        *                       Question        -       The question to be posed to the user.
        *                       Title           -       The title of the dialog box.
        *                       Default         -       A default value for the answer to the question. If not specified, no default value is presented.
-       *                       cLabel          -       A short string to appear in front of and on the same line as the edit text field. 
-       *                       bPassword       -       If true, indicates that the user's response should show as asterisks (*) or bullets (?) to mask the response, which might be sensitive information. The default is false.               
-       *                       response        -       A string buffer allocated by SDK, to receive the user's response. 
+       *                       cLabel          -       A short string to appear in front of and on the same line as the edit text field.
+       *                       bPassword       -       If true, indicates that the user's response should show as asterisks (*) or bullets (?) to mask the response, which might be sensitive information. The default is false.
+       *                       response        -       A string buffer allocated by SDK, to receive the user's response.
        *                       length          -   The length of the buffer, number of bytes. Currently, It's always be 2048.
        * Return Value:
-       *                       Number of bytes the user input text consumes, not including trailing zeros. If the text exceed 2048 bytes,
-       *                       the exceeded part will be ignored.
+       *               Number of bytes the complete user input would actually require, not including trailing zeros, regardless of the value of the length
+       *               parameter or the presence of the response buffer.
        * Comments:
-       *                       No matter on what platform, the response should be always input in UTF-16LE encoding.
-       *                       The return value always indicated number of bytes required for the buffer, even when there is
-       *                       no buffer specified, or the buffer size is less then required. In this case, the buffer will not
-       *                       be modified.
+       *               No matter on what platform, the response buffer should be always written using UTF-16LE encoding. If a response buffer is
+       *               present and the size of the user input exceeds the capacity of the buffer as specified by the length parameter, only the
+       *               first "length" bytes of the user input are to be written to the buffer.
        */
        int (*app_response)(struct _IPDF_JsPlatform* pThis, FPDF_WIDESTRING Question, FPDF_WIDESTRING Title, FPDF_WIDESTRING Default, FPDF_WIDESTRING cLabel, FPDF_BOOL bPassword, void* response, int length);
-       
-       
-       
+
        /*
        * Method: Doc_getFilePath
        *                       Get the file path of the current document. 
index 3b92a99..0546be8 100644 (file)
@@ -153,8 +153,8 @@ FX_BOOL app::activeDocs(OBJ_PROP_PARAMS)
                CJS_Array aDocs(pRuntime->GetIsolate());
 //             int iNumDocs = pApp->CountDocuments();
                
-//             for(int iIndex = 0; iIndex<iNumDocs; iIndex++)
-//             {
+//             for(int iIndex = 0; iIndex<iNumDocs; iIndex++)
+//             {
                        CPDFSDK_Document* pDoc = pApp->GetCurrentDoc();
                        if (pDoc)
                        {
@@ -210,10 +210,10 @@ FX_BOOL app::calculate(OBJ_PROP_PARAMS)
                ASSERT(pRuntime != NULL);
 
                CJS_Array aDocs(pRuntime->GetIsolate());
-//             int iNumDocs = pApp->CountDocuments();
-//             
-//             for (int iIndex = 0;iIndex < iNumDocs; iIndex++)
-//             {
+//             int iNumDocs = pApp->CountDocuments();
+//             
+//             for (int iIndex = 0;iIndex < iNumDocs; iIndex++)
+//             {
                        if (CPDFSDK_Document* pDoc = pApp->GetCurrentDoc())
                        {
                                CPDFSDK_InterForm* pInterForm = (CPDFSDK_InterForm*)pDoc->GetInterForm();
@@ -251,10 +251,10 @@ FX_BOOL app::viewerType(OBJ_PROP_PARAMS)
 
        if (vp.IsGetting())
        {
-//             if (pApp->GetAppName() == PHANTOM)
-//                     vp << JS_STR_VIEWERTYPE_STANDARD;
-//             else
-//                     vp << JS_STR_VIEWERTYPE_READER;
+//             if (pApp->GetAppName() == PHANTOM)
+//                     vp << JS_STR_VIEWERTYPE_STANDARD;
+//             else
+//                     vp << JS_STR_VIEWERTYPE_READER;
                vp << L"unknown";
 
                //vp << pApp->GetAppTitle();
@@ -367,8 +367,8 @@ FX_BOOL app::alert(OBJ_METHOD_PARAMS)
                                        CJS_Value* pValue = new CJS_Value(isolate);
 //                                     if (iLenth == 1)
 //                                             pValue = new CJS_Value(isolate);
-//                                     else if (iLenth > 1)
-//                                             pValue = new CJS_Value[iLenth];
+//                                     else if (iLenth > 1)
+//                                             pValue = new CJS_Value[iLenth];
 
                                        for(int i = 0; i < iLenth; i++)
                                        {
@@ -379,16 +379,16 @@ FX_BOOL app::alert(OBJ_METHOD_PARAMS)
                                        }
 
                                        if(pValue) delete pValue;
-//                                     if ((iLenth > 1) && pValue)
-//                                     {
-//                                             delete[]pValue;
-//                                             pValue = NULL;
-//                                     }
-//                                     else if ((iLenth == 1) && pValue)
-//                                     {
-//                                             delete pValue;
-//                                             pValue = NULL;
-//                                     }
+//                                     if ((iLenth > 1) && pValue)
+//                                     {
+//                                             delete[]pValue;
+//                                             pValue = NULL;
+//                                     }
+//                                     else if ((iLenth == 1) && pValue)
+//                                     {
+//                                             delete pValue;
+//                                             pValue = NULL;
+//                                     }
                                }
                        }
 
@@ -947,7 +947,7 @@ FX_BOOL app::browseForDoc(OBJ_METHOD_PARAMS)
                JS_PutObjectString(isolate,pRetObj, L"cFS", CFX_WideString::FromLocal("DOS"));
        }
        
-       vRet =  pRetObj;
+       vRet =  pRetObj;
 
        return TRUE;
 }
@@ -1037,15 +1037,13 @@ FX_BOOL app::response(OBJ_METHOD_PARAMS)
        CFX_WideString swTitle = L"PDF";
 #endif
        CFX_WideString swDefault = L"";
-       CFX_WideString swResponse = L"";
        bool bPassWord = false;
-       
+
        v8::Isolate* isolate = GetIsolate(cc);
-       
-       int iLength = params.size();    
+
+       int iLength = params.size();
        if (iLength > 0 && params[0].GetType() == VT_object)
        {
-               
                JSObject pObj = (JSObject )params[0];
                v8::Handle<v8::Value> pValue = JS_GetObjectElement(isolate,pObj,L"cQuestion");
                        swQuestion = CJS_Value(isolate,pValue,GET_VALUE_TYPE(pValue)).operator CFX_WideString();
@@ -1101,22 +1099,25 @@ FX_BOOL app::response(OBJ_METHOD_PARAMS)
 
        CPDFDoc_Environment* pApp = pContext->GetReaderApp();
        ASSERT(pApp != NULL);
-       int nLength = 2048;
-       char* pBuff = new char[nLength];
-       nLength = pApp->JS_appResponse(swQuestion, swTitle, swDefault, swLabel, bPassWord, pBuff, nLength);
-       if(nLength<=0)
+
+       const int MAX_INPUT_BYTES = 2048;
+       char* pBuff = new char[MAX_INPUT_BYTES + 2];
+       if (!pBuff)
+               return FALSE;
+
+       memset(pBuff, 0, MAX_INPUT_BYTES + 2);
+       int nLengthBytes = pApp->JS_appResponse(swQuestion, swTitle, swDefault, swLabel, bPassWord, pBuff, MAX_INPUT_BYTES);
+       if (nLengthBytes <= 0)
        {
-               delete[] pBuff;
                vRet.SetNull();
+               delete[] pBuff;
                return FALSE;
        }
-       else
-       {
-               nLength = nLength > sizeof(pBuff) ? sizeof(pBuff) : nLength;
-        vRet = swResponse = CFX_WideString::FromUTF16LE((unsigned short*)pBuff, nLength / 2);
-       }
-       delete[] pBuff;
+       if (nLengthBytes > MAX_INPUT_BYTES)
+               nLengthBytes = MAX_INPUT_BYTES;
 
+       vRet = CFX_WideString::FromUTF16LE((unsigned short*)pBuff, nLengthBytes);
+       delete[] pBuff;
        return TRUE;
 }