Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix problems with joinUrls and absolute URLs. #3148

Merged
merged 1 commit into from
Oct 30, 2015
Merged

Fix problems with joinUrls and absolute URLs. #3148

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 33 additions & 10 deletions Source/Core/joinUrls.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ define([
* This is useful when the base URL has a query string that needs to be maintained
* (e.g. a presigned base URL).
* @param {String|Uri} first The base URL.
* @param {String|Uri} second The URL path to join to the base URL.
* @param {String|Uri} second The URL path to join to the base URL. If this URL is absolute, it is returned unmodified.
* @param {Boolean} [appendSlash=true] The boolean determining whether there should be a forward slash between first and second.
* @private
*/
Expand All @@ -42,25 +42,48 @@ define([
second = new Uri(second);
}

// Uri.isAbsolute returns false for a URL like '//foo.com'. So if we have an authority but
// not a scheme, add a scheme matching the page's scheme.
if (definedNotNull(second.authority) && !definedNotNull(second.scheme)) {
if (typeof document !== 'undefined' && defined(document.location) && defined(document.location.href)) {
second.scheme = new Uri(document.location.href).scheme;
} else {
// Not in a browser? Use the first URL's scheme instead.
second.scheme = first.scheme;
}
}

// If the second URL is absolute, use it for the scheme, authority, and path.
var baseUri = first;
if (second.isAbsolute()) {
baseUri = second;
}

var url = '';
if (definedNotNull(first.scheme)) {
url += first.scheme + ':';
if (definedNotNull(baseUri.scheme)) {
url += baseUri.scheme + ':';
}
if (definedNotNull(first.authority)) {
url += '//' + first.authority;
if (definedNotNull(baseUri.authority)) {
url += '//' + baseUri.authority;

if (first.path !== "") {
if (baseUri.path !== '') {
url = url.replace(/\/?$/, '/');
first.path = first.path.replace(/^\/?/g, '');
baseUri.path = baseUri.path.replace(/^\/?/g, '');
}
}

if (appendSlash) {
url += first.path.replace(/\/?$/, '/') + second.path.replace(/^\/?/g, '');
// Combine the paths (only if second is relative).
if (baseUri === first) {
if (appendSlash) {
url += first.path.replace(/\/?$/, '/') + second.path.replace(/^\/?/g, '');
} else {
url += first.path + second.path;
}
} else {
url += first.path + second.path;
url += second.path;
}

// Combine the queries and fragments.
var hasFirstQuery = definedNotNull(first.query);
var hasSecondQuery = definedNotNull(second.query);
if (hasFirstQuery && hasSecondQuery) {
Expand Down
29 changes: 29 additions & 0 deletions Specs/Core/CesiumTerrainProviderSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ defineSuite([
'Core/QuantizedMeshTerrainData',
'Core/TerrainProvider',
'Specs/pollToPromise',
'ThirdParty/Uri',
'ThirdParty/when'
], function(
CesiumTerrainProvider,
Expand All @@ -24,6 +25,7 @@ defineSuite([
QuantizedMeshTerrainData,
TerrainProvider,
pollToPromise,
Uri,
when) {
"use strict";
/*global jasmine,describe,xdescribe,it,xit,expect,beforeEach,afterEach,beforeAll,afterAll,spyOn*/
Expand Down Expand Up @@ -377,6 +379,33 @@ defineSuite([
});
});

it('supports scheme-less template URLs in layer.json resolved with absolute URL', function() {
returnTileJson('Data/CesiumTerrainTileJson/MultipleUrls.tile.json');

var baseUri = new Uri(document.location.href);
var relativeUri = new Uri('Data/CesiumTerrainTileJson');
var url = relativeUri.resolve(baseUri).toString();

var provider = new CesiumTerrainProvider({
url : url
});

return pollToPromise(function() {
return provider.ready;
}).then(function() {
spyOn(loadWithXhr, 'load');
provider.requestTileGeometry(0, 0, 0);
expect(loadWithXhr.load.calls.mostRecent().args[0]).toContain('foo0.com');
provider.requestTileGeometry(1, 0, 0);
expect(loadWithXhr.load.calls.mostRecent().args[0]).toContain('foo1.com');
provider.requestTileGeometry(1, -1, 0);
expect(loadWithXhr.load.calls.mostRecent().args[0]).toContain('foo2.com');
provider.requestTileGeometry(1, 0, 1);
expect(loadWithXhr.load.calls.mostRecent().args[0]).toContain('foo3.com');
});
});


it('uses the proxy if one is supplied', function() {
var baseUrl = 'made/up/url';

Expand Down
4 changes: 2 additions & 2 deletions Specs/Core/joinUrlsSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ defineSuite([

it('appends qualfied url correctly to qualified url', function() {
var result = joinUrls(qualifiedUrl, qualifiedUrl);
expect(result).toEqual(qualifiedUrl + "/");
expect(result).toEqual(qualifiedUrl);
});

it('appends qualfied url correctly to qualified url with path', function() {
Expand All @@ -109,7 +109,7 @@ defineSuite([

it('appends qualfied url with path correctly to qualified url with path', function() {
var result = joinUrls(qualifiedUrlWithPath, qualifiedUrlWithPath);
expect(result).toEqual(qualifiedUrlWithPath + absolutePath);
expect(result).toEqual(qualifiedUrlWithPath);
});

it('appends absolute path correctly to qualified url with query string', function() {
Expand Down