Skip to content

Commit

Permalink
fix: coerce port to number in ConnectionOptionsEnvReader (#6786)
Browse files Browse the repository at this point in the history
the expected type of `port` in all drivers is a number - and
in MSSQL this is a problem as the underlying driver does not
properly handle a string port - so we have to parseInt

closes #6781
  • Loading branch information
imnotjames committed Sep 26, 2020
1 parent b55a417 commit 55fbb69
Showing 1 changed file with 11 additions and 1 deletion.
12 changes: 11 additions & 1 deletion src/connection/options-reader/ConnectionOptionsEnvReader.ts
Expand Up @@ -21,7 +21,7 @@ export class ConnectionOptionsEnvReader {
type: PlatformTools.getEnvVariable("TYPEORM_CONNECTION") || (PlatformTools.getEnvVariable("TYPEORM_URL") ? PlatformTools.getEnvVariable("TYPEORM_URL").split("://")[0] : undefined),
url: PlatformTools.getEnvVariable("TYPEORM_URL"),
host: PlatformTools.getEnvVariable("TYPEORM_HOST"),
port: PlatformTools.getEnvVariable("TYPEORM_PORT"),
port: this.stringToNumber(PlatformTools.getEnvVariable("TYPEORM_PORT")),
username: PlatformTools.getEnvVariable("TYPEORM_USERNAME"),
password: PlatformTools.getEnvVariable("TYPEORM_PASSWORD"),
database: PlatformTools.getEnvVariable("TYPEORM_DATABASE"),
Expand Down Expand Up @@ -95,4 +95,14 @@ export class ConnectionOptionsEnvReader {
return variable.split(",").map(str => str.trim());
}

/**
* Converts a string which contains a number into a javascript number
*/
private stringToNumber(value: any): number|undefined {
if (!value) {
return undefined;
}

return parseInt(value);
}
}

4 comments on commit 55fbb69

@tododo
Copy link

@tododo tododo commented on 55fbb69 Sep 28, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As shown in the below compiled code, seems issue still exists? @imnotjames

port: PlatformTools_1.PlatformTools.getEnvVariable("TYPEORM_PORT"),

@imnotjames
Copy link
Contributor Author

@imnotjames imnotjames commented on 55fbb69 Sep 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where are you seeing that?

At this time this code hasn't been released yet.

@tododo
Copy link

@tododo tododo commented on 55fbb69 Sep 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh Right, @imnotjames sorry. I am seeing it in 0.2.6. I am currently bypassing it by removing the port from .env.

Do you know when is this going to be released?

@imnotjames
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No worries! I was just worried I somehow messed something up :)

I don't know if there's an exact timeline for another release. I think the goal is within the next few weeks

Please sign in to comment.