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

Add StellarTomlResolver timeout option #166

Merged
merged 8 commits into from
Jun 5, 2018
Merged
12 changes: 10 additions & 2 deletions src/call_builder.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,18 @@ let toBluebird = require("bluebird").resolve;
*
* This is an **abstract** class. Do not create this object directly, use {@link Server} class.
* @param {string} serverUrl
* @param {object} [opts]
* @param {number} [opts.timeout] - Allow a timeout, default: 0. In ms.
* @class CallBuilder
*/
export class CallBuilder {
constructor(serverUrl) {
constructor(serverUrl, opts = {}) {
this.timeout = 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Default here should be a global setting so Config.getTimeout().


if (typeof opts.timeout === 'number') {
this.timeout = opts.timeout;
}

this.url = serverUrl;
this.filter = [];
this.originalSegments = this.url.segment() || [];
Expand Down Expand Up @@ -163,7 +171,7 @@ export class CallBuilder {

// Temp fix for: https://github.com/stellar/js-stellar-sdk/issues/15
url.addQuery('c', Math.random());
var promise = axios.get(url.toString())
var promise = axios.get(url.toString(), {timeout: this.timeout})
.then(response => response.data)
.catch(this._handleNetworkError);
return toBluebird(promise);
Expand Down
23 changes: 22 additions & 1 deletion src/config.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import clone from 'lodash/clone';

let defaultConfig = {
allowHttp: false
allowHttp: false,
timeout: 0
};

let config = clone(defaultConfig);
Expand All @@ -13,11 +14,13 @@ let config = clone(defaultConfig);
* ```
* import {Config} from 'stellar-sdk';
* Config.setAllowHttp(true);
* Config.setTimout(5000);
* ```
*
* Usage browser:
* ```
* StellarSdk.Config.setAllowHttp(true);
* StellarSdk.Config.setTimout(5000);
* ```
* @static
*/
Expand All @@ -32,6 +35,16 @@ class Config {
config.allowHttp = value;
}

/**
* Sets `timeout` flag globally. When set to anything besides 0, StellarTOMLResolver will timeout after specified time (ms).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update the comment. It's not StellarTOMLResolver only anymore.

* Default: 0.
* @param {number} value
* @static
*/
static setTimeout(value) {
config.timeout = value;
}

/**
* Returns the value of `allowHttp` flag.
* @static
Expand All @@ -40,6 +53,14 @@ class Config {
return clone(config.allowHttp);
}

/**
* Returns the value of `timeout` flag.
* @static
*/
static getTimeout() {
return clone(config.timeout);
}

/**
* Sets all global config flags to default values.
* @static
Expand Down
42 changes: 32 additions & 10 deletions src/federation_server.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export const FEDERATION_RESPONSE_MAX_SIZE = 100 * 1024;
* @param {string} domain Domain this server represents
* @param {object} [opts]
* @param {boolean} [opts.allowHttp] - Allow connecting to http servers, default: `false`. This must be set to false in production deployments! You can also use {@link Config} class to set this globally.
* @param {number} [opts.timeout] - Allow a timeout, default: 0. In ms.
*/
export class FederationServer {
constructor(serverURL, domain, opts = {}) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Static method instantiate this class but opts.timeout is not persisted anywhere.

Expand Down Expand Up @@ -71,6 +72,7 @@ export class FederationServer {
* @param {string} value Stellar Address (ex. `bob*stellar.org`)
* @param {object} [opts]
* @param {boolean} [opts.allowHttp] - Allow connecting to http servers, default: `false`. This must be set to false in production deployments!
* @param {number} [opts.timeout] - Allow a timeout, default: 0. Allows user to avoid nasty lag due to TOML resolve issue.
* @returns {Promise}
*/
static resolve(value, opts = {}) {
Expand Down Expand Up @@ -109,10 +111,11 @@ export class FederationServer {
* @param {string} domain Domain to get federation server for
* @param {object} [opts]
* @param {boolean} [opts.allowHttp] - Allow connecting to http servers, default: `false`. This must be set to false in production deployments!
* @param {number} [opts.timeout] - Allow a timeout, default: 0. Allows user to avoid nasty lag due to TOML resolve issue.
* @returns {Promise}
*/
static createForDomain(domain, opts = {}) {
return StellarTomlResolver.resolve(domain)
return StellarTomlResolver.resolve(domain, opts)
.then(tomlObject => {
if (!tomlObject.FEDERATION_SERVER) {
return Promise.reject(new Error('stellar.toml does not contain FEDERATION_SERVER field'));
Expand All @@ -125,43 +128,62 @@ export class FederationServer {
* Returns a Promise that resolves to federation record if the user was found for a given Stellar address.
* @see <a href="https://www.stellar.org/developers/learn/concepts/federation.html" target="_blank">Federation doc</a>
* @param {string} address Stellar address (ex. `bob*stellar.org`). If `FederationServer` was instantiated with `domain` param only username (ex. `bob`) can be passed.
* @returns {Promise}
* @param {object} [opts]
* @param {boolean} [opts.allowHttp] - Allow connecting to http servers, default: `false`. This must be set to false in production deployments!
* @param {number} [opts.timeout] - Allow a timeout, default: 0. In ms.
*/
resolveAddress(address) {
resolveAddress(address, opts = {}) {
if (address.indexOf('*') < 0) {
if (!this.domain) {
return Promise.reject(new Error('Unknown domain. Make sure `address` contains a domain (ex. `bob*stellar.org`) or pass `domain` parameter when instantiating the server object.'));
}
address = `${address}*${this.domain}`;
}
let url = this.serverURL.query({type: 'name', q: address});
return this._sendRequest(url);
return this._sendRequest(url, opts);
}

/**
* Returns a Promise that resolves to federation record if the user was found for a given account ID.
* @see <a href="https://www.stellar.org/developers/learn/concepts/federation.html" target="_blank">Federation doc</a>
* @param {string} accountId Account ID (ex. `GBYNR2QJXLBCBTRN44MRORCMI4YO7FZPFBCNOKTOBCAAFC7KC3LNPRYS`)
* @param {object} [opts]
* @param {boolean} [opts.allowHttp] - Allow connecting to http servers, default: `false`. This must be set to false in production deployments!
* @param {number} [opts.timeout] - Allow a timeout, default: 0. In ms.
* @returns {Promise}
*/
resolveAccountId(accountId) {
resolveAccountId(accountId, opts = {}) {
let url = this.serverURL.query({type: 'id', q: accountId});
return this._sendRequest(url);
return this._sendRequest(url, opts);
}

/**
* Returns a Promise that resolves to federation record if the sender of the transaction was found for a given transaction ID.
* @see <a href="https://www.stellar.org/developers/learn/concepts/federation.html" target="_blank">Federation doc</a>
* @param {string} transactionId Transaction ID (ex. `3389e9f0f1a65f19736cacf544c2e825313e8447f569233bb8db39aa607c8889`)
* @param {object} [opts]
* @param {boolean} [opts.allowHttp] - Allow connecting to http servers, default: `false`. This must be set to false in production deployments!
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

allowHttp is not needed here as it's checked in constructor per server.

* @param {number} [opts.timeout] - Allow a timeout, default: 0. In ms.
* @returns {Promise}
*/
resolveTransactionId(transactionId) {
resolveTransactionId(transactionId, opts = {}) {
let url = this.serverURL.query({type: 'txid', q: transactionId});
return this._sendRequest(url);
return this._sendRequest(url, opts);
}

_sendRequest(url) {
return axios.get(url.toString(), {maxContentLength: FEDERATION_RESPONSE_MAX_SIZE})
_sendRequest(url, opts = {}) {
let allowHttp = Config.isAllowHttp();
let timeout = Config.getTimeout();

if (typeof opts.allowHttp !== 'undefined') {
allowHttp = opts.allowHttp;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for allowHttp processing here, it's been checked in constructor.


if (typeof opts.timeout === 'number') {
timeout = opts.timeout;
}

return axios.get(url.toString(), {maxContentLength: FEDERATION_RESPONSE_MAX_SIZE, timeout})
.then(response => {
if (typeof response.data.memo != "undefined" && typeof response.data.memo != 'string') {
throw new Error("memo value should be of type string");
Expand Down
1 change: 1 addition & 0 deletions src/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export const SUBMIT_TRANSACTION_TIMEOUT = 60*1000;
* @param {string} serverURL Horizon Server URL (ex. `https://horizon-testnet.stellar.org`).
* @param {object} [opts]
* @param {boolean} [opts.allowHttp] - Allow connecting to http servers, default: `false`. This must be set to false in production deployments! You can also use {@link Config} class to set this globally.
* @param {number} [opts.timeout] - Allow a timeout, default: 0. In ms.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not passed to CallBuilder anywhere.

*/
export class Server {
constructor(serverURL, opts = {}) {
Expand Down
10 changes: 9 additions & 1 deletion src/stellar_toml_resolver.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,19 +26,27 @@ export class StellarTomlResolver {
* @param {string} domain Domain to get stellar.toml file for
* @param {object} [opts]
* @param {boolean} [opts.allowHttp] - Allow connecting to http servers, default: `false`. This must be set to false in production deployments!
* @param {number} [opts.timeout] - Allow a timeout, default: 0. Allows user to avoid nasty lag due to TOML resolve issue.
* @returns {Promise}
*/
static resolve(domain, opts = {}) {
let allowHttp = Config.isAllowHttp();
let timeout = Config.getTimeout();

if (typeof opts.allowHttp !== 'undefined') {
allowHttp = opts.allowHttp;
}

if (typeof opts.timeout === 'number') {
timeout = opts.timeout;
}

let protocol = 'https';
if (allowHttp) {
protocol = 'http';
}
return axios.get(`${protocol}://${domain}/.well-known/stellar.toml`, {maxContentLength: STELLAR_TOML_MAX_SIZE})

return axios.get(`${protocol}://${domain}/.well-known/stellar.toml`, {maxContentLength: STELLAR_TOML_MAX_SIZE, timeout})
.then(response => {
try {
let tomlObject = toml.parse(response.data);
Expand Down
19 changes: 19 additions & 0 deletions test/unit/federation_server_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,25 @@ FEDERATION_SERVER="https://api.stellar.org/federation"

StellarSdk.FederationServer.createForDomain('acme.com').should.be.rejectedWith(/stellar.toml does not contain FEDERATION_SERVER field/).and.notify(done);
});

it("rejects after given timeout when timeout specified in opts param", function (done) {
// Unable to create temp server in a browser
if (typeof window != 'undefined') {
return done();
}
var response = Array(StellarSdk.STELLAR_TOML_MAX_SIZE+10).join('a');
let tempServer = http.createServer((req, res) => {
setTimeout(() => {
res.setHeader('Content-Type', 'text/x-toml; charset=UTF-8');
res.end(response);
}, 10000);
}).listen(4444, () => {
StellarSdk.FederationServer.createForDomain("localhost:4444", {allowHttp: true, timeout: 1000})
.should.be.rejectedWith(/timeout of 1000ms exceeded/)
.notify(done)
.then(() => tempServer.close());
});
});
});

describe('FederationServer.resolve', function () {
Expand Down
39 changes: 39 additions & 0 deletions test/unit/stellar_toml_resolver_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -106,5 +106,44 @@ FEDERATION_SERVER="https://api.stellar.org/federation"
.then(() => tempServer.close());
});
});

it("rejects after given timeout when global Config.timeout flag is set", function (done) {
StellarSdk.Config.setTimeout(1000);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add Config.setDefault(); to afterEach here too.


if (typeof window != 'undefined') {
return done();
}
var response = Array(StellarSdk.STELLAR_TOML_MAX_SIZE+10).join('a');
let tempServer = http.createServer((req, res) => {
setTimeout(() => {
res.setHeader('Content-Type', 'text/x-toml; charset=UTF-8');
res.end(response);
}, 10000);
}).listen(4444, () => {
StellarSdk.StellarTomlResolver.resolve("localhost:4444", {allowHttp: true})
.should.be.rejectedWith(/timeout of 1000ms exceeded/)
.notify(done)
.then(() => tempServer.close());
});
});

it("rejects after given timeout when timeout specified in StellarTomlResolver opts param", function (done) {
// Unable to create temp server in a browser
if (typeof window != 'undefined') {
return done();
}
var response = Array(StellarSdk.STELLAR_TOML_MAX_SIZE+10).join('a');
let tempServer = http.createServer((req, res) => {
setTimeout(() => {
res.setHeader('Content-Type', 'text/x-toml; charset=UTF-8');
res.end(response);
}, 10000);
}).listen(4444, () => {
StellarSdk.StellarTomlResolver.resolve("localhost:4444", {allowHttp: true, timeout: 1000})
.should.be.rejectedWith(/timeout of 1000ms exceeded/)
.notify(done)
.then(() => tempServer.close());
});
});
});
});