none
Dynamic SQL - Running From Classic ASP - Quotes Problem

    Question

  • Hi,

    I am having a bit of trouble with a stored procedure that I would like to run on SQL Server 2008. I am calling this stored procedure from a web page that is built using Classic ASP. Hopefully someone can give me some pointers because I'm out of ideas as to where to go next. I have tried lots of other forums and various methods of coding but with no luck. The sproc itself works fine when I run it from SQL Server but it fails when I run it from Classic ASP.

    My stored procedure looks something like this:

    USE [Charlie]
    GO
    /****** Object:  StoredProcedure [dbo].[Test]    Script Date: 08/01/2011 15:07:39 ******/
    SET ANSI_NULLS ON
    GO
    SET QUOTED_IDENTIFIER ON
    GO
    ALTER Proc [dbo].[Test] @SalesSetup varchar(max)
    AS

    DECLARE @col nvarchar(8)
    DECLARE @sql nvarchar(1000)
    SET @col='T439'
    SET @sql = 'SELECT '
    SET @sql = @sql + @col
    SET @sql = @sql + ' FROM INPUT_DATA WHERE '
    SET @sql = @sql + @col
    SET @sql = @sql + ' = '
    SET @sql = @sql + @SalesSetup
    PRINT @sql
    EXEC sp_executesql @sql

    The sproc accesses a database called Charlie and takes in a string held in the variable @SalesSetup. It uses a select statement to pick all the records from a table where the column name is held in the variable @col and the value in that column is the string in the @SalesSetup string. I call this using the following in SQL Server:

    use Charlie
    exec test @SalesSetup= "'Individual Plan - McMaster Tom, 123456'"

    When I run this on SQL Server 2008 it returns the right results (e.g. two records from the table where the data in column T439 has the value 'Individual Plan - McMaster Tom, 123456' in it.

    When I call this from classic ASP however I use the following:

    <%SalesSetup = "'" & SalesSetup & "'"%>

    <%set rsAllUserDeals = conn.execute ("EXEC dbo.Test @SalesSetup="" & SalesSetup & """)%>                                   

    When I use this code I get an error on the asp page saying that there has been a problem on the server. I can't understand why it works on SQL Server but not on ASP.

    As you can probably tell, I think the problem is with the quotes I use. Whenever I used single quotes or change them around it says that the value in the string is an invalid column. This is the first time I've really used Dynamic SQL and the single quotes have driven me mad. I've also tried using quotename but with no luck. Maybe I've been using it wrong (wouldn't surprise me).

    If anyone could shed some light on the best way to solve this problem then I'd really appreciate it. If I've missed anything out that you'd like to know then please let me know.

    Kind regards,

    Tom.

    Monday, August 01, 2011 5:07 PM

Answers

  • Tom,

    If it works from SSMS and not from you ASP application, then I would suggest to post the question in the ASP forum.

    As a good practice, try to parameterize the statement instead concatenating values to the string.

    Using the SQLCommand Object
    http://www.aspfree.com/c/a/Database/Using-the-SQLCommand-Object/

    The Curse and Blessings of Dynamic SQL
    http://www.sommarskog.se/dynamic_sql.html

     


    AMB

    Some guidelines for posting questions...

    Monday, August 01, 2011 5:17 PM
    Moderator
  • So this is an example of bad pratice, and I am not going to spend any time of trying to find out why your code does not work. Instead I will spend my time to tell you how you should have written the code.

    Either you build a stored procedure which accepts a number of parameters, and from this builds the SQL string - as far as dynamic is called for at all - from these parameters and pass the parameters to the SQL string.

    Or you scrap using the stored procedure altogther and build the SQL string entirely in ASP. Still passing parameters as parameters.

    There are several reasons for this, of which I will only mention one: coupling. Your stored procedure is now dependent on the ASP code passing a correct SQL string to the procedure. And your ASP code is dependent on that procedure uses @SalesSetup correctly.

    In your ASP code you have:

    <%set rsAllUserDeals = conn.execute ("EXEC dbo.Test @SalesSetup="" & SalesSetup & """)%>  

    I have already told you that the way your procedure is set up, this is all wrong, but it illustrates another thing I want to highlight. The way this should look in VB6 (I don't know ASP, so I use VB6 for the example):

    Set cmd = CreateObject("ADODB.Command")
    Set cmd.ActiveConnection = conn
    cmd.CommandType = Text
    cmd.CommandText = "EXEC dbo.Test @SalesSetup = ?'
    cmd.Parameterss.Add (@SalesSetup, adChar, adParamInput, -1, SalesSetup)
    Set rs = cmd.Execute

    That is, you should never build SQL Strings by string concatenation, but always pass values as parameters. There are severals reasons for this. One is illustrated by your post - you lose yourself in a maze of nested quotes. Another is protection against SQL injection.

    (Note: actually the above is not how you call a stored procedure from VB, since you should use command type adStoredProc, but I used procedure call as an example of how SQL statements in general should be handled.)


    Erland Sommarskog, SQL Server MVP, esquel@sommarskog.se
    Monday, August 01, 2011 6:29 PM
  • > I deliberately avoided putting my sql strings together in the ASP client code. I've had a read through the documents that Hunchback posted earlier this morning (they seem pretty popular) and it confirmed my worries about SQL injection and security. I built another project in another life that used the ASP code to build the SQL strings and it was untidy and a nightmare. I ended up spending months changing it to use sps.

    But you haven't. What you have now is the worst of two worlds.

    You should aim at writing your stored procedures with static SQL. And call them with adStoredProc. For some type of functions, like search forms where the user can choose between lots of things to search from, dynamic SQL may be inevitable.

    I have an article on my web site that covers dynamic SQL in general: http://www.sommarskog.se/dynamic_sql.html

    If you need dynamic searches, I have a second article dedicated to that topic: http://www.sommarskog.se/dyn-search.html


    Erland Sommarskog, SQL Server MVP, esquel@sommarskog.se
    Monday, August 01, 2011 8:22 PM

All replies

  • Tom,

    If it works from SSMS and not from you ASP application, then I would suggest to post the question in the ASP forum.

    As a good practice, try to parameterize the statement instead concatenating values to the string.

    Using the SQLCommand Object
    http://www.aspfree.com/c/a/Database/Using-the-SQLCommand-Object/

    The Curse and Blessings of Dynamic SQL
    http://www.sommarskog.se/dynamic_sql.html

     


    AMB

    Some guidelines for posting questions...

    Monday, August 01, 2011 5:17 PM
    Moderator
  • 1. If you set the column name in SP explicitly, then why do you use dynamic SQL at all? In any case, since you're passing the parameter's value, you need to change the bottom of your SP to (BTW, is your field varchar(max))? If not, use the correct length of the field:

    SET @sql = @sql + ' = @SalesSetup'
    PRINT @sql
    EXEC sp_executesql @sql, N'@SalesSetup varchar(max)', @SalesSetup
    



    For every expert, there is an equal and opposite expert. - Becker's Law


    My blog
    Monday, August 01, 2011 5:21 PM
    Moderator
  • So this is an example of bad pratice, and I am not going to spend any time of trying to find out why your code does not work. Instead I will spend my time to tell you how you should have written the code.

    Either you build a stored procedure which accepts a number of parameters, and from this builds the SQL string - as far as dynamic is called for at all - from these parameters and pass the parameters to the SQL string.

    Or you scrap using the stored procedure altogther and build the SQL string entirely in ASP. Still passing parameters as parameters.

    There are several reasons for this, of which I will only mention one: coupling. Your stored procedure is now dependent on the ASP code passing a correct SQL string to the procedure. And your ASP code is dependent on that procedure uses @SalesSetup correctly.

    In your ASP code you have:

    <%set rsAllUserDeals = conn.execute ("EXEC dbo.Test @SalesSetup="" & SalesSetup & """)%>  

    I have already told you that the way your procedure is set up, this is all wrong, but it illustrates another thing I want to highlight. The way this should look in VB6 (I don't know ASP, so I use VB6 for the example):

    Set cmd = CreateObject("ADODB.Command")
    Set cmd.ActiveConnection = conn
    cmd.CommandType = Text
    cmd.CommandText = "EXEC dbo.Test @SalesSetup = ?'
    cmd.Parameterss.Add (@SalesSetup, adChar, adParamInput, -1, SalesSetup)
    Set rs = cmd.Execute

    That is, you should never build SQL Strings by string concatenation, but always pass values as parameters. There are severals reasons for this. One is illustrated by your post - you lose yourself in a maze of nested quotes. Another is protection against SQL injection.

    (Note: actually the above is not how you call a stored procedure from VB, since you should use command type adStoredProc, but I used procedure call as an example of how SQL statements in general should be handled.)


    Erland Sommarskog, SQL Server MVP, esquel@sommarskog.se
    Monday, August 01, 2011 6:29 PM
  • Echo what Erland said on best practices - you really should do this differently - but my best guess is that the quotes are not getting escaped correctly from the ASP page (one of the issues he mentioned.

    To try and troubleshoot the problem as is, try setting a variable to the complete value of what is going in the conn.execute and see if that string can be pasted directly in SSMS or some other SQL tool to surface the problem

    Probably easier to re-write it correctly

     

     

    Monday, August 01, 2011 6:55 PM
  • Hi folks,
    Firstly, thanks for your replies and the time you spent sending them. I know that some people don't like coming to these places and having their code picked apart but it's good that you have. Your feedback is great.
    Just to highlight a few things @Naomi: I do change the column name dynamically as well. I have only set it explicitly in this sp because I was being driven mad thinking that the error was with the other variable. Sorry, should have put the code up and made this clear.

    I deliberately avoided putting my sql strings together in the ASP client code. I've had a read through the documents that Hunchback posted earlier this morning (they seem pretty popular) and it confirmed my worries about SQL injection and security. I built another project in another life that used the ASP code to build the SQL strings and it was untidy and a nightmare. I ended up spending months changing it to use sps.

    I think I'll take some time tonight to restart this from scratch as recommended.

    Thanks again for your advice and time.

    Tom.
    Monday, August 01, 2011 7:28 PM
  • Tom, 

    If you plan to pass one column name and one column value (and you always assume that this column is char), then I will still recommend using the approach I suggested (e.g. parameter in sp_ExecuteSQL).

     

     


    For every expert, there is an equal and opposite expert. - Becker's Law


    My blog
    Monday, August 01, 2011 7:34 PM
    Moderator
  • > I deliberately avoided putting my sql strings together in the ASP client code. I've had a read through the documents that Hunchback posted earlier this morning (they seem pretty popular) and it confirmed my worries about SQL injection and security. I built another project in another life that used the ASP code to build the SQL strings and it was untidy and a nightmare. I ended up spending months changing it to use sps.

    But you haven't. What you have now is the worst of two worlds.

    You should aim at writing your stored procedures with static SQL. And call them with adStoredProc. For some type of functions, like search forms where the user can choose between lots of things to search from, dynamic SQL may be inevitable.

    I have an article on my web site that covers dynamic SQL in general: http://www.sommarskog.se/dynamic_sql.html

    If you need dynamic searches, I have a second article dedicated to that topic: http://www.sommarskog.se/dyn-search.html


    Erland Sommarskog, SQL Server MVP, esquel@sommarskog.se
    Monday, August 01, 2011 8:22 PM
  • Any progress?

    Just two comments:

    1. Always put SET NOCOUNT ON as the first line in a stored procedure

    2. Ask your DBA to monitor with SQL Profiler what you are sending to the server

     


    Kalman Toth, SQL Server & Business Intelligence Training; sqlusa.com
    Saturday, August 06, 2011 4:35 PM
    Moderator