Skip to content
This repository has been archived by the owner on Feb 8, 2024. It is now read-only.

Commit

Permalink
Don't assume that we're getting back JSON from anyone (#145)
Browse files Browse the repository at this point in the history
- Imbibe API requests as text, and wrap the JSON parsing in try blocks
- Remove trailing slashes from transfer urls
  • Loading branch information
Morley Zhi authored Mar 10, 2020
1 parent 8f4dcf0 commit ba51675
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 16 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -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",
Expand Down
20 changes: 15 additions & 5 deletions src/KeyManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
41 changes: 31 additions & 10 deletions src/transfers/TransferProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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}`);
}
}

/**
Expand Down Expand Up @@ -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(
Expand Down

0 comments on commit ba51675

Please sign in to comment.