GitHub PullRequestReviewCommentEvent at 2014-04-25 19:47:21
Repository: brackets-shell
Actor: RaymondLim
Payload: {"comment":{"url":"https://api.github.com/repos/adobe/brackets-shell/pulls/comments/12013076","id":12013076,"diff_hunk":"@@ -746,21 +870,87 @@ int32 ReadFile(ExtensionString filename, ExtensionString encoding, std::string&\n     if (INVALID_HANDLE_VALUE == hFile)\n         return ConvertWinErrorCode(GetLastError()); \n \n+    char* buffer = NULL;\n     DWORD dwFileSize = GetFileSize(hFile, NULL);\n     DWORD dwBytesRead;\n-    char* buffer = (char*)malloc(dwFileSize);\n-    if (buffer && ReadFile(hFile, buffer, dwFileSize, &dwBytesRead, NULL)) {\n-        contents = std::string(buffer, dwFileSize);\n+        \n+    // first just read a few bytes of the file\n+    //  to check for a binary or text format \n+    //  that we can't handle\n+\n+    // we just want to read enough to satisfy the\n+    //  UTF-16 or UTF-32 test with or without a BOM\n+    // the UTF-8 test could result in a false-positive\n+    //  but we'll check again with all bits if we \n+    //  think it's UTF-8 based on just a few characters\n+        \n+    // if we're going to read fewer bytes than our\n+    //  quick test then we skip the quick test and just\n+    //  do the full test below since it will be fewer reads\n+\n+    // We need a buffer that can handle UTF16 or UTF32 with or without a BOM \n+    //  but with enough that we can test for true UTF data to test against \n+    //  without reading partial character streams roughly 1000 characters \n+    //  at UTF32 should do it:\n+    // 1000 chars + 32-bit BOM (UTF-32) = 4004 bytes \n+    // 1001 chars without BOM  (UTF-32) = 4004 bytes \n+    // 2001 chars + 16 bit BOM (UTF-16) = 4004 bytes \n+    // 2002 chars without BOM  (UTF-16) = 4004 bytes \n+    const DWORD quickTestSize = 4004; \n+    static char quickTestBuffer[quickTestSize+1];\n+\n+    UTFValidationState validationState;\n+\n+    validationState.data = quickTestBuffer;\n+    validationState.dataLen = quickTestSize;\n+\n+    if (dwFileSize > quickTestSize) {\n+        ZeroMemory(quickTestBuffer, sizeof(quickTestBuffer));\n+        if (ReadFile(hFile, quickTestBuffer, quickTestSize, &dwBytesRead, NULL)) {\n+            if (!quickTestBufferForUTF8(validationState)) {\n+                error = ERR_UNSUPPORTED_ENCODING;\n+            }\n+            else {\n+                // reset the file pointer back to beginning\n+                //  since we're going to re-read the file wi\n+                SetFilePointer(hFile, 0, NULL, FILE_BEGIN);\n+            }\n+        }\n     }\n-    else {\n-        if (!buffer)\n-            error = ERR_UNKNOWN;\n-        else\n-            error = ConvertWinErrorCode(GetLastError());\n+\n+    if (error == NO_ERROR) {","path":"appshell/appshell_extensions_win.cpp","position":191,"original_position":191,"commit_id":"27163058cf39eb64de1b2eab13ae3a3ffc03c354","original_commit_id":"27163058cf39eb64de1b2eab13ae3a3ffc03c354","user":{"login":"RaymondLim","id":1542488,"avatar_url":"https://avatars.githubusercontent.com/u/1542488?","gravatar_id":"5e78f1f6398b643b4e5269d2ab5e24c6","url":"https://api.github.com/users/RaymondLim","html_url":"https://github.com/RaymondLim","followers_url":"https://api.github.com/users/RaymondLim/followers","following_url":"https://api.github.com/users/RaymondLim/following{/other_user}","gists_url":"https://api.github.com/users/RaymondLim/gists{/gist_id}","starred_url":"https://api.github.com/users/RaymondLim/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/RaymondLim/subscriptions","organizations_url":"https://api.github.com/users/RaymondLim/orgs","repos_url":"https://api.github.com/users/RaymondLim/repos","events_url":"https://api.github.com/users/RaymondLim/events{/privacy}","received_events_url":"https://api.github.com/users/RaymondLim/received_events","type":"User","site_admin":false},"body":"We should be checking `dwFileSize > 0` in this if condition so that we don't even try to allocate buffer and detecting encoding.","created_at":"2014-04-25T19:47:21Z","updated_at":"2014-04-25T19:47:21Z","html_url":"https://github.com/adobe/brackets-shell/pull/435#discussion_r12013076","pull_request_url":"https://api.github.com/repos/adobe/brackets-shell/pulls/435","_links":{"self":{"href":"https://api.github.com/repos/adobe/brackets-shell/pulls/comments/12013076"},"html":{"href":"https://github.com/adobe/brackets-shell/pull/435#discussion_r12013076"},"pull_request":{"href":"https://api.github.com/repos/adobe/brackets-shell/pulls/435"}}}}