Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Port used for MSSQL databases #11633

Closed
2 of 7 tasks
jeffest opened this issue May 26, 2020 · 3 comments · Fixed by #11642
Closed
2 of 7 tasks

Port used for MSSQL databases #11633

jeffest opened this issue May 26, 2020 · 3 comments · Fixed by #11642

Comments

@jeffest
Copy link

jeffest commented May 26, 2020

  • Gitea version (or commit ref): 1.12
  • Git version: 2.22
  • Operating system: Windows 10
  • Database (use [x]):
    • PostgreSQL
    • MySQL
    • MSSQL
    • SQLite
  • Can you reproduce the bug at https://try.gitea.io:
    • Yes (provide example URL)
    • No
    • Not relevant
  • Log gist:

Description

The installation process assumes that the MSSQL server instance provided by the user will listen on the default port 1433. This is defined in function ParseMSSQLHostPort.
By doing so, it prevents the mssql driver from actually detecting the port automatically. This has a major impact if the db server is hosting multiple instances of SQL Server. The port 1433 is being used by the default instance and all other instances will use dynamic ports. In order to determine the correct port for a particular instance (the one supplied by the end user), the driver is querying the SQL Server's Browser service (here). But it only does that if the port is not specified ( p.port == 0 ) which never happens considering the default value of 1433.

IMHO, there should not be a default port for mssql driver if not provided by the user and the driver should be responsible for finding it.

@jolheiser
Copy link
Member

This is the install page regarding MSSQL options:

mssql

And then after installation, when the config is loaded, the host would be parsed like so:

// ParseMSSQLHostPort splits the host into host and port
func ParseMSSQLHostPort(info string) (string, string) {
host, port := "127.0.0.1", "1433"
if strings.Contains(info, ":") {
host = strings.Split(info, ":")[0]
port = strings.Split(info, ":")[1]
} else if strings.Contains(info, ",") {
host = strings.Split(info, ",")[0]
port = strings.TrimSpace(strings.Split(info, ",")[1])
} else if len(info) > 0 {
host = info
}
return host, port
}

Port 1433 does serve as default, but only if none of the following if-else match. If you set it as 127.0.0.1:0 then it would parse the port as 0 afaik

@zeripath
Copy link
Contributor

But if we change the port in line 167 to 0 then they never have to put the port in and, it will still work for those who have it on 1433... (AFAICS)

@jeffest
Copy link
Author

jeffest commented May 27, 2020

Yes @zeripath, this is how it should work.
@jolheiser, your remark is 100% valid but it's neither intuitive or user-friendly (AFAICT)

zeripath added a commit to zeripath/gitea that referenced this issue May 27, 2020
Fix go-gitea#11633

Signed-off-by: Andrew Thornton <art27@cantab.net>
techknowlogick added a commit that referenced this issue May 29, 2020
Fix #11633

Signed-off-by: Andrew Thornton <art27@cantab.net>

Co-authored-by: techknowlogick <techknowlogick@gitea.io>
zeripath added a commit to zeripath/gitea that referenced this issue May 29, 2020
lafriks pushed a commit that referenced this issue May 29, 2020
…#11673)

Backport #11642

Fix #11633

Signed-off-by: Andrew Thornton <art27@cantab.net>
ydelafollye pushed a commit to ydelafollye/gitea that referenced this issue Jul 31, 2020
…a#11642)

Fix go-gitea#11633

Signed-off-by: Andrew Thornton <art27@cantab.net>

Co-authored-by: techknowlogick <techknowlogick@gitea.io>
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants