Skip to content

Commit

Permalink
fix(HTTP and WebSocket servers): Instantiate correct type of address
Browse files Browse the repository at this point in the history
  • Loading branch information
nokome committed Oct 30, 2020
1 parent 9776675 commit e747f0f
Show file tree
Hide file tree
Showing 6 changed files with 90 additions and 10 deletions.
5 changes: 5 additions & 0 deletions src/base/Transports.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,11 @@ describe('HttpAddress', () => {
expect(new HttpAddress()).toEqual(defaults)
})

test('constructor: port only', () => {
expect(new HttpAddress(8000)).toEqual({ ...defaults, port: 8000 })
expect(new HttpAddress('8000')).toEqual({ ...defaults, port: 8000 })
})

test('constructor: port is scheme default', () => {
expect(new HttpAddress('https://127.0.0.1')).toEqual({
...defaults,
Expand Down
2 changes: 1 addition & 1 deletion src/base/Transports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ export function parseTcpAddress(
let { scheme, host, path } = { path: undefined, ...defaults }
let port
if (typeof address === 'string') {
// Parse as string of port number (useful for C)
// Attempt to parse string as port number
const match = /^[0-9]{2,5}$/.exec(address)
if (match !== null) {
port = parseInt(match[0])
Expand Down
31 changes: 31 additions & 0 deletions src/http/HttpServer.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import { HttpAddress } from '../base/Transports'
import { HttpServer } from './HttpServer'

describe('HttpServer.address.url', () => {
test('default', () => {
expect(new HttpServer().address.url()).toBe('http://127.0.0.1:8000')
})

test('string', () => {
expect(new HttpServer('8001').address.url()).toBe('http://127.0.0.1:8001')
})

test('integer', () => {
expect(new HttpServer(8002).address.url()).toBe('http://127.0.0.1:8002')
})

test('object', () => {
expect(
new HttpServer({
host: '192.0.0.1',
port: 8003,
}).address.url()
).toBe('http://192.0.0.1:8003')
})

test('HttpAddress', () => {
expect(new HttpServer(new HttpAddress(8004)).address.url()).toBe(
'http://127.0.0.1:8004'
)
})
})
27 changes: 19 additions & 8 deletions src/http/HttpServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,15 @@ const log = getLogger('executa:http:server')
*
* This server class performs JSON-RPC over HTTP.
*
* For the `/` endpoint, the request body should be
* an JSON-RPC request and the response body will be
* a JSON-RPC request.
* For the `POST /` route, the request body should be
* a JSON-RPC request. The response body will be
* a JSON-RPC response.
*
* For the `POST /<method>` routes, the request body should
* correspond to the `params` property of a JSON-RPC request.
* The response body corresponds to the `result` property of
* a JSON-RPC response for `200 OK` responses and the `error`
* property otherwise.
*/
export class HttpServer extends TcpServer {
/**
Expand All @@ -59,7 +65,7 @@ export class HttpServer extends TcpServer {
address: HttpAddressInitializer = new HttpAddress({ port: 8000 }),
jwtSecret?: string
) {
super(address)
super(new HttpAddress(address))

if (jwtSecret === undefined) {
jwtSecret = crypto.randomBytes(16).toString('hex')
Expand Down Expand Up @@ -117,7 +123,12 @@ export class HttpServer extends TcpServer {
app.post('/', async (request, reply) => {
reply.header('Content-Type', 'application/json')
const { body, user } = request
if (typeof body === 'object' && body !== null && 'jsonrpc' in body) {
if (
typeof body === 'object' &&
body !== null &&
'jsonrpc' in body &&
'id' in body
) {
const jsonRpcRequest = body as JsonRpcRequest
const claims = typeof user === 'object' ? user : {}
const jsonRpcResponse = await this.receive(
Expand All @@ -127,7 +138,7 @@ export class HttpServer extends TcpServer {
)
reply.send(jsonRpcResponse)
} else {
reply.status(400).send('Request body must be a JSON-RPC request')
reply.status(400).send('Request body must be a valid JSON-RPC request')
}
})

Expand Down Expand Up @@ -158,7 +169,7 @@ export class HttpServer extends TcpServer {
.send(error)
} else {
// Send bare result
// To to fastify's (change in) handling of strings it is necessary
// Due to Fastify's (change in) handling of strings it is necessary
// to stringify them before sending.
// See https://github.com/stencila/executa/pull/95#issuecomment-591054049
reply.send(typeof result === 'string' ? `"${result}"` : result)
Expand Down Expand Up @@ -257,7 +268,7 @@ export class HttpServer extends TcpServer {
public async start(executor: Executor): Promise<void> {
this.executor = executor

log.debug(`Starting server: ${this.address.url()}`)
log.debug(`Starting HTTP server: ${this.address.url()}`)
const app = (this.app = await this.buildApp())

// Wait for plugins to be ready
Expand Down
33 changes: 33 additions & 0 deletions src/ws/WebSocketServer.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import { WebSocketAddress } from '../base/Transports'
import { WebSocketServer } from './WebSocketServer'

describe('WebSocketServer.address.url', () => {
test('default', () => {
expect(new WebSocketServer().address.url()).toBe('ws://127.0.0.1:9000')
})

test('string', () => {
expect(new WebSocketServer('9001').address.url()).toBe(
'ws://127.0.0.1:9001'
)
})

test('integer', () => {
expect(new WebSocketServer(9002).address.url()).toBe('ws://127.0.0.1:9002')
})

test('object', () => {
expect(
new WebSocketServer({
host: '192.0.0.1',
port: 9003,
}).address.url()
).toBe('ws://192.0.0.1:9003')
})

test('WebSocketAddress', () => {
expect(new WebSocketServer(new WebSocketAddress(9004)).address.url()).toBe(
'ws://127.0.0.1:9004'
)
})
})
2 changes: 1 addition & 1 deletion src/ws/WebSocketServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ export class WebSocketServer extends TcpServer {
address: WebSocketAddressInitializer = new WebSocketAddress({ port: 9000 }),
jwtSecret?: string
) {
super(address)
super(new WebSocketAddress(address))

if (jwtSecret === undefined) {
jwtSecret = crypto.randomBytes(16).toString('hex')
Expand Down

0 comments on commit e747f0f

Please sign in to comment.