views:

61

answers:

1

i have the a query which is union of two querries, the resulting query is bringing duplicate records, i dont want duplicate records, i tried putting DISTINCT but getting the same result, can anybody help me rectifying this query. i also want to know whether this query is safe from sql injection...i'll be pasting my query below:

ALTER PROCEDURE [dbo].[sp_GetTrashListWithSorting] --'6dbf9a01-c88f-414d-8dd9-696749258cef', '6dbf9a01-c88f-414d-8dd9-696749258cef','DateTime ASC','0','30'
(
  @p_CreatedBy UNIQUEIDENTIFIER,
  @p_ToReceipientID UNIQUEIDENTIFIER,
  @p_SortExpression NVARCHAR(100),
  @p_StartIndex INT,
  @p_MaxRows INT
)
As
SET NOCOUNT ON;

IF LEN(@p_SortExpression)=0 
  SET @p_SortExpression ='DateTime DESC'

DECLARE @Sql NVARCHAR(4000)
SET @sql ='SELECT ID, DateTime, Subject, CreatedBy, ToReceipientID, Status
             FROM (SELECT ID, 
                          DateTime, 
                          Subject, 
                          CreatedBy, 
                          ToReceipientID, 
                          Status,
                          ROW_NUMBER() OVER(ORDER BY '+ @p_SortExpression +') AS Indexing
                     FROM (SELECT ID,
                                  DateTime, 
                                  Subject, 
                                  CreatedBy, 
                                  ToReceipientID, 
                                  SenderStatus AS Status
                             FROM ComposeMail 
                            WHERE (CreatedBy =@p)
                              AND (SenderStatus = 7 OR SenderStatus = 8 )
                  UNION
                  SELECT ID,
                         DateTime, 
                         Subject, 
                         CreatedBy, 
                         ToReceipientID, 
                         ReceiverStatus As Status
                    FROM ComposeMail
                   WHERE (ToReceipientID = @p1) 
                     AND (ReceiverStatus = 7 OR ReceiverStatus = 8)) AS NewDataTable
            ) AS IndexTable
   WHERE Indexing > @p2 AND Indexing<= (@p2+@p3)' 

DECLARE @paramDefination NVARCHAR(500)
SET @paramDefination =N'@p UNIQUEIDENTIFIER ,@p1 UNIQUEIDENTIFIER, @p2 INT, @p3 INT'

EXEC sp_executesql @sql, @paramDefination,
    @p = @p_CreatedBy,
    @p1 = @p_ToReceipientID,
    @p2 = @p_StartIndex ,
    @p3 = @p_MaxRows
+2  A: 

1) I re-wrote your SQL as:

WITH trash_list AS (
SELECT cm.id,
       cm.datetime, 
       cm.subject,
       cm.createdby,
       cm.toreceipientid, 
       cm.senderstatus AS Status
  FROM COMPOSEMAIL cm
 WHERE cm.createdBy = @p
   AND cm.enderStatus IN(7, 8)
UNION
SELECT cm.id,
       cm.datetime, 
       cm.subject,
       cm.createdby,
       cm.toreceipientid, 
       cm.receiverstatus AS Status
  FROM COMPOSEMAIL cm
 WHERE cm.toreceipientid = @p1
   AND cm.receiverstatus IN (7, 8))
SELECT t.id,
       t.datetime, 
       t.subject,
       t.createdby,
       t.toreceipientid, 
       t.status
  FROM (SELECT tl.id,
               tl.datetime, 
               tl.subject,
               tl.createdby,
               tl.toreceipientid, 
               tl.status,
               ROW_NUMBER() OVER(ORDER BY '+ @p_SortExpression +') AS Indexing
          FROM trash_list tl
      GROUP BY tl.id,
               tl.datetime, 
               tl.subject,
               tl.createdby,
               tl.toreceipientid, 
               tl.status) t
 WHERE t.indexing BETWEEN @p2 AND (@p2+@p3)

...but if you still get duplicates, review the logic in the SELECT/UNION in the WITH clause.

Get it to work as normal SQL before turning it into dynamic SQL.

2) The query is not safe from injection attacks because you aren't handling single quotes when users can provide text:

IF LEN(@p_SortExpression)=0 
  SET @p_SortExpression ='DateTime DESC'

...should be:

IF LEN(@p_SortExpression)=0 
  SET @p_SortExpression ='DateTime DESC'
ELSE
  SET @p_SortExpression = REPLACE(@p_SortExpression, '''', '''''')
OMG Ponies