views:

206

answers:

6

Basically i have a query string that when i hardcode in the catalogue value its fine. when I try adding it via a variable it just doesn't pick it up.

This works:

  Dim WaspConnection As New SqlConnection("Data Source=JURA;Initial Catalog=WaspTrackAsset_NROI;User id=" & ConfigurationManager.AppSettings("WASPDBUserName") & ";Password='" & ConfigurationManager.AppSettings("WASPDBPassword").ToString & "';")

This doesn't:

Public Sub GetWASPAcr()

    connection.Open()

    Dim dt As New DataTable()
    Dim username As String = HttpContext.Current.User.Identity.Name
    Dim sqlCmd As New SqlCommand("SELECT WASPDatabase FROM dbo.aspnet_Users WHERE UserName = '" & username & "'", connection)

    Dim sqlDa As New SqlDataAdapter(sqlCmd)

    sqlDa.Fill(dt)

    If dt.Rows.Count > 0 Then

        For i As Integer = 0 To dt.Rows.Count - 1
            If dt.Rows(i)("WASPDatabase") Is DBNull.Value Then
                WASP = ""
            Else
                WASP = "WaspTrackAsset_" + dt.Rows(i)("WASPDatabase")
            End If

        Next

    End If
    connection.Close()

End Sub

Dim WaspConnection As New SqlConnection("Data Source=JURA;Initial Catalog=" & WASP & ";User id=" & ConfigurationManager.AppSettings("WASPDBUserName") & ";Password='" & ConfigurationManager.AppSettings("WASPDBPassword").ToString & "';")

When I debug the catalog is empty in the query string but the WASP variable holds the value "WaspTrackAsset_NROI"

Any idea's why?

Cheers,

jonesy

alt text

+1  A: 

Your string passed into the constructor for this SqlConnection object will be evaluated when the class is instantiated. Your WASP variable (I'm assuming) won't be set until the method you have shown is called.

Paddy
A: 

Might want to quit looking one you have found your database:

For i As Integer = 0 To dt.Rows.Count - 1
            If dt.Rows(i)("WASPDatabase") Is DBNull.Value Then
                WASP = ""
            Else
                WASP = "WaspTrackAsset_" + dt.Rows(i)("WASPDatabase")
                break
            End If

        Next
edosoft
can't use break (says not declared) will this solve my problem?
iamjonesy
I'm sorry, break is the C#, the VB syntax is Exit For. And I don't know if it will solve your problem, it might
edosoft
A: 

[link text][1]You are building your string on the fly by adding the value of a column to a string. So, for the row in question for the column "WASPDatabase" was tacked on to your string. So you got what it had. On another note, your earlier query of "select ... from ... where ..." where you are manually concatinating the string of a variable makes you WIDE OPEN to SQL-Injection attacks.

Although this link [1]: http://stackoverflow.com/questions/2675610/how-to-update-a-table-using-oledb-parameters/2675922#2675922 "Sample query using parameterization" is to a C# sample of querying with parameterized values, the similar principles apply to most all SQL databases.

DRapp
Correct me if I'm wrong but there is no user interaction going on here so how can it be open to a injection?
iamjonesy
In this specific case based on you getting the user name from an environment based setting, no, but if you are starting like that, just head caution for any data-entry based values that could get injected.
DRapp
A: 

At the time you're creating the new connection, WASP is holding the value you want it to be holding? It is a string data type? Try adding .ToString after WASP and see if that helps anything.

Interesting problem. =-)

Tychumsempir
A: 

The problem is, as Paddy already points out, that the WaspConnection object gets initialized before you even have the chance to call GetWASPAcr. Try this:

Public Sub GetWASPAcr()
   '[...]

End Sub

Dim _waspConnection As SqlConnection
Public Readonly Property WaspConnection As SqlConnection
  Get
    If _waspConnection Is Nothing Then
      GetWASPAcr()
      _waspConnection = New SqlConnection("Data Source=JURA;Initial Catalog=" & WASP & ";User id=" & ConfigurationManager.AppSettings("WASPDBUserName") & ";Password='" & ConfigurationManager.AppSettings("WASPDBPassword").ToString & "';")
    End If
    Return _waspConnection
  End Get
End Property
Jan Willem B
+4  A: 

I can see a few problems.

  1. You are using concatenation in a SQL statement. This is a bad practice. Use a parameterized query instead.
  2. You are surrounding the password with single quotes. They are not needed and in fact, I'm surprised it even works assuming the password itself does not have single quotes.
  3. You should surround classes that implement IDisposable with a Using block
  4. You should recreate the WASP connection object in GetWASPcr like so:
Public Sub GetWASPAcr()
    Dim username As String = HttpContext.Current.User.Identity.Name
    Dim listOfDatabaseConnectionString As String = "..."

    Using listOfDatabaseConnection As SqlConnection( listOfDatabaseConnectionString )
        Using cmd As New SqlCommand("SELECT WASPDatabase FROM dbo.aspnet_Users WHERE UserName = @Username")
            cmd.Parameters.AddWithValue( "@Username", username )

            Dim dt As New DataTable()
            Using da As New SqlDataAdapter( cmd )
                da.Fill( dt )

                If dt.Rows.Count = 0 Then
                    WaspConnection = Null
                Else
                    Dim connString As String = String.Format("Data Source=JURA;Initial Catalog={0};User Id={1};Password={2};" _ 
                        , dt.Rows(0)("WASPDatabase") _ 
                        , ConfigurationManager.AppSettings("WASPDBUserName") _ 
                        , ConfigurationManager.AppSettings("WASPDBPassword"))

                    WaspConnection = New SqlConnection(connString);
                End If  
            End Using
        End Using
    End Using
End Sub

In this example, listOfDatabaseConnectionString is the initial connection string to the central database where it can find the catalog name that should be used for subsequent connections.

All that said, why would you need a class level variable to hold a connection? You should make all your database calls open a connection, do a sql statement, close the connection. So, five database calls would open and close a connection five times. This sounds expensive except that .NET gives you connection pooling so when you finish with a connection and another is requested to be opened, it will pull it from the pool.

Thomas