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.

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

The naive root + path might cause cases of double slashes e.g. http://path//relative-path. #109

Merged
merged 6 commits into from
Dec 10, 2013
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
node_modules/*
.cache
b041042364d89046c003ca151a6254ef
.idea
135 changes: 110 additions & 25 deletions lib/requests.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ var BatchParser = require('./exp/batchparser'),
utils = require('./utils.js'),
querystring = require('querystring'),
url = require('url'),
path = require('path'),
util = require('util');


Expand Down Expand Up @@ -99,6 +100,79 @@ BaseRequest.prototype.handleResponse = function(opt_fn) {
};
};

function slashPath(fullPath) {
function strEndsWith(str, suffix) {
// don't try to make this !=== it won't work
return str.match(suffix+"$") == suffix;
}

if (strEndsWith(fullPath, "/")) {
return fullPath;
}

return fullPath + '/';
}

function unslashPath(fullPath) {
function strStartsWith(str, prefix) {
return str.lastIndexOf(prefix, 0) === 0;
}

if (!strStartsWith(fullPath, '/')) {
return fullPath;
}

return fullPath.substring(1);
}

/**
* private: create the upload url for the method

This comment was marked as spam.

* @param {string} root
* @param {Array} paths
* @param {object} queryParams
*/
BaseRequest.buildUri = function(root, paths, queryParams) {
var root = root || '',

This comment was marked as spam.

fullPath = '';

for (var i=0;i<paths.length;i++) {

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

var currentPath = paths[i];

var isFirstPathElement = i === 0,
isLastPathElement = i === paths.length-1;

if (!isFirstPathElement) {
currentPath = unslashPath(currentPath);
}

fullPath = url.resolve(fullPath, currentPath);

if (!isLastPathElement) {
fullPath = slashPath(fullPath);
}
}

fullPath = url.resolve(root, fullPath);

queryParams = queryParams || {}
// replace path query parameters, if there are on the path
for (var paramName in queryParams) {
var param = '{' + paramName + '}';
if (fullPath.indexOf(param) >= 0) {
fullPath = fullPath.replace(param, queryParams[paramName]);
delete queryParams[paramName];
}
}

// append the other query parameters
if (Object.keys(queryParams).length > 0) {
fullPath += '?' + querystring.stringify(queryParams);
}

return fullPath;
};


/**
* Constructs a new Request.
* @constructor
Expand Down Expand Up @@ -139,26 +213,32 @@ util.inherits(Request, BaseRequest);
* @param {object} queryParams Query params.
* @return {String} The method's REST path.
*/
Request.prototype.generatePath = function(queryParams) {
var path = this.apiMeta.basePath + this.methodMeta.path;
Request.prototype.generateMethodUri = function(baseUri, paths, queryParams) {

queryParams = queryParams || {};
if (this.apiKey) {
queryParams.key = this.apiKey;
}
if (typeof paths == 'string' || paths instanceof String) {

This comment was marked as spam.

paths = paths.length > 0 ? [paths] : [];
} else {
paths = paths || [];
}

// replace path query parameters, if there are on the path
for (var paramName in queryParams) {
if (path.indexOf('{' + paramName + '}') >= 0) {
path = path.replace('{' + paramName + '}', queryParams[paramName]);
delete queryParams[paramName];
if (this.apiMeta.basePath) {
paths.push(this.apiMeta.basePath);
}
}
// append the oher query parameters
if (Object.keys(queryParams).length > 0) {
path += '?' + querystring.stringify(queryParams);
}
return path;

if (this.methodMeta.path) {

This comment was marked as spam.

paths.push(this.methodMeta.path)

This comment was marked as spam.

}

queryParams = queryParams || {};
if (this.apiKey) {
queryParams.key = this.apiKey;
}

return BaseRequest.buildUri(baseUri, paths, queryParams)

This comment was marked as spam.

};

Request.prototype.generatePath = function(params) {
return this.generateMethodUri('', [], params);
};

/**
Expand All @@ -169,14 +249,17 @@ Request.prototype.generatePayload = function(rootUrl) {
return !this.media ? this.generateBody(rootUrl) : this.generateUploadBody(rootUrl);
};


/**
* Generates request opts.
* @return {object} The request object.
*/
Request.prototype.generateBody = function(rootUrl) {
var path = this.generatePath(this.params);
var path = this.generatePath(this.params),
rootUrl = rootUrl || this.methodMeta.baseUri;

return {
uri: rootUrl + path,
uri: this.generateMethodUri(rootUrl, '', this.params),
path: path,
method: this.methodMeta.httpMethod,
json: this.body
Expand All @@ -190,13 +273,13 @@ Request.prototype.generateBody = function(rootUrl) {
*/
Request.prototype.generateUploadBody = function(rootUrl) {
var params = this.params || {};
rootUrl = rootUrl.replace(/\/+$/, ''); // Remove any trailing slashes.
rootUrl = rootUrl || this.methodMeta.baseUri

if (!this.body) {
params.uploadType = 'media';

return {
uri: rootUrl + '/upload' + this.generatePath(params),
uri: this.generateMethodUri(rootUrl, "/upload", this.params),
method: this.methodMeta.httpMethod,
headers: {
'Content-Type': this.media.mimeType,
Expand All @@ -209,7 +292,7 @@ Request.prototype.generateUploadBody = function(rootUrl) {
params.uploadType = 'multipart';

return {
uri: rootUrl + '/upload' + this.generatePath(params),
uri: this.generateMethodUri(rootUrl, "/upload", this.params),

This comment was marked as spam.

method: this.methodMeta.httpMethod,
multipart: [{
'Content-Type': 'application/json',
Expand Down Expand Up @@ -288,7 +371,9 @@ BatchRequest.prototype.add = function(request) {
*/
BatchRequest.prototype.generatePayload = function(rootUrl) {
// todo: implement
var multipart = [];
var multipart = [],
rootUrl = rootUrl || BaseRequest.BASE_URL;

this.requests_.forEach(function(request, key) {
var body = [];
var payload = request.generatePayload(rootUrl);
Expand All @@ -307,7 +392,7 @@ BatchRequest.prototype.generatePayload = function(rootUrl) {

return {
method: 'POST',
uri: rootUrl + '/batch',
uri: BaseRequest.buildUri(rootUrl, ['/batch']),
multipart: multipart,
headers: {
'content-type': 'multipart/mixed'
Expand Down Expand Up @@ -352,7 +437,7 @@ BatchRequest.prototype.handleResponse = function(opt_fn) {
BatchRequest.prototype.execute = function(opt_callback) {
opt_callback = opt_callback || function() {};
var callback = this.handleResponse(opt_callback),
requestOpts = this.generatePayload(BaseRequest.BASE_URL);
requestOpts = this.generatePayload();

if (this.authClient) {
this.authClient.request(requestOpts, callback);
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
"url": "0.7.9"
},
"scripts": {
"test": "mocha tests/* --reporter spec --timeout 40000"
"test": "mocha --reporter spec --timeout 40000"
},
"license": "Apache 2"
}
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
16 changes: 8 additions & 8 deletions tests/test.requests.js → test/test.requests.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,9 @@ describe('Requests', function() {
.execute(function(err, client) {

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

var obj = { longUrl: 'http://someurl...' };
var request = client.urlshortener.url.insert(obj);
var payload = request.generatePayload('root-url');
var payload = request.generatePayload('http://root-url');

assert.equal(payload.uri, 'root-url/urlshortener/v1/url');
assert.equal(payload.uri, 'http://root-url/urlshortener/v1/url');
assert.equal(payload.method, 'POST');
assert.equal(payload.json.longUrl, 'http://someurl...');
done();
Expand Down Expand Up @@ -91,8 +91,8 @@ describe('Requests', function() {
.execute(function(err, client) {
var params = { shortUrl: 'a' };
var request = client.urlshortener.url.get(params);
var payload = request.generatePayload('root-url');
assert.equal(payload.uri, 'root-url/urlshortener/v1/url?shortUrl=a');
var payload = request.generatePayload('http://root-url');
assert.equal(payload.uri, 'http://root-url/urlshortener/v1/url?shortUrl=a');
assert.equal(payload.method, 'GET');
done();
});
Expand Down Expand Up @@ -195,9 +195,9 @@ describe('Requests', function() {
googleapis.discover('drive', 'v2').execute(function(err, client){
var req = client.drive.files.insert().withMedia('text/plain', 'hey');

var payload = req.generatePayload('root-url');
var payload = req.generatePayload('http://root-url');
assert.equal(payload.method, 'POST');
assert.equal(payload.uri, 'root-url/upload/drive/v2/files?uploadType=media');
assert.equal(payload.uri, 'http://root-url/upload/drive/v2/files?uploadType=media');
assert.equal(payload.headers['Content-Type'], 'text/plain');
assert.equal(payload.body, 'hey');
done();
Expand All @@ -211,9 +211,9 @@ describe('Requests', function() {
.insert({ title: 'title' })
.withMedia('text/plain', 'hey');

var payload = req.generatePayload('root-url');
var payload = req.generatePayload('http://root-url');
assert.equal(payload.method, 'POST');
assert.equal(payload.uri, 'root-url/upload/drive/v2/files?uploadType=multipart');
assert.equal(payload.uri, 'http://root-url/upload/drive/v2/files?uploadType=multipart');
assert.equal(payload.multipart[0]['Content-Type'], 'application/json');
assert.equal(payload.multipart[0].body, '{"title":"title"}');
assert.equal(payload.multipart[1]['Content-Type'], 'text/plain');
Expand Down
File renamed without changes.