Add support for customized database name with --using #290

apoorvdeshmukh · 2023-03-08T11:19:24Z

This commit adds support for specifying database name with --using option by specifying ,DbName at the end of the URL
This PR closes #216

Example without specifying customized database name, in which case Dbname defaults to the filename without extension

\git\go-sqlcmd>sqlcmd create mssql --accept-eula --using https://aka.ms/AdventureWorksLT.bak
Downloading mcr.microsoft.com/mssql/server:latest
Starting mcr.microsoft.com/mssql/server:latest
Created context "mssql" in "C:\Users\apdeshmukh\.sqlcmd\sqlconfig", configuring user account...
Disabled "sa" account (and rotated "sa" password). Creating user "apdeshmukh"Downloading AdventureWorksLT.bak from aka.ms
Restoring database AdventureWorksLT
Processed 840 pages for database 'AdventureWorksLT', file 'AdventureWorksLT2012_Data' on file 1.
Processed 2 pages for database 'AdventureWorksLT', file 'AdventureWorksLT2012_Log' on file 1.
Converting database 'AdventureWorksLT' from version 904 to the current version 957.
Database 'AdventureWorksLT' running the upgrade step from version 904 to version 905.
Database 'AdventureWorksLT' running the upgrade step from version 955 to version 956.
Database 'AdventureWorksLT' running the upgrade step from version 956 to version 957.
RESTORE DATABASE successfully processed 842 pages in 0.038 seconds (173.005 MB/sec).
Now ready for client connections on port 1435HINT:
  1. Open in Azure Data Studio: sqlcmd open ads
  2. Run a query:               sqlcmd query "SELECT @@version"  3. Start interactive session: sqlcmd query
  4. View sqlcmd configuration: sqlcmd config view
  5. See connection strings:    sqlcmd config connection-strings
  6. Remove:                    sqlcmd delete
\git\go-sqlcmd>.\sqlcmd.exe query
1> SELECT * FROM INFORMATION_SCHEMA.TABLES WHERE TABLE_TYPE='BASE TABLE'
2> go
TABLE_CATALOG                                                                                                                    TABLE_SCHEMA                                                                                                                     TABLE_NAME
                                                                                                           TABLE_TYPE
-------------------------------------------------------------------------------------------------------------------------------- -------------------------------------------------------------------------------------------------------------------------------- -------------------------------------------------------------------------------------------------------------------------------- ----------AdventureWorksLT                                                                                                                 dbo                                                                                                                              ErrorLog
                                                                                                           BASE TABLE
AdventureWorksLT                                                                                                                 dbo                                                                                                                              BuildVersion
                                                                                                           BASE TABLE
AdventureWorksLT                                                                                                                 SalesLT                                                                                                                          Address
                                                                                                           BASE TABLE
AdventureWorksLT                                                                                                                 SalesLT                                                                                                                          Customer
                                                                                                           BASE TABLE
AdventureWorksLT                                                                                                                 SalesLT                                                                                                                          CustomerAddress                                                                                                                  BASE TABLE
AdventureWorksLT                                                                                                                 SalesLT                                                                                                                          Product
                                                                                                           BASE TABLE
AdventureWorksLT                                                                                                                 SalesLT                                                                                                                          ProductCategory                                                                                                                  BASE TABLE
AdventureWorksLT                                                                                                                 SalesLT                                                                                                                          ProductDescription                                                                                                               BASE TABLE
AdventureWorksLT                                                                                                                 SalesLT                                                                                                                          ProductModel
                                                                                                           BASE TABLE
AdventureWorksLT                                                                                                                 SalesLT                                                                                                                          ProductModelProductDescription                                                                                                   BASE TABLE
AdventureWorksLT                                                                                                                 SalesLT                                                                                                                          SalesOrderDetail                                                                                                                 BASE TABLE
AdventureWorksLT                                                                                                                 SalesLT                                                                                                                          SalesOrderHeader                                                                                                                 BASE TABLE
(12 rows affected)

Example with customized database name, in which case Dbname is set to user provided value

\git\go-sqlcmd>sqlcmd create mssql --accept-eula --using https://aka.ms/AdventureWorksLT.bak,WorldWideImporters
Downloading mcr.microsoft.com/mssql/server:latest
Starting mcr.microsoft.com/mssql/server:latest
Created context "mssql" in "C:\Users\apdeshmukh\.sqlcmd\sqlconfig", configuring user account...
Disabled "sa" account (and rotated "sa" password). Creating user "apdeshmukh"Downloading AdventureWorksLT.bak from aka.ms
Restoring database WorldWideImporters
Processed 840 pages for database 'WorldWideImporters', file 'AdventureWorksLT2012_Data' on file 1.
Processed 2 pages for database 'WorldWideImporters', file 'AdventureWorksLT2012_Log' on file 1.
Converting database 'WorldWideImporters' from version 904 to the current version 957.
Database 'WorldWideImporters' running the upgrade step from version 904 to version 905.
Database 'WorldWideImporters' running the upgrade step from version 954 to version 955.
Database 'WorldWideImporters' running the upgrade step from version 955 to version 956.
Database 'WorldWideImporters' running the upgrade step from version 956 to version 957.
RESTORE DATABASE successfully processed 842 pages in 0.038 seconds (173.005 MB/sec).
Now ready for client connections on port 1435HINT:
  1. Open in Azure Data Studio: sqlcmd open ads
  2. Run a query:               sqlcmd query "SELECT @@version"  3. Start interactive session: sqlcmd query
  4. View sqlcmd configuration: sqlcmd config view
  5. See connection strings:    sqlcmd config connection-strings
  6. Remove:                    sqlcmd delete
\git\go-sqlcmd>.\sqlcmd.exe query
1> select db_name()
2> go
--------------------------------------------------------------------------------------------------------------------------------WorldWideImporters
(1 row affected)
1> SELECT * FROM INFORMATION_SCHEMA.TABLES WHERE TABLE_TYPE='BASE TABLE'
2> go
TABLE_CATALOG                                                                                                                    TABLE_SCHEMA                                                                                                                     TABLE_NAME                                                                                                                       TABLE_TYPE
-------------------------------------------------------------------------------------------------------------------------------- -------------------------------------------------------------------------------------------------------------------------------- -------------------------------------------------------------------------------------------------------------------------------- ----------WorldWideImporters                                                                                                               dbo                                                                                                                              ErrorLog                                                                                                                         BASE TABLE
WorldWideImporters                                                                                                               dbo                                                                                                                              BuildVersion                                                                                                                     BASE TABLE
WorldWideImporters                                                                                                               SalesLT                                                                                                                          Address                                                                                                                          BASE TABLE
WorldWideImporters                                                                                                               SalesLT                                                                                                                          Customer                                                                                                                         BASE TABLE
WorldWideImporters                                                                                                               SalesLT                                                                                                                          CustomerAddress                                                                                                                  BASE TABLE
WorldWideImporters                                                                                                               SalesLT                                                                                                                          Product                                                                                                                          BASE TABLE
WorldWideImporters                                                                                                               SalesLT                                                                                                                          ProductCategory                                                                                                                  BASE TABLE
WorldWideImporters                                                                                                               SalesLT                                                                                                                          ProductDescription                                                                                                               BASE TABLE
WorldWideImporters                                                                                                               SalesLT                                                                                                                          ProductModel                                                                                                                     BASE TABLE
WorldWideImporters                                                                                                               SalesLT                                                                                                                          ProductModelProductDescription                                                                                                   BASE TABLE
WorldWideImporters                                                                                                               SalesLT                                                                                                                          SalesOrderDetail                                                                                                                 BASE TABLE
WorldWideImporters                                                                                                               SalesLT                                                                                                                          SalesOrderHeader                                                                                                                 BASE TABLE
(12 rows affected)

cmd/modern/root/install/mssql-base.go

shueybubbles · 2023-03-08T15:55:51Z

please add tests. There's a pretty big matrix of input to cover here and the tests need to show what's valid and what's not. We need to handle edge cases that could lead to sql injection if we don't escape the name, etc.
Can I pass a local file path like --using "c:\path with a space and a comm,a\path","db space and co,mma" ?

shueybubbles

🕐

shueybubbles · 2023-03-08T16:00:25Z

do we want to allow the user to provide the db name as a bracketed identifier?
eg

--using https://someurl/somefile.bak,[my database]

In reply to: 1460392606

shueybubbles · 2023-03-08T16:03:15Z

if there's a space in the name or the path will the user have to surround the entire parameter with quotes?

In reply to: 1460405555

cmd/modern/root/install/mssql-base.go

stuartpa · 2023-03-09T11:03:13Z

Yes, add an example with --using with a space in the db name (it works, we tested it)

In reply to: 1460405555

shueybubbles · 2023-03-09T15:16:26Z

  String:        &c.usingDatabaseUrl,

Is there a way we could change this flag type to a StringSlice and restrict it to only being used once? That could let cobra do the heavy lifting of delimiting the components by the comma, and our code could potentially allow more parameters in the future.

I think the default behavior of StringSlice is to allow the same flag to be used multiple times like

-u f1,f2 -u f3

#Closed

Refers to: cmd/modern/root/install/mssql-base.go:192 in 246bd0e. [](commit_id = 246bd0e, deletion_comment = False)

apoorvdeshmukh · 2023-03-09T15:27:19Z

please add tests. There's a pretty big matrix of input to cover here and the tests need to show what's valid and what's not. We need to handle edge cases that could lead to sql injection if we don't escape the name, etc. Can I pass a local file path like --using "c:\path with a space and a comm,a\path","db space and co,mma" ?

We do not support filesystem paths at the moment. Only http and https. But I will add tests to check what happens if such input is intentionally passed.

apoorvdeshmukh · 2023-03-09T15:30:41Z

do we want to allow the user to provide the db name as a bracketed identifier? eg
--using https://someurl/somefile.bak,[my database]
In reply to: 1460392606

I checked with @stuartpa and for now we will only support URL,DbName where ,DbName is optional.
If there are spaces or commas in the DbName then we would expect user to surround the entire argument of --using with ""
E.g. --using "https://aka.ms/AdventureWorksLT.bak,My Db"

shueybubbles · 2023-03-09T17:20:05Z

per our meeting - make sure you bracket-escape the name before putting it in a SQL query
"https:/server/file.bak,db [name]" ==> [db [name]]]

see https://github.com/microsoft/sqlmanagementobjects/blob/bf460b75d413fb3d366aa59fc5d48f3c01cccdd5/src/Microsoft/SqlServer/Management/Smo/SqlSmoObject.cs#L3494

In reply to: 1461812698

shueybubbles · 2023-03-10T16:51:09Z

SET @Sql = 'RESTORE DATABASE [%s] FROM DISK = ''%s/%s'' WITH '

if databaseName has a ] in it this query becomes a vector for sql injection.
Similarly if any of the other parameters have a ' in their value this query will go awry.

Either use a properly parameterized query (go-mssqldb supports parameterized queries) or escape everything appropriately.
It's even more complicated here because it's using dynamic SQL. Any values you are adding that have a ' in them may need to be double escaped.
Look at the SMO code for guidance/examples.

Refers to: cmd/modern/root/install/mssql-base.go:530 in 7a37b69. [](commit_id = 7a37b69, deletion_comment = False)

stuartpa · 2023-03-11T10:25:07Z

We want to incrementally improve the quality of the experience here. There is a quality issue with the experience right now, in that folks don't want to be stuck with the name of the .bak file as the database name. It would improve the experience if we allow a database name to be specified. In the limit we should support a database name that covers the entire input space for T/SQL identifiers, but we don't have to do that in a single PR. It is fine in this PR to limit the input space for the database name so SQL injection is not possible, and enter an issue to widen the input space. What we don't want to see is we end up reducing quality by not thinking through the complete solution for all T/SQL identifier input space in one PR.

cmd/modern/root/install/mssql.go

cmd/modern/root/install/mssql-base_test.go

shueybubbles · 2023-03-13T14:47:59Z

We should be able to write a functional test for this in the ADO pipeline. ADO hosted agents have docker on them, so the pipeline should be able to use the sqlcmd it builds to run some permutations on sqlcmd create --using and verify the E2E scenario.
The unit test is insufficient to prove we are doing the right thing with the user input all the way through the download and rename process.

In reply to: 1462251401

cmd/modern/root/install/mssql-base.go

apoorvdeshmukh · 2023-03-14T12:12:47Z

We should be able to write a functional test for this in the ADO pipeline. ADO hosted agents have docker on them, so the pipeline should be able to use the sqlcmd it builds to run some permutations on sqlcmd create --using and verify the E2E scenario. The unit test is insufficient to prove we are doing the right thing with the user input all the way through the download and rename process.

In reply to: 1462251401

Created #306 to add functional tests for E2E testing scenarios

cmd/modern/root/install/mssql-base_test.go

cmd/modern/root/install/mssql-base.go

shueybubbles

Add support for customized database name with --using

866cc09

apoorvdeshmukh requested a review from stuartpa March 8, 2023 11:19

stuartpa reviewed Mar 8, 2023

View reviewed changes

cmd/modern/root/install/mssql-base.go Outdated Show resolved Hide resolved

apoorvdeshmukh added 2 commits March 8, 2023 17:00

Fix function name typo

676a0e3

Merge remote-tracking branch 'origin/main' into apoorvdeshmukh/216

246bd0e

shueybubbles requested changes Mar 8, 2023

View reviewed changes

stuartpa reviewed Mar 9, 2023

View reviewed changes

cmd/modern/root/install/mssql-base.go Outdated Show resolved Hide resolved

JyotikaGargg self-requested a review March 10, 2023 10:37

apoorvdeshmukh added 2 commits March 11, 2023 16:12

Add basic test scenarios

4db0535

Merge remote-tracking branch 'origin/main' into apoorvdeshmukh/216

c48b2b9

apoorvdeshmukh force-pushed the apoorvdeshmukh/216 branch from 7a37b69 to c48b2b9 Compare March 11, 2023 10:54

stuartpa reviewed Mar 13, 2023

View reviewed changes

cmd/modern/root/install/mssql.go Outdated Show resolved Hide resolved

Modify dbname in example

c00a8e9

shueybubbles reviewed Mar 13, 2023

View reviewed changes

cmd/modern/root/install/mssql-base_test.go Outdated Show resolved Hide resolved

shueybubbles reviewed Mar 13, 2023

View reviewed changes

cmd/modern/root/install/mssql-base_test.go Outdated Show resolved Hide resolved

shueybubbles reviewed Mar 13, 2023

View reviewed changes

cmd/modern/root/install/mssql-base.go Outdated Show resolved Hide resolved

shueybubbles reviewed Mar 13, 2023

View reviewed changes

cmd/modern/root/install/mssql-base.go Show resolved Hide resolved

stuartpa mentioned this pull request Mar 13, 2023

User's database default is not master or tempdb #284

Closed

Display user provided database name as is

608382c

Merge remote-tracking branch 'origin/main' into apoorvdeshmukh/216

e8ff6c9

apoorvdeshmukh added 2 commits March 16, 2023 19:27

Rework URL and dbName parsing

58ca399

Parse dbName only if url path is not empty

06421e1

apoorvdeshmukh marked this pull request as draft March 16, 2023 14:16

shueybubbles reviewed Mar 16, 2023

View reviewed changes

cmd/modern/root/install/mssql-base_test.go Show resolved Hide resolved

shueybubbles reviewed Mar 16, 2023

View reviewed changes

cmd/modern/root/install/mssql-base.go Outdated Show resolved Hide resolved

shueybubbles reviewed Mar 16, 2023

View reviewed changes

cmd/modern/root/install/mssql-base.go Show resolved Hide resolved

shueybubbles reviewed Mar 16, 2023

View reviewed changes

cmd/modern/root/install/mssql-base.go Show resolved Hide resolved

Address review comments

bddac70

apoorvdeshmukh marked this pull request as ready for review March 17, 2023 11:48

shueybubbles reviewed Mar 17, 2023

View reviewed changes

cmd/modern/root/install/mssql-base.go Outdated Show resolved Hide resolved

shueybubbles reviewed Mar 17, 2023

View reviewed changes

cmd/modern/root/install/mssql-base.go Outdated Show resolved Hide resolved

apoorvdeshmukh added 2 commits March 20, 2023 14:16

Merge remote-tracking branch 'origin/main' into apoorvdeshmukh/216

be79abd

Escape dbname separately as identifier and non-identifier

48dc082

shueybubbles reviewed Mar 20, 2023

View reviewed changes

cmd/modern/root/install/mssql-base.go Show resolved Hide resolved

shueybubbles approved these changes Mar 20, 2023

View reviewed changes

apoorvdeshmukh mentioned this pull request Mar 20, 2023

Add functional tests for sqlcmd (verify all types of public and SAS URLs work well in --using) #306

Open

apoorvdeshmukh added 2 commits March 20, 2023 22:56

Return URL as is if no databasename specified

3354aca

Add unit test for extractUrl

d972500

apoorvdeshmukh merged commit 9780801 into main Mar 20, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for customized database name with --using #290

Add support for customized database name with --using #290

apoorvdeshmukh commented Mar 8, 2023 •

edited

Loading

shueybubbles commented Mar 8, 2023

shueybubbles left a comment

shueybubbles commented Mar 8, 2023 •

edited

Loading

shueybubbles commented Mar 8, 2023

stuartpa commented Mar 9, 2023

shueybubbles commented Mar 9, 2023 •

edited

Loading

apoorvdeshmukh commented Mar 9, 2023

apoorvdeshmukh commented Mar 9, 2023

shueybubbles commented Mar 9, 2023

shueybubbles commented Mar 10, 2023 •

edited

Loading

stuartpa commented Mar 11, 2023

shueybubbles commented Mar 13, 2023

apoorvdeshmukh commented Mar 14, 2023

shueybubbles left a comment

Add support for customized database name with --using #290

Add support for customized database name with --using #290

Conversation

apoorvdeshmukh commented Mar 8, 2023 • edited Loading

shueybubbles commented Mar 8, 2023

shueybubbles left a comment

Choose a reason for hiding this comment

shueybubbles commented Mar 8, 2023 • edited Loading

shueybubbles commented Mar 8, 2023

stuartpa commented Mar 9, 2023

shueybubbles commented Mar 9, 2023 • edited Loading

apoorvdeshmukh commented Mar 9, 2023

apoorvdeshmukh commented Mar 9, 2023

shueybubbles commented Mar 9, 2023

shueybubbles commented Mar 10, 2023 • edited Loading

stuartpa commented Mar 11, 2023

shueybubbles commented Mar 13, 2023

apoorvdeshmukh commented Mar 14, 2023

shueybubbles left a comment

Choose a reason for hiding this comment

apoorvdeshmukh commented Mar 8, 2023 •

edited

Loading

shueybubbles commented Mar 8, 2023 •

edited

Loading

shueybubbles commented Mar 9, 2023 •

edited

Loading

shueybubbles commented Mar 10, 2023 •

edited

Loading