Always check res.ok and content-type header

stackernews · Nov 20, 2024 · 856014e · 856014e
1 parent c88afc5
commit 856014e
Show file tree

Hide file tree

Showing 8 changed files with 44 additions and 25 deletions.
diff --git a/lib/cln.js b/lib/cln.js
@@ -1,7 +1,7 @@
 import fetch from 'cross-fetch'
 import crypto from 'crypto'
 import { getAgent } from '@/lib/proxy'
-import { assertContentTypeJson } from './url'
+import { assertContentTypeJson, assertResponseOk } from './url'
 
 export const createInvoice = async ({ socket, rune, cert, label, description, msats, expiry }) => {
   const agent = getAgent({ hostname: socket, cert })
@@ -27,9 +27,8 @@ export const createInvoice = async ({ socket, rune, cert, label, description, ms
     })
   })
 
-  if (!res.ok) {
-    assertContentTypeJson(res)
-  }
+  assertResponseOk(res)
+  assertContentTypeJson(res)
 
   const inv = await res.json()
   if (inv.error) {

diff --git a/lib/url.js b/lib/url.js
@@ -213,6 +213,12 @@ export function parseNwcUrl (walletConnectUrl) {
   return params
 }
 
+export function assertResponseOk (res) {
+  if (!res.ok) {
+    throw new Error(`POST ${res.url}: ${res.status} ${res.statusText}`)
+  }
+}
+
 export function assertContentTypeJson (res) {
   const contentType = res.headers.get('content-type')
   if (!contentType || !contentType.includes('application/json')) {

diff --git a/wallets/blink/common.js b/wallets/blink/common.js
@@ -1,3 +1,5 @@
+import { assertContentTypeJson, assertResponseOk } from '@/lib/url'
+
 export const galoyBlinkUrl = 'https://api.blink.sv/graphql'
 export const galoyBlinkDashboardUrl = 'https://dashboard.blink.sv/'
 
@@ -37,15 +39,14 @@ export async function request (authToken, query, variables = {}) {
     body: JSON.stringify({ query, variables })
   }
   const res = await fetch(galoyBlinkUrl, options)
-  if (res.status >= 400 && res.status <= 599) {
-    // consume res
-    res.text().catch(() => {})
-    if (res.status === 401) {
-      throw new Error('unauthorized')
-    } else {
-      throw new Error('API responded with HTTP ' + res.status)
-    }
-  }
+
+  // consume response body to avoid memory leaks
+  // see https://github.com/nodejs/node/issues/51162
+  res.text().catch(() => {})
+
+  assertResponseOk(res)
+  assertContentTypeJson(res)
+
   return res.json()
 }
 

diff --git a/wallets/lightning-address/server.js b/wallets/lightning-address/server.js
@@ -1,5 +1,6 @@
 import { msatsSatsFloor } from '@/lib/format'
 import { lnAddrOptions } from '@/lib/lnurl'
+import { assertContentTypeJson, assertResponseOk } from '@/lib/url'
 
 export * from '@/wallets/lightning-address'
 
@@ -24,8 +25,13 @@ export const createInvoice = async (
   }
 
   // call callback with amount and conditionally comment
-  const res = await (await fetch(callbackUrl.toString())).json()
-  if (res.status === 'ERROR') {
+  const res = await fetch(callbackUrl.toString())
+
+  assertResponseOk(res)
+  assertContentTypeJson(res)
+
+  const body = await res.json()
+  if (body.status === 'ERROR') {
     throw new Error(res.reason)
   }
 

diff --git a/wallets/lnbits/client.js b/wallets/lnbits/client.js
@@ -33,8 +33,9 @@ async function getWallet ({ url, adminKey, invoiceKey }) {
   headers.append('X-Api-Key', adminKey || invoiceKey)
 
   const res = await fetch(url + path, { method: 'GET', headers })
+
+  assertContentTypeJson(res)
   if (!res.ok) {
-    assertContentTypeJson(res)
     const errBody = await res.json()
     throw new Error(errBody.detail)
   }
@@ -54,8 +55,9 @@ async function postPayment (bolt11, { url, adminKey }) {
   const body = JSON.stringify({ bolt11, out: true })
 
   const res = await fetch(url + path, { method: 'POST', headers, body })
+
+  assertContentTypeJson(res)
   if (!res.ok) {
-    assertContentTypeJson(res)
     const errBody = await res.json()
     throw new Error(errBody.detail)
   }
@@ -73,8 +75,9 @@ async function getPayment (paymentHash, { url, adminKey }) {
   headers.append('X-Api-Key', adminKey)
 
   const res = await fetch(url + path, { method: 'GET', headers })
+
+  assertContentTypeJson(res)
   if (!res.ok) {
-    assertContentTypeJson(res)
     const errBody = await res.json()
     throw new Error(errBody.detail)
   }

diff --git a/wallets/lnbits/server.js b/wallets/lnbits/server.js
@@ -44,8 +44,9 @@ export async function createInvoice (
     agent,
     body
   })
+
+  assertContentTypeJson(res)
   if (!res.ok) {
-    assertContentTypeJson(res)
     const errBody = await res.json()
     throw new Error(errBody.detail)
   }

diff --git a/wallets/phoenixd/client.js b/wallets/phoenixd/client.js
@@ -1,3 +1,5 @@
+import { assertContentTypeJson, assertResponseOk } from '@/lib/url'
+
 export * from '@/wallets/phoenixd'
 
 export async function testSendPayment (config, { logger }) {
@@ -24,9 +26,9 @@ export async function sendPayment (bolt11, { url, primaryPassword }) {
     headers,
     body
   })
-  if (!res.ok) {
-    throw new Error(`POST ${res.url}: ${res.status} ${res.statusText}`)
-  }
+
+  assertResponseOk(res)
+  assertContentTypeJson(res)
 
   const payment = await res.json()
   const preimage = payment.paymentPreimage

diff --git a/wallets/phoenixd/server.js b/wallets/phoenixd/server.js
@@ -1,4 +1,5 @@
 import { msatsToSats } from '@/lib/format'
+import { assertContentTypeJson, assertResponseOk } from '@/lib/url'
 
 export * from '@/wallets/phoenixd'
 
@@ -28,9 +29,9 @@ export async function createInvoice (
     headers,
     body
   })
-  if (!res.ok) {
-    throw new Error(`POST ${res.url}: ${res.status} ${res.statusText}`)
-  }
+
+  assertResponseOk(res)
+  assertContentTypeJson(res)
 
   const payment = await res.json()
   return payment.serialized