Skip to content

Commit 0f51f64

Browse files
chkuang-gchkuang-g
committed
Replace the use of HttpUtility.UrlEncode()
- `HttpUtility.UrlEncode()` is causing NullReferneceException in certain version of Unity. Use `Uri.EscapeDataString()` instead. - Prevent any exception thrown during analytics reporting from blocking ther remaining process. - Add test-cases to TestPortableWebRequest Fixes #431 Bug: 183803624 Change-Id: I0a9db2d9df6318465dc3d0dbc542b84778129c9d
1 parent d973cf8 commit 0f51f64

File tree

4 files changed

+87
-75
lines changed

4 files changed

+87
-75
lines changed

‎source/VersionHandlerImpl/src/EditorMeasurement.cs‎

Lines changed: 54 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -493,53 +493,60 @@ public void Report(string reportUrl, string reportName) {
493493

494494
PromptToEnable(() => {
495495
if (!Enabled) return;
496-
497-
var uri = new Uri("http://ignore.host/" + reportUrl);
498-
bool reported = false;
499-
var path = String.Join("", uri.Segments);
500-
var queryPrefix =
501-
ConcatenateQueryStrings(
502-
ConcatenateQueryStrings(uri.Query,
503-
ConcatenateQueryStrings(CommonQuery, BaseQuery)), "scope=");
504-
var fragment = uri.Fragment;
505-
if (!String.IsNullOrEmpty(BasePath)) path = BasePath + path;
506-
if (!String.IsNullOrEmpty(BaseReportName)) reportName = BaseReportName + reportName;
507-
// Strip all extraneous path separators.
508-
while (path.Contains("//")) path = path.Replace("//", "/");
509-
foreach (var cookie in
510-
new KeyValuePair<string, string>[] {
511-
new KeyValuePair<string, string>(Cookie, queryPrefix + "project"),
512-
new KeyValuePair<string, string>(SystemCookie, queryPrefix + "system")
513-
}) {
514-
if (String.IsNullOrEmpty(cookie.Key)) continue;
515-
// See https://developers.google.com/analytics/devguides/collection/protocol/v1
516-
var status = PortableWebRequest.DefaultInstance.Post(
517-
"http://www.google-analytics.com/collect",
518-
new[] {
519-
// Version
520-
new KeyValuePair<string, string>("v", "1"),
521-
// Tracking ID.
522-
new KeyValuePair<string, string>("tid", trackingId),
523-
// Client ID.
524-
new KeyValuePair<string, string>("cid", cookie.Key),
525-
// Hit type.
526-
new KeyValuePair<string, string>("t", "pageview"),
527-
// "URL" / string to report.
528-
new KeyValuePair<string, string>(
529-
"dl", path + "?" + cookie.Value + fragment),
530-
// Document title.
531-
new KeyValuePair<string, string>("dt", reportName),
532-
// Cache buster
533-
new KeyValuePair<string, string>("z", random.Next().ToString())
534-
},
535-
null, null);
536-
if (status != null) reported = true;
537-
}
538-
if (reported) {
539-
logger.Log(String.Format("Reporting analytics data: {0}{1}{2} '{3}'", path,
540-
String.IsNullOrEmpty(queryPrefix) ? "" : "?" + queryPrefix,
541-
fragment, reportName),
542-
level: LogLevel.Verbose);
496+
try {
497+
var uri = new Uri("http://ignore.host/" + reportUrl);
498+
bool reported = false;
499+
var path = String.Join("", uri.Segments);
500+
var queryPrefix =
501+
ConcatenateQueryStrings(
502+
ConcatenateQueryStrings(uri.Query,
503+
ConcatenateQueryStrings(CommonQuery, BaseQuery)), "scope=");
504+
var fragment = uri.Fragment;
505+
if (!String.IsNullOrEmpty(BasePath)) path = BasePath + path;
506+
if (!String.IsNullOrEmpty(BaseReportName)) reportName = BaseReportName + reportName;
507+
// Strip all extraneous path separators.
508+
while (path.Contains("//")) path = path.Replace("//", "/");
509+
foreach (var cookie in
510+
new KeyValuePair<string, string>[] {
511+
new KeyValuePair<string, string>(Cookie, queryPrefix + "project"),
512+
new KeyValuePair<string, string>(SystemCookie, queryPrefix + "system")
513+
}) {
514+
if (String.IsNullOrEmpty(cookie.Key)) continue;
515+
// See https://developers.google.com/analytics/devguides/collection/protocol/v1
516+
var status = PortableWebRequest.DefaultInstance.Post(
517+
"http://www.google-analytics.com/collect",
518+
new[] {
519+
// Version
520+
new KeyValuePair<string, string>("v", "1"),
521+
// Tracking ID.
522+
new KeyValuePair<string, string>("tid", trackingId),
523+
// Client ID.
524+
new KeyValuePair<string, string>("cid", cookie.Key),
525+
// Hit type.
526+
new KeyValuePair<string, string>("t", "pageview"),
527+
// "URL" / string to report.
528+
new KeyValuePair<string, string>(
529+
"dl", path + "?" + cookie.Value + fragment),
530+
// Document title.
531+
new KeyValuePair<string, string>("dt", reportName),
532+
// Cache buster
533+
new KeyValuePair<string, string>("z", random.Next().ToString())
534+
},
535+
null, null);
536+
if (status != null) reported = true;
537+
}
538+
if (reported) {
539+
logger.Log(String.Format("Reporting analytics data: {0}{1}{2} '{3}'", path,
540+
String.IsNullOrEmpty(queryPrefix) ? "" : "?" + queryPrefix,
541+
fragment, reportName),
542+
level: LogLevel.Verbose);
543+
}
544+
} catch (Exception e) {
545+
// Make sure no exception thrown during analytics reporting will be raised to
546+
// the main thread and interupt the process.
547+
logger.Log(String.Format(
548+
"Failed to reporting analytics data due to exception: {0}", e),
549+
level: LogLevel.Verbose);
543550
}
544551
});
545552
}

‎source/VersionHandlerImpl/src/PortableWebRequest.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -552,8 +552,8 @@ public IPortableWebRequestStatus Post(string path,
552552
foreach (var param in queryParams) {
553553
url.AppendFormat("{0}{1}={2}",
554554
url.Length == 0 ? "?" : "&",
555-
HttpUtility.UrlEncode(param.Key),
556-
HttpUtility.UrlEncode(param.Value));
555+
Uri.EscapeDataString(param.Key).Trim(),
556+
Uri.EscapeDataString(param.Value).Trim());
557557
}
558558
url.Insert(0, path);
559559
return Post(url.ToString(), headers, formFields);

‎source/VersionHandlerImpl/test/webrequest/Assets/PlayServicesResolver/Editor/TestPortableWebRequest.cs_DISABLED‎

Lines changed: 25 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -162,15 +162,16 @@ public class TestPortableWebRequest {
162162
}, synchronous: true);
163163

164164
UnityEngine.Debug.Log("Running post test...");
165-
var postStatus = webRequest.Post("http://localhost:8000/post",
166-
new Dictionary<string, string> {
167-
{ "Echo-Foo", "Bar" },
168-
{ "Echo-Bish", "Bosh" }
169-
},
170-
new[] {
171-
new KeyValuePair<string, string>("foo1", "bar1"),
172-
new KeyValuePair<string, string>("foo2", "bar2")
173-
});
165+
var postStatus = webRequest.Post(
166+
"http://localhost:8000/post?queryfoo1=querybar1&queryfoo2=querybar2",
167+
headers: new Dictionary<string, string> {
168+
{ "Echo-Foo", "Bar" },
169+
{ "Echo-Bish", "Bosh" }
170+
},
171+
formFields: new[] {
172+
new KeyValuePair<string, string>("foo1", "bar1"),
173+
new KeyValuePair<string, string>("foo2", "bar2")
174+
});
174175
RunOnMainThread.PollOnUpdateUntilComplete(() => {
175176
var complete = postStatus.Complete;
176177
if (complete) {
@@ -184,7 +185,9 @@ public class TestPortableWebRequest {
184185
"{\"data\": \"Hello from a test server\", " +
185186
"\"form\": {\"foo1\": [\"bar1\"], \"foo2\": [\"bar2\"]}, " +
186187
"\"headers\": {\"Echo-Bish\": \"Bosh\", \"Echo-Foo\": \"Bar\"}, " +
187-
"\"path\": \"/post\", \"query\": {}}";
188+
"\"path\": \"/post?queryfoo1=querybar1&queryfoo2=querybar2\", " +
189+
"\"query\": {\"queryfoo1\": [\"querybar1\"], " +
190+
"\"queryfoo2\": [\"querybar2\"]}}";
188191
postSucceeded &= CheckEqual(result, expected);
189192
UnityEngine.Debug.Log(String.Format("Post complete succeeded={0}\n{1}",
190193
postSucceeded, result));
@@ -194,16 +197,18 @@ public class TestPortableWebRequest {
194197

195198
UnityEngine.Debug.Log("Running post test with no form fields...");
196199
var postNoFormFieldsStatus = webRequest.Post(
197-
"http://localhost:8000/post?foo1=bar1&foo2=bar2",
198-
new[] {
200+
"http://localhost:8000/post",
201+
queryParams: new[] {
199202
new KeyValuePair<string, string>("foo1", "bar1"),
200-
new KeyValuePair<string, string>("foo2", "bar2")
203+
new KeyValuePair<string, string>("foo2", "bar2"),
204+
new KeyValuePair<string, string>("foo with/special+char",
205+
"bar with/special+char")
201206
},
202-
new Dictionary<string, string> {
207+
headers: new Dictionary<string, string> {
203208
{ "Echo-Foo", "Bar" },
204209
{ "Echo-Bish", "Bosh" }
205210
},
206-
null);
211+
formFields: null);
207212
RunOnMainThread.PollOnUpdateUntilComplete(() => {
208213
var complete = postNoFormFieldsStatus.Complete;
209214
if (complete) {
@@ -217,10 +222,10 @@ public class TestPortableWebRequest {
217222
postNoFormFieldsStatus.Result);
218223
var expected =
219224
"{\"data\": \"Hello from a test server\", " +
220-
"\"form\": {}, " +
221225
"\"headers\": {\"Echo-Bish\": \"Bosh\", \"Echo-Foo\": \"Bar\"}, " +
222-
"\"path\": \"/post\", " +
223-
"\"query\": {\"foo1\": [\"bar1\"], \"foo2\": [\"bar2\"]}";
226+
"\"path\": \"/post?foo1=bar1&foo2=bar2&foo%20with%2Fspecial%2Bchar=bar%20with%2Fspecial%2Bchar\", " +
227+
"\"query\": {\"foo with/special+char\": [\"bar with/special+char\"], " +
228+
"\"foo1\": [\"bar1\"], \"foo2\": [\"bar2\"]}}";
224229
postNoFormFieldsSucceeded &= CheckEqual(result, expected);
225230
UnityEngine.Debug.Log(String.Format(
226231
"Post with no firm fields succeeded={0}\n{1}",
@@ -232,8 +237,8 @@ public class TestPortableWebRequest {
232237
UnityEngine.Debug.Log("Running post test with no headers...");
233238
var postNoHeadersStatus = webRequest.Post(
234239
"http://localhost:8000/post/with/no/headers",
235-
null,
236-
new[] {
240+
headers: null,
241+
formFields: new[] {
237242
new KeyValuePair<string, string>("foo1", "bar1"),
238243
new KeyValuePair<string, string>("foo2", "bar2")
239244
});

‎source/VersionHandlerImpl/unit_tests/src/EditorMeasurementTest.cs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -115,8 +115,8 @@ public IPortableWebRequestStatus Post(
115115

116116
url.AppendFormat("{0}{1}={2}",
117117
url.Length == 0 ? "?" : "&",
118-
HttpUtility.UrlEncode(param.Key),
119-
HttpUtility.UrlEncode(value));
118+
Uri.EscapeDataString(param.Key).Trim(),
119+
Uri.EscapeDataString(value).Trim());
120120
}
121121
url.Insert(0, path);
122122
return Post(url.ToString(), headers, formFields);
@@ -374,10 +374,10 @@ private KeyValuePair<string, string> CreateMeasurementEvent(
374374
"&dl={2}" +
375375
"&dt={3}" +
376376
"&z=0",
377-
HttpUtility.UrlEncode(GA_TRACKING_ID),
378-
HttpUtility.UrlEncode(cookie),
379-
HttpUtility.UrlEncode(reportUrl),
380-
HttpUtility.UrlEncode(reportName)),
377+
Uri.EscapeDataString(GA_TRACKING_ID).Trim(),
378+
Uri.EscapeDataString(cookie).Trim(),
379+
Uri.EscapeDataString(reportUrl).Trim(),
380+
Uri.EscapeDataString(reportName).Trim()),
381381
"" /* empty form fields */);
382382
}
383383

0 commit comments

Comments
 (0)