Skip to content

Commit

Permalink
Fix URL configuration with the default HTTP/HTTPS ports (#259)
Browse files Browse the repository at this point in the history
  • Loading branch information
slvrtrn authored Apr 2, 2024
1 parent 64b5f81 commit 720ca1b
Show file tree
Hide file tree
Showing 6 changed files with 70 additions and 8 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
# 1.0.1 (Common, Node.js, Web)

## Bug fixes

- Fixed the regression where the default HTTP/HTTPS port numbers (80/443) could not be used with the URL configuration ([#258](https://github.com/ClickHouse/clickhouse-js/issues/258)).

# 1.0.0 (Common, Node.js, Web)

Formal stable release milestone with a lot of improvements and some [breaking changes](#breaking-changes-in-100).
Expand Down
63 changes: 61 additions & 2 deletions packages/client-common/__tests__/unit/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,63 @@ describe('config', () => {
expect(res).toEqual(defaultConfig)
})

it('should fall back to default HTTP/HTTPS port numbers', async () => {
expect(
prepareConfigWithURL({ url: 'http://localhost:80' }, logger, null),
).toEqual({
url: new URL('http://localhost/'), // default HTTP port 80 is omitted
})
expect(
prepareConfigWithURL({ url: 'https://localhost:443' }, logger, null),
).toEqual({
url: new URL('https://localhost/'), // default HTTPS port 443 is omitted
})
})

it('should use non-default HTTP/HTTPS port numbers', async () => {
const sampleValidPorts = ['1', '65535', '8123', '8080', '8443']
for (const protocol of ['http', 'https']) {
for (const port of sampleValidPorts) {
expect(
prepareConfigWithURL(
{ url: `${protocol}://localhost:${port}` },
logger,
null,
),
)
.withContext(`${protocol} with valid port ${port} should not throw`)
.toEqual({
url: new URL(`${protocol}://localhost:${port}/`),
})
}
}
})

it('should throw when the HTTP/HTTPS port is not valid', async () => {
const invalidPorts = ['foo', '65536', '-1']
for (const protocol of ['http', 'https']) {
for (const port of invalidPorts) {
expect(() =>
prepareConfigWithURL(
{ url: `${protocol}://localhost:${port}` },
logger,
null,
),
)
.withContext(
`${protocol} with invalid port ${port} is expected to throw`,
)
.toThrow(
jasmine.objectContaining({
message: jasmine.stringContaining(
'ClickHouse URL is malformed',
),
}),
)
}
}
})

it('should set everything, overriding the defaults', async () => {
const res = prepareConfigWithURL(
{
Expand Down Expand Up @@ -555,8 +612,10 @@ describe('config', () => {
message: jasmine.stringContaining('ClickHouse URL is malformed.'),
}),
)
expect(() => createUrl('http://localhost/foo')).toThrowError(
'ClickHouse URL must contain a valid port number.',
expect(() => createUrl('http://localhost:foo')).toThrow(
jasmine.objectContaining({
message: jasmine.stringContaining('ClickHouse URL is malformed.'),
}),
)
expect(() => createUrl('tcp://localhost:8443')).toThrowError(
'ClickHouse URL protocol must be either http or https. Got: tcp:',
Expand Down
3 changes: 0 additions & 3 deletions packages/client-common/src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -283,9 +283,6 @@ export function createUrl(configURL: string | URL | undefined): URL {
`ClickHouse URL protocol must be either http or https. Got: ${url.protocol}`,
)
}
if (url.port === '' || isNaN(Number(url.port))) {
throw new Error('ClickHouse URL must contain a valid port number.')
}
return url
}

Expand Down
2 changes: 1 addition & 1 deletion packages/client-common/src/version.ts
Original file line number Diff line number Diff line change
@@ -1 +1 @@
export default '1.0.0'
export default '1.0.1'
2 changes: 1 addition & 1 deletion packages/client-node/src/version.ts
Original file line number Diff line number Diff line change
@@ -1 +1 @@
export default '1.0.0'
export default '1.0.1'
2 changes: 1 addition & 1 deletion packages/client-web/src/version.ts
Original file line number Diff line number Diff line change
@@ -1 +1 @@
export default '1.0.0'
export default '1.0.1'

0 comments on commit 720ca1b

Please sign in to comment.