1

Here is my code:

I should get output of the department id (did) as an integer and the templatefilename (result) that is required.

The errors I get are: Conversion from string "ad" to type 'Integer' is not valid. I'm fairly new to asp.net and cannot see where the did variable picks up the "ad" string.

Any help would be greatly appreciated.

Thanks

Phil
  • 1,811
  • 9
  • 38
  • 60
  • If you turn the option on to stop on exceptions when thrown, it should stop on the guilty line, which might make it a little more obvious – Rowland Shaw Jan 29 '10 at 08:37
  • Thank you, I have done and it stops at this line: result = (cmd.ExecuteScalar) – Phil Jan 29 '10 at 08:43

2 Answers2

3

When you construct the query to the table departmentsgroupings, you're changing the value of sql, but you aren't creating a new SqlCommand. This means that cmd still contains the old SQL statement (the query to the Modules table) which, when executed, returns "ad".

To fix this, change your code as follows: 

sql = ("select departmentsid from departmentsgroupings where groupingid =" & pageid & "")
Set cmd = New SqlCommand(sql, conn)    
did = (cmd.ExecuteScalar)

You may have expected the change you made to sql to get passed on automatically to the SqlCommand -- but it doesn't work that way.

Edit: Your code, as written, is vulnerable to SQL injection attacks. If you don't know what these are, you need to read the first answer to this:

How does the SQL injection from the "Bobby Tables" XKCD comic work?

To protect yourself against these kinds of attacks, use parameterized queries.

Community
  • 1
  • 1
Martin B
  • 23,670
  • 6
  • 53
  • 72
  • Thanks a lot Martin, I'll edit and put your advise into practice now. – Phil Jan 29 '10 at 08:59
  • BTW, i have a sqlinjection stopper, which comes from a usercontrol. It basically grabs the querystring on page init, and if it contains any bad words like select, drop, etc the user is redirected to 504.aspx before hitting this code. Would that work as prevention? – Phil Jan 29 '10 at 09:00
  • That sounds like it would Phil, be its still good practice to use parameterized queries anyway. So if you have the time i would recommend the rewrite. – kevchadders Jan 29 '10 at 09:09
  • Thanks Kev, ill look into this as a priority! – Phil Jan 29 '10 at 09:10
0

The mistake is in these lines:

sql = ("select departmentsid from departmentsgroupings where groupingid =" & pageid & "")
did = (cmd.ExecuteScalar)     <---- Wrong command executed here.

You presumably meant to execute the code in sql, not cmd again.

Tom Smith
  • 1,659
  • 1
  • 15
  • 20
  • `sql` is a `String`. A string doesn't have an `ExecuteScalar` method. The problem is that the OP needed to create a new `SqlCommand`. – Martin B Jan 29 '10 at 08:45
  • OK, corrected so it makes sense. Your answer is more useful though. – Tom Smith Jan 29 '10 at 08:47
  • Hi There, Thanks for the comment - My compiler is now stopping at result = (cmd.ExecuteScalar) In your comment you say strings do not have executescalar method. Can you recommend an alternative way of writing this line? Cheers, Phil. – Phil Jan 29 '10 at 09:18