From ba5167596aec06da2eb58d179cfc22540200a4f0 Mon Sep 17 00:00:00 2001 From: Morley Zhi Date: Tue, 10 Mar 2020 15:35:07 -0400 Subject: [PATCH] Don't assume that we're getting back JSON from anyone (#145) - Imbibe API requests as text, and wrap the JSON parsing in try blocks - Remove trailing slashes from transfer urls --- CHANGELOG.md | 2 ++ package.json | 2 +- src/KeyManager.ts | 20 +++++++++++---- src/transfers/TransferProvider.ts | 41 +++++++++++++++++++++++-------- 4 files changed, 49 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 63c3e12b..0ddf6fa8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## In master +- When parsing API responses, take extra care making sure that the responses are + valid JSON, and throw if not. - [KeyManager] Correctly handle non-successful HTTP status codes when fetching auth tokens. - [Transfers] Correctly handle non-successful HTTP status codes for diff --git a/package.json b/package.json index a26c7edb..047f07fa 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@stellar/wallet-sdk", - "version": "0.0.9-rc.3", + "version": "0.0.9-rc.4", "description": "Libraries to help you write Stellar-enabled wallets in Javascript", "main": "dist/index.js", "types": "dist/index.d.ts", diff --git a/src/KeyManager.ts b/src/KeyManager.ts index 941789e5..b376b1df 100644 --- a/src/KeyManager.ts +++ b/src/KeyManager.ts @@ -325,11 +325,21 @@ export class KeyManager { const keyNetwork = key.network || this.defaultNetworkPassphrase; - const { - transaction, - network_passphrase, - error, - } = await challengeRes.json(); + const text = await challengeRes.text(); + + let transaction; + let network_passphrase; + let error; + + try { + const json = JSON.parse(text); + + transaction = json.transaction; + network_passphrase = json.network_passphrase; + error = json.error; + } catch (e) { + throw new Error(`Request for challenge returned invalid JSON: ${text}`); + } if (error) { throw new Error(error); diff --git a/src/transfers/TransferProvider.ts b/src/transfers/TransferProvider.ts index a64d2e48..6f1e3f5e 100644 --- a/src/transfers/TransferProvider.ts +++ b/src/transfers/TransferProvider.ts @@ -99,7 +99,8 @@ export abstract class TransferProvider { throw new Error("Required parameter `operation` missing!"); } - this.transferServer = transferServer; + // remove the trailing / + this.transferServer = transferServer.replace(/\/$/, ""); this.operation = operation; this.account = account; @@ -127,10 +128,20 @@ export abstract class TransferProvider { } } - const rawInfo = (await response.json()) as RawInfoResponse; - const info = parseInfo(rawInfo); - this.info = info; - return info; + const text = await response.text(); + + try { + const rawInfo = JSON.parse(text) as RawInfoResponse; + const info = parseInfo(rawInfo); + this.info = info; + return info; + } catch (e) { + throw new Error( + `Error parsing the response of ${ + this.transferServer + }/info as JSON: ${text}`, + ); + } } protected getHeaders(): Headers { @@ -282,9 +293,13 @@ export abstract class TransferProvider { } } - const { transaction }: { transaction: Transaction } = await response.json(); - - return _normalizeTransaction(transaction); + const text = await response.text(); + try { + const { transaction }: { transaction: Transaction } = JSON.parse(text); + return _normalizeTransaction(transaction); + } catch (e) { + throw new Error(`Auth challenge response wasn't valid JSON: ${text}`); + } } /** @@ -631,8 +646,14 @@ export abstract class TransferProvider { } } - const { fee: feeResponse } = await response.json(); - return feeResponse as number; + const text = await response.text(); + + try { + const { fee: feeResponse } = JSON.parse(text); + return feeResponse as number; + } catch (e) { + throw new Error(`Fee endpoint returned invalid JSON: ${text}`); + } default: throw new Error(