Skip to content

Commit

Permalink
refactor: move to typescript & vitest
Browse files Browse the repository at this point in the history
  • Loading branch information
JaneJeon committed Aug 19, 2023
1 parent e07515c commit 399b56f
Show file tree
Hide file tree
Showing 14 changed files with 1,688 additions and 2,346 deletions.
7 changes: 6 additions & 1 deletion .eslintrc.yml
Original file line number Diff line number Diff line change
@@ -1 +1,6 @@
extends: '@janejeon'
extends:
- '@janejeon'
- plugin:@typescript-eslint/recommended
parser: '@typescript-eslint/parser'
plugins:
- '@typescript-eslint'
3 changes: 2 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ jobs:
node-version: 18
cache: npm
- run: npm ci
- run: npm run typecheck
- run: npm run lint

test:
Expand All @@ -32,4 +33,4 @@ jobs:
with:
node-version: ${{ matrix.node }}
- run: npm ci
- run: npm test -- --ci
- run: npm test
2 changes: 1 addition & 1 deletion .husky/pre-push
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#!/usr/bin/env sh
. "$(dirname -- "$0")/_/husky.sh"

npx skip-ci && echo "Skipping test..." || npm test
npx skip-ci && echo "Skipping test..." || CI=1 npm test
3 changes: 0 additions & 3 deletions .npmignore

This file was deleted.

24 changes: 19 additions & 5 deletions __mocks__/dns.js → __mocks__/dns.ts
Original file line number Diff line number Diff line change
@@ -1,32 +1,46 @@
import type { LookupOneOptions } from 'dns'

type LookupCallback = (
err: NodeJS.ErrnoException | null,
// In the real dns module, the callback signature is (err, address, family).
// However, because this needs to get promisify'd (just like the real dns.lookup),
// I'm choosing to return an object so the promisify'd function returns the object.
result?: { address: string; family: number }
) => void

// We use the hostname lookups to define how got should behave when visiting a URL.
// For URLs that are supposed to be public, we return a public IP address.
// For URLs that are supposed to be private, we return a private IP address.
// This, *in conjunction with* the HTTP request mocking (the `nock` module),
// defines the entire behaviour of what a request looks like.
export function lookup(hostname, options, callback) {
export function lookup(
hostname: string,
options: LookupOneOptions | LookupCallback,
callback?: LookupCallback
) {
if (callback === undefined) {
callback = options
callback = options as LookupCallback
}

if (hostname === 'public-url.com') {
// The callback return format has to be in form of object as util.promisify doesn't know
// that it's supposed to translate callback(error, address, family) into return value of {address, family}
callback(undefined, { address: '1.1.1.1', family: 4 }) // something I know is public
callback(null, { address: '1.1.1.1', family: 4 }) // something I know is public
}

if (
hostname === 'private-url.com' ||
hostname === 'public-url.com.' ||
hostname === 'private'
) {
callback(undefined, { address: '192.168.0.1', family: 4 }) // should be caught by SSRF protection
callback(null, { address: '192.168.0.1', family: 4 }) // should be caught by SSRF protection
}

if (
hostname ===
'public-url-that-redirects-to-private-url-that-redirects-to-public-url.com'
) {
callback(undefined, { address: '1.1.1.1', family: 4 })
callback(null, { address: '1.1.1.1', family: 4 })
}

// Really make sure that all test cases - at least, the ones that resolve an actual hostname -
Expand Down
3 changes: 0 additions & 3 deletions index.d.ts

This file was deleted.

15 changes: 7 additions & 8 deletions index.test.js → index.test.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
import { expect, describe, it, jest } from '@jest/globals'
import { promisify } from 'util'
import nock from 'nock'
import CacheableLookup from 'cacheable-lookup'

// We can directly mock the "import { lookup } from 'dns'" call in index.js with jest.
const mockDnsModule = await import('./__mocks__/dns.js')
jest.unstable_mockModule('dns', () => mockDnsModule)
// We can directly mock the "import { lookup } from 'dns'" call in index.js with vitest.
import * as mockDns from './__mocks__/dns.js'
vi.mock('dns', () => mockDns)

// However, it does mean that we need to do a dynamic import to make sure we load the mocked import.
// See: https://jestjs.io/docs/ecmascript-modules#module-mocking-in-esm
Expand All @@ -22,9 +21,9 @@ const dnsCache = new CacheableLookup()
// Also, for some reason trying to pass a mocked resolver with a resolve4() and a resolve6()
// that always throws ENOTFOUND doesn't work (i.e. it doesn't use the `lookup` we pass to the options).
// So we just directly mock lookupAsync.
jest
.spyOn(dnsCache, 'lookupAsync')
.mockImplementation(promisify(mockDnsModule.lookup))
vi.spyOn(dnsCache, 'lookupAsync').mockImplementation(
promisify(mockDns.lookup) as CacheableLookup['lookupAsync']
)

const setups = [
{
Expand All @@ -36,7 +35,7 @@ const setups = [
{
title: 'dnsLookup',
options: {
dnsLookup: mockDnsModule.lookup
dnsLookup: mockDns.lookup as unknown as CacheableLookup['lookup']
}
},
{
Expand Down
22 changes: 15 additions & 7 deletions index.js → index.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,27 @@
import { lookup as nativeCallbackLookup } from 'dns'
import { promisify } from 'util'
import got from 'got'
import got, { Options } from 'got'
import ip from 'ipaddr.js'
import debugGen from 'debug'

import type CacheableLookup from 'cacheable-lookup'

const debug = debugGen('got-ssrf')
const nativeLookup = promisify(nativeCallbackLookup) // importing straight from dns/promises limits node.js version to 15 or higher

type LookupFn = (hostname: string) => Promise<{ address: string }>

// Assume all URLs are properly formed by the time it hits the hooks
const protect = async options => {
let lookup
const protect = async (options: Options) => {
let lookup: LookupFn
if (options.dnsCache) {
debug('Using user-provided dnsCache.lookupAsync')
lookup = options.dnsCache.lookupAsync
lookup = (options.dnsCache as CacheableLookup).lookupAsync
} else if (options.dnsLookup) {
debug('Promisifying user-provided dnsLookup')
lookup = promisify(options.dnsLookup)
lookup = promisify(
options.dnsLookup
) as unknown as CacheableLookup['lookupAsync'] // yay wildly incorrect types
} else {
debug('Falling back to native dns/promises lookup')
lookup = nativeLookup
Expand All @@ -28,8 +34,10 @@ const protect = async options => {
// https://github.com/sindresorhus/got/blob/8f77e8d07d8684cde95d351feafaa308b466dff4/source/core/options.ts#L1411

// Check if the hostname is an IP address - we don't need to "lookup" IP addresses!
let IP
const { hostname } = options.url
let IP: string

// Even the got author himself casts this incorrect type: https://github.com/sindresorhus/got/blob/b1d61c173a681755ac23afb2f155f08801c1e7e4/source/core/index.ts#L1121
const { hostname } = options.url as URL

if (ip.IPv4.isIPv4(hostname)) {
IP = hostname
Expand Down
7 changes: 0 additions & 7 deletions jest.config.cjs

This file was deleted.

Loading

0 comments on commit 399b56f

Please sign in to comment.