Skip to content

Commit 7eae0c3

Browse files
authored
Fix URL processing code (#1180)
1 parent 9c69a24 commit 7eae0c3

File tree

4 files changed

+196
-226
lines changed

4 files changed

+196
-226
lines changed

Apps/UnitTests/Scripts/tests.js

Lines changed: 38 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -31,62 +31,58 @@ const expect = chai.expect;
3131
describe("XMLHTTPRequest", function () {
3232
this.timeout(0);
3333
it("should have readyState=4 when load ends", async function () {
34-
const xhr = await createRequest("GET", "https://babylonjs.com");
34+
const xhr = await createRequest("GET", "https://httpbin.org/get");
3535
expect(xhr.readyState).to.equal(4);
36-
})
36+
});
3737
it("should have status=200 for a file that exists", async function () {
38-
const xhr = await createRequest("GET", "https://babylonjs.com");
39-
expect(xhr.status).to.equal(200);
40-
})
41-
it("should load unescaped URLs", async function () {
42-
const xhr = await createRequest("GET", "https://github.com/BabylonJS/Assets/raw/master/meshes/στρογγυλεμένος % κύβος.glb");
38+
const xhr = await createRequest("GET", "https://httpbin.org/status/200");
4339
expect(xhr.status).to.equal(200);
44-
})
45-
it("should load partially unescaped URLs", async function () {
46-
const xhr = await createRequest("GET", "https://github.com/BabylonJS/Assets/raw/master/meshes/στρογγυλεμένος%20%%20κύβος.glb");
40+
});
41+
it("should load URLs with escaped unicode characters", async function () {
42+
const xhr = await createRequest("GET", "https://raw.githubusercontent.com/BabylonJS/Assets/master/meshes/%CF%83%CF%84%CF%81%CE%BF%CE%B3%CE%B3%CF%85%CE%BB%CE%B5%CE%BC%CE%AD%CE%BD%CE%BF%CF%82%20%25%20%CE%BA%CF%8D%CE%B2%CE%BF%CF%82.glb");
4743
expect(xhr.status).to.equal(200);
48-
})
49-
it("should load escaped URLs", async function () {
50-
const xhr = await createRequest("GET", "https://github.com/BabylonJS/Assets/raw/master/meshes/%CF%83%CF%84%CF%81%CE%BF%CE%B3%CE%B3%CF%85%CE%BB%CE%B5%CE%BC%CE%AD%CE%BD%CE%BF%CF%82%20%25%20%CE%BA%CF%8D%CE%B2%CE%BF%CF%82.glb");
44+
});
45+
it("should load URLs with unescaped unicode characters", async function () {
46+
const xhr = await createRequest("GET", "https://raw.githubusercontent.com/BabylonJS/Assets/master/meshes/στρογγυλεμένος%20%25%20κύβος.glb");
5147
expect(xhr.status).to.equal(200);
52-
})
53-
it("should load URLs with unescaped %s", async function () {
54-
const xhr = await createRequest("GET", "https://github.com/BabylonJS/Assets/raw/master/meshes/%CF%83%CF%84%CF%81%CE%BF%CE%B3%CE%B3%CF%85%CE%BB%CE%B5%CE%BC%CE%AD%CE%BD%CE%BF%CF%82%20%%20%CE%BA%CF%8D%CE%B2%CE%BF%CF%82.glb");
48+
});
49+
it("should load URLs with unescaped unicode characters and spaces", async function () {
50+
const xhr = await createRequest("GET", "https://raw.githubusercontent.com/BabylonJS/Assets/master/meshes/στρογγυλεμένος %25 κύβος.glb");
5551
expect(xhr.status).to.equal(200);
56-
})
52+
});
5753
it("should have status=404 for a file that does not exist", async function () {
58-
const xhr = await createRequest("GET", "https://babylonjs.com/invalid");
54+
const xhr = await createRequest("GET", "https://httpbin.org/status/404");
5955
expect(xhr.status).to.equal(404);
60-
})
56+
});
6157
it("should throw something when opening //", async function () {
6258
function openDoubleSlash() {
6359
const xhr = new XMLHttpRequest();
6460
xhr.open("GET", "//");
6561
xhr.send();
6662
}
6763
expect(openDoubleSlash).to.throw();
68-
})
64+
});
6965
it("should throw something when opening a url with no scheme", function () {
7066
function openNoProtocol() {
7167
const xhr = new XMLHttpRequest();
7268
xhr.open("GET", "noscheme.glb");
7369
xhr.send();
7470
}
7571
expect(openNoProtocol).to.throw();
76-
})
72+
});
7773
it("should throw something when sending before opening", function () {
7874
function sendWithoutOpening() {
7975
const xhr = new XMLHttpRequest();
8076
xhr.send();
8177
}
8278
expect(sendWithoutOpening).to.throw();
83-
})
79+
});
8480
it("should open a local file", async function () {
8581
const xhr = await createRequest("GET", "app:///Scripts/tests.js");
8682
expect(xhr).to.have.property('readyState', 4);
8783
expect(xhr).to.have.property('status', 200);
8884
expect(xhr).to.have.property('responseText').with.lengthOf.above(0);
89-
})
85+
});
9086
});
9187

9288
describe("RequestFile", function () {
@@ -99,8 +95,8 @@ describe("RequestFile", function () {
9995
);
10096
}
10197
expect(RequestFile).to.throw();
102-
})
103-
})
98+
});
99+
});
104100

105101
describe("ColorParsing", function () {
106102
expect(_native.Canvas.parseColor("")).to.equal(0);
@@ -120,70 +116,70 @@ describe("ColorParsing", function () {
120116
_native.Canvas.parseColor("unknownColor");
121117
}
122118
expect(incorrectColor).to.throw();
123-
})
119+
});
124120

125121
it("should throw", function () {
126122
function incorrectColor() {
127123
_native.Canvas.parseColor("#");
128124
}
129125
expect(incorrectColor).to.throw();
130-
})
126+
});
131127

132128
it("should throw", function () {
133129
function incorrectColor() {
134130
_native.Canvas.parseColor("#12345");
135131
}
136132
expect(incorrectColor).to.throw();
137-
})
133+
});
138134

139135
it("should throw", function () {
140136
function incorrectColor() {
141137
_native.Canvas.parseColor("rgb(11)");
142138
}
143139
expect(incorrectColor).to.throw();
144-
})
140+
});
145141

146142
it("should throw", function () {
147143
function incorrectColor() {
148144
_native.Canvas.parseColor("rgb(11,22,33");
149145
}
150146
expect(incorrectColor).to.throw();
151-
})
147+
});
152148

153149
it("should throw", function () {
154150
function incorrectColor() {
155151
_native.Canvas.parseColor("rgb(11,22,33,");
156152
}
157153
expect(incorrectColor).to.throw();
158-
})
154+
});
159155

160156
it("should throw", function () {
161157
function incorrectColor() {
162158
_native.Canvas.parseColor("rgba(11, 22, 33, )");
163159
}
164160
expect(incorrectColor).to.throw();
165-
})
161+
});
166162

167163
it("should throw", function () {
168164
function incorrectColor() {
169165
_native.Canvas.parseColor("rgba(11, 22, 33, 44, 55, 66 )");
170166
}
171167
expect(incorrectColor).to.throw();
172-
})
168+
});
173169

174170
it("should throw", function () {
175171
function incorrectColor() {
176172
_native.Canvas.parseColor("rgb");
177173
}
178174
expect(incorrectColor).to.throw();
179-
})
175+
});
180176
it("should throw", function () {
181177
function incorrectColor() {
182178
_native.Canvas.parseColor("rgba");
183179
}
184180
expect(incorrectColor).to.throw();
185-
})
186-
})
181+
});
182+
});
187183

188184
function createSceneAndWait(callback, done) {
189185
const engine = new BABYLON.NativeEngine();
@@ -192,7 +188,7 @@ function createSceneAndWait(callback, done) {
192188
callback(engine, scene);
193189
scene.executeWhenReady(() => {
194190
done();
195-
})
191+
});
196192
}
197193

198194
describe("Materials", function() {
@@ -210,7 +206,7 @@ describe("Materials", function() {
210206
}, done);
211207
}
212208
createEmptyShaderMat()
213-
})
209+
});
214210
/*
215211
TODO: Uncomment tests for materials as we implement more features
216212
it("GradientMaterial should compile", function(done) {
@@ -220,7 +216,7 @@ describe("Materials", function() {
220216
sphere.material = gradientMaterial;
221217
}, done);
222218
});*/
223-
})
219+
});
224220

225221
describe("PostProcesses", function() {
226222
this.timeout(0);
@@ -325,7 +321,7 @@ describe("PostProcesses", function() {
325321
new BABYLON.ScreenSpaceReflectionPostProcess("ssr", scene, 1.0, camera);
326322
}, done);
327323
});*/
328-
})
324+
});
329325

330326
describe("setTimeout", function () {
331327
this.timeout(1000);
@@ -423,7 +419,7 @@ describe("setTimeout", function () {
423419
}, i * 10);
424420
}
425421
});
426-
})
422+
});
427423

428424
describe("clearTimeout", function () {
429425
this.timeout(0);
@@ -438,7 +434,7 @@ describe("clearTimeout", function () {
438434
setTimeout(() => { done(); }, 0);
439435
clearTimeout(undefined);
440436
});
441-
})
437+
});
442438

443439
mocha.run(failures => {
444440
// Test program will wait for code to be set before exiting

Dependencies/UrlLib/Source/UrlRequest_Apple.mm

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,17 @@
66

77
#import <Foundation/Foundation.h>
88

9+
namespace
10+
{
11+
auto URLAllowedCharacterSet = []()
12+
{
13+
NSRange range;
14+
range.location = 0x21;
15+
range.length = 0x7e - range.location + 1;
16+
return [NSCharacterSet characterSetWithRange:range];
17+
}();
18+
}
19+
920
namespace UrlLib
1021
{
1122
class UrlRequest::Impl : public ImplBase
@@ -14,34 +25,32 @@
1425
void Open(UrlMethod method, const std::string& url)
1526
{
1627
m_method = method;
17-
NSString* urlString = [NSString stringWithUTF8String:url.data()];
18-
NSURL* nsURL{[NSURL URLWithString:urlString]};
19-
if (!nsURL || !nsURL.scheme)
28+
m_url = [NSURL URLWithString:[[NSString stringWithUTF8String:url.data()] stringByAddingPercentEncodingWithAllowedCharacters:URLAllowedCharacterSet]];
29+
if (!m_url || !m_url.scheme)
2030
{
2131
throw std::runtime_error{"URL does not have a valid scheme"};
2232
}
23-
NSString* scheme{nsURL.scheme};
33+
NSString* scheme{m_url.scheme};
2434
if ([scheme isEqual:@"app"])
2535
{
26-
NSString* path{[[NSBundle mainBundle] pathForResource:nsURL.path ofType:nil]};
36+
NSString* path = [[NSBundle mainBundle] pathForResource:[m_url.path substringFromIndex:1] ofType:nil];
2737
if (path == nil)
2838
{
2939
throw std::runtime_error{"No file exists at local path"};
3040
}
31-
nsURL = [NSURL fileURLWithPath:path];
41+
m_url = [NSURL fileURLWithPath:path];
3242
}
33-
m_nsURL = nsURL; // Only store the URL if we didn't throw
3443
}
3544

3645
arcana::task<void, std::exception_ptr> SendAsync()
3746
{
38-
if (m_nsURL == nil)
47+
if (m_url == nil)
3948
{
4049
// Complete the task, but retain the default status code of 0 to indicate a client side error.
4150
return arcana::task_from_result<std::exception_ptr>();
4251
}
4352
NSURLSession* session{[NSURLSession sharedSession]};
44-
NSURLRequest* request{[NSURLRequest requestWithURL:m_nsURL]};
53+
NSURLRequest* request{[NSURLRequest requestWithURL:m_url]};
4554

4655
__block arcana::task_completion_source<void, std::exception_ptr> taskCompletionSource{};
4756

@@ -112,7 +121,7 @@ void Open(UrlMethod method, const std::string& url)
112121
}
113122

114123
private:
115-
NSURL* m_nsURL{};
124+
NSURL* m_url{};
116125
NSData* m_responseBuffer{};
117126
};
118127
}

0 commit comments

Comments
 (0)