Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Delta Between Two Patch Sets: src/plugin/PluginClientBase.cpp

Issue 6012307226230784: Issue #1234 - std::wstring version of UnescapeUrl (Closed)
Left Patch Set: Created Aug. 5, 2014, 5:51 p.m.
Right Patch Set: Created Oct. 1, 2014, 6:52 p.m.
Left:
Right:
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
Left: Side by side diff | Download
Right: Side by side diff | Download
« no previous file with change/comment | « src/plugin/PluginClientBase.h ('k') | no next file » | no next file with change/comment »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
LEFTRIGHT
1 #include "PluginStdAfx.h" 1 #include "PluginStdAfx.h"
2 2
3 // Internet / FTP 3 // Internet / FTP
4 #include <wininet.h> 4 #include <wininet.h>
5 5
6 // IP adapter 6 // IP adapter
7 #include <iphlpapi.h> 7 #include <iphlpapi.h>
8 8
9 #include "PluginSettings.h" 9 #include "PluginSettings.h"
10 #include "PluginSystem.h" 10 #include "PluginSystem.h"
(...skipping 22 matching lines...) Expand all
33 CPluginClientBase::CPluginClientBase() 33 CPluginClientBase::CPluginClientBase()
34 { 34 {
35 } 35 }
36 36
37 37
38 CPluginClientBase::~CPluginClientBase() 38 CPluginClientBase::~CPluginClientBase()
39 { 39 {
40 } 40 }
41 41
42 42
43 bool CPluginClientBase::IsValidDomain(const CString& domain)
44 {
45 return domain != L"about:blank" &&
46 domain != L"about:tabs" &&
47 domain.Find(L"javascript:") != 0 &&
48 !domain.IsEmpty();
49 }
50
51
52 CString& CPluginClientBase::UnescapeUrl(CString& url) 43 CString& CPluginClientBase::UnescapeUrl(CString& url)
53 { 44 {
54 CString unescapedUrl; 45 CString unescapedUrl;
55 DWORD cb = 2048; 46 DWORD cb = 2048;
56 47
57 if (SUCCEEDED(::UrlUnescape(url.GetBuffer(), unescapedUrl.GetBufferSetLength(c b), &cb, 0))) 48 if (SUCCEEDED(::UrlUnescape(url.GetBuffer(), unescapedUrl.GetBufferSetLength(c b), &cb, 0)))
58 { 49 {
59 unescapedUrl.ReleaseBuffer(); 50 unescapedUrl.ReleaseBuffer();
60 unescapedUrl.Truncate(cb); 51 unescapedUrl.Truncate(cb);
61 52
62 url.ReleaseBuffer(); 53 url.ReleaseBuffer();
63 url = unescapedUrl; 54 url = unescapedUrl;
64 } 55 }
65 56
66 return url; 57 return url;
67 } 58 }
68 59
69 void UnescapeUrl(std::wstring& url) 60 void UnescapeUrl(std::wstring& url)
70 { 61 {
71 /*
72 * When the code can tolerate exceptions better,
Felix Dahlke 2014/08/15 15:32:29 I think this comment should go into the catch bloc
Eric 2014/09/29 11:50:07 OK. Since I've written this code, I've come to th
73 * there's no reason to keep this try-catch statement in place.
74 */
75 try 62 try
76 { 63 {
77 DWORD result_length = INTERNET_MAX_URL_LENGTH; 64 DWORD result_length = INTERNET_MAX_URL_LENGTH;
78 /* 65 std::unique_ptr<wchar_t[]> result(new wchar_t[result_length]);
79 * The buffer length is greater than 2 Kb, so we keep it off the stack and a llocate it. 66 HRESULT hr = UrlUnescapeW(const_cast<wchar_t*>(url.c_str()), result.get(), & result_length, 0);
Eric 2014/09/29 11:50:07 Done.
80 */
81 std::unique_ptr<wchar_t[]> result(new wchar_t[result_length]); // can throw bad_alloc
82 /*
83 * Casting away const here is harmless because we're not using the in-place modification mode of UrlUnescape
Felix Dahlke 2014/08/15 15:32:29 This is pretty obvious from the documentation of U
Felix Dahlke 2014/09/30 09:10:28 We have the exact same problem in several cases -
Eric 2014/09/30 15:22:21 UrlUnescape is worse than the usual Windows API, b
Felix Dahlke 2014/09/30 16:08:16 Yeah, that's true. Still - at the end of the day t
sergei 2014/10/06 09:08:54 I would say that this comment could be here or may
84 */
85 HRESULT hr = UrlUnescapeW(const_cast<wchar_t *>(url.c_str()), result.get(), &result_length, 0);
Felix Dahlke 2014/08/15 15:32:29 No space between wchar_t and *.
Eric 2014/09/29 11:50:07 Done.
86 if (hr == S_OK) 67 if (hr == S_OK)
87 { 68 {
88 url = std::wstring(result.get(), result_length); 69 url = std::wstring(result.get(), result_length);
89 } 70 }
90 /* 71 /*
91 * If the call to UrlUnescape fails, we don't alter the string. 72 * Do nothing. This masks error return values from UrlUnescape without loggi ng the error.
92 * This matches the behavior of the previous version of this function. 73 */
Felix Dahlke 2014/08/15 15:32:29 How about changing "the previous version" to "the
Eric 2014/09/29 11:50:07 I'm moving this comment to the function header, si
Felix Dahlke 2014/09/30 09:10:28 Yeah, good point. I think the comment remaining he
Eric 2014/09/30 15:22:21 I'll fix the comment again. What's not obvious i
Felix Dahlke 2014/09/30 16:08:16 Fair enough.
93 * Because there's no error handling, however, this masks failures in UrlUne scape. 74 }
75 catch(std::bad_alloc e)
76 {
77 /*
78 * When the code has a systematic way of handling bad_alloc, we'll rethrow ( probably).
79 * Until then, we mask the exception and make no modification.
94 */ 80 */
95 } 81 }
96 catch(...) 82 catch(...)
97 { 83 {
98 // no modification if exception 84 // no modification if any other exception
99 } 85 }
100 } 86 }
101 87
102 void CPluginClientBase::LogPluginError(DWORD errorCode, int errorId, int errorSu bid, const CString& description, bool isAsync, DWORD dwProcessId, DWORD dwThread Id) 88 void CPluginClientBase::LogPluginError(DWORD errorCode, int errorId, int errorSu bid, const CString& description, bool isAsync, DWORD dwProcessId, DWORD dwThread Id)
103 { 89 {
104 // Prevent circular references 90 // Prevent circular references
105 if (CPluginSettings::HasInstance() && isAsync) 91 if (CPluginSettings::HasInstance() && isAsync)
106 { 92 {
107 DEBUG_ERROR_CODE_EX(errorCode, description, dwProcessId, dwThreadId); 93 DEBUG_ERROR_CODE_EX(errorCode, description, dwProcessId, dwThreadId);
108 94
(...skipping 41 matching lines...) Expand 10 before | Expand all | Expand 10 after
150 136
151 hasError = true; 137 hasError = true;
152 138
153 s_pluginErrors.erase(it); 139 s_pluginErrors.erase(it);
154 } 140 }
155 } 141 }
156 s_criticalSectionLocal.Unlock(); 142 s_criticalSectionLocal.Unlock();
157 143
158 return hasError; 144 return hasError;
159 } 145 }
LEFTRIGHT

Powered by Google App Engine
This is Rietveld