Don't forget about SQL Injection - C# Discussion Boards

If you are using crystal reports

19-Jul-06 9:44

Problem is already solved Laugh | :laugh:

What can I do now Poke tongue | ;-P

how can I sum ...

mostafa_h19-Jul-06 8:22

mostafa_h

19-Jul-06 8:22

Hi All !
I want to get a Sum after Filtering some special record , for example [price] is a field in my Table1 .
strFilter="SELECT price From Table1 WHERE City='Athen'";
After filtering with this Query in my Report , I want to get a Sum for the all price related for City='Athen' and I show it in a Label .
Thanks A lot ,
Regards ,

s_mostafa_h

Ennis Ray Lynch, Jr.19-Jul-06 8:28

Re: If you are using crystal reports

19-Jul-06 8:28

You can create a running total field or a function. If you just want the sum

SELECT sum(price) as sumOfPrice FROM table WHERE city='Athen'

A man said to the universe:
"Sir I exist!"
"However," replied the Universe, "The fact has not created in me A sense of obligation."

-- Stephen Crane

mostafa_h19-Jul-06 10:09

mostafa_h

19-Jul-06 10:09

thanks
infact :
strFilter="SELECT sum(price) as sumOfPrice FROM table WHERE city='"+txtCity.Text+"'";
I want to put the result of this query in a Label .

s_mostafa_h

Don't forget about SQL Injection

Ennis Ray Lynch, Jr.19-Jul-06 10:12

Re: Don't forget about SQL Injection

19-Jul-06 10:12

I don't know if your particular application is subject to this but it is always wise to use stored procedures or at the very least parameterization of your query.

SELECT sum(price) as sumOfPrice FROM table WHERE city = @city;

A man said to the universe:
"Sir I exist!"
"However," replied the Universe, "The fact has not created in me A sense of obligation."

-- Stephen Crane

mostafa_h19-Jul-06 10:36

mostafa_h

19-Jul-06 10:36

thanks for ur patient ,
I'm very Blush | :O

,
Crystal Report simply provide this .
D'Oh! | :doh:

s_mostafa_h

Thread Safety??? [modified]

RizwanSharp19-Jul-06 8:12

19-Jul-06 8:12

Hello Devs,
I'm developing a Chat application and I'm facing a problem. I have the following Method which is called from multiple threads. And when I start sending too much messages in speed (which invokes this method so fast) then client is disconnected. Let me show you the method and then continue my discussion:

public void SendMessage(TextMessage messageToSend)<br />
        {<br />
            try<br />
            {<br />
                lock (this.dataStream)<br />
                {<br />
                    byte[] serializedBytes = messageToSend.GetBytes();                      // Get bytes from the Message object<br />
                    byte[] encMessageBytes = encryptorDecryptor.Encrypt(serializedBytes);   // Encrypt bytes<br />
                    serializedBytes = null;<br />
                    int messageSize = encMessageBytes.Length;<br />
                    byte[] messageSizeBuffer = new byte[this.messageDataLengthBufferSize];<br />
                    messageSizeBuffer = BitConverter.GetBytes(messageSize);<br />
                    this.dataStream.Write(messageSizeBuffer, 0, this.messageDataLengthBufferSize);                                  // Send the size of the message<br />
                    this.dataStream.Write(encMessageBytes, 0, messageSize);                        // Send the actual message<br />
                    this.dataStream.Flush();<br />
                    messageSizeBuffer = null;<br />
                    encMessageBytes = null;<br />
                }<br />
<br />
            }<br />
            catch (Exception ex)<br />
            {<br />
                string str = ex.Message + "\n" + ex.StackTrace;<br />
                Disconnect();<br />
                if (Disconnected != null)<br />
                {<br />
                    Disconnected(this, EventArgs.Empty);<br />
                }<br />
            }<br />
        }

In my opnion, the cause of this problem is multi threaded environment or it overflows data when i start sending to fast. for example if it is already working to send data on NetworkStream then a new call to this method is made which passes a new object of TextMessage to it which over writes the existing one so it is all messed up.
Note: Actual exception I get on the receiving end when length of data is not one which is supposed to be. See the code for Receiving:

int messageSize = BitConverter.ToInt32(messageDataLengthBuffer, 0);<br />
                MemoryStream memstrm = new MemoryStream();<br />
                <br />
                readByteCount = 0;<br />
                while (messageSize > 0)<br />
                {<br />
                    readByteCount = dataStream.Read(this.messageDataBuffer, 0, messageDataBuffer.Length);<br />
                    if (readByteCount < 1)<br />
                    {<br />
                        Disconnect();<br />
                        return;<br />
                    }<br />
                    memstrm.Write(this.messageDataBuffer, 0, readByteCount);<br />
                    messageSize -= readByteCount;<br />
                }<br />
                memstrm.Position = 0;<br />
                <br />
                byte[] decryptedData = encryptorDecryptor.Decrypt(memstrm.ToArray());<br />
                receivedMessage = new TextMessage(decryptedData);<br />
<br />
                memstrm.Close();<br />
                memstrm = null;

On the receving end I read 4 bytes as a size of incoming message then loop the piece of code to get all data untill complete message is received as of the message size. So when i start messages too fast then at the end of the loop "messageSize" usually gets in -ve which must never be less than 0 and when i try to get data decrypted again, it throughs exception "Length of data is invalid".

Is there any way that i can implement a mechanism that if SendMessage is doing its work then it should not be called again untill its done with its existing work? Or any other way to solve this issue?

Its really urgent. Any help will be really appreciated!!!

Best Regards

Ennis Ray Lynch, Jr.19-Jul-06 8:34

19-Jul-06 8:34

I assume that this.dataStream is the stream you are sending? If so better to create a queue of messages to send and then have a thread loop through the queue. Best part is that you will have a minimum of fuss.

private void AThreadedMethod(){
    while(go){
        if(queue.Count >0){
            //No need to lock the queue
            someObject = queue.Dequeue();
            ... do your stuff here
         }//end if
    }//end while
}

A man said to the universe:
"Sir I exist!"
"However," replied the Universe, "The fact has not created in me A sense of obligation."

-- Stephen Crane

RizwanSharp19-Jul-06 9:50

19-Jul-06 9:50

Thanks for reply, Yes dataStream is an object of NetworkStream. I can understand the mechanism of Queing the messages but creating 100 threads for 50 Clients(50 on client end and 50 on server end) will seriously do a rape with CPU;P. One easiest way to solve this problem is to put Thread.Sleep(50); above each call of this method. Which will also not consume CPU but its cheap and will hit performance of application.

Best Regards,

Perhaps only one thread?

Ennis Ray Lynch, Jr.19-Jul-06 10:32

Re: Perhaps only one thread?

19-Jul-06 10:32

And queue objects with message and connection details?

A man said to the universe:
"Sir I exist!"
"However," replied the Universe, "The fact has not created in me A sense of obligation."

-- Stephen Crane

RizwanSharp19-Jul-06 11:16

19-Jul-06 11:16

Did not get it Unsure | :~

Super Lloyd19-Jul-06 17:22

Super Lloyd

19-Jul-06 17:22

Thread.Sleep(50) is VERY WRONG synchronization.
In fact it's no syncrhonization at all, it just decrease bug frequency, making them even more difficult to debug.

Beside you only need 2 thread and a queue.
1 reader thread (or num CPU/2)
1 writer thread (or num CPU/2)

and then a queue of
{
Stream dest;
byte[] data
}

Dustin Metzgar19-Jul-06 9:13

19-Jul-06 9:13

Just to be sure, you're reading from and writing to a dataStream variable. Are those two different data streams in two different classes or are they the same stream?

Logifusion[^]

RizwanSharp19-Jul-06 9:52

19-Jul-06 9:52

yes dataStream is an object of NetworkStream which is used for both reading and writing messages on the stream. Any solution? Confused | :confused:

Best Regards

Dustin Metzgar19-Jul-06 10:16

19-Jul-06 10:16

The situation you have matches what you would use a ReaderWriterLock[^] for. A reader/writer lock allows only one writer at a time, but several readers at a time.

The only concern I would have after that is if you can only allow one reader at a time. In that case, I would stick with the lock you have, but apply the lock to readers as well. In the code you have above, it looks like you obtain a lock to write to the stream, but not to read. If a call to the read method happens in the middle of your write method, there's nothing in that read method checking if there's a lock or not. So it's entirely possible reads are still happening during your writes. You have to use the lock on both sides.

Logifusion[^]

RizwanSharp19-Jul-06 11:15

19-Jul-06 11:15

First of all yes! I'm maintainging locks on both read and write that is not problem with my case. The problem in my case is when
A piece of code calls
SendMessage(TextMessage textMessage)
{
// Doing work here
}
then an other piece of code calls this Method with a new object of TextMessage
Send(TextMessage textMessage)
{
// Doing work here
}
the second class overwrite the previous object of TextMessage Class and then the some part of data written to the stream is from the last object and one from the new object.
Its different from the scenerio you are pointing that stream is misbehaved. Stream is already locked and that is safe the only thing is grabbled is TextMessage Objec that is passed to the method.

Do you think I'm right?

If so then please suggest something. I'm really stucked at this problem.

Best Regards,
Rizwan

Dustin Metzgar19-Jul-06 11:25

19-Jul-06 11:25

There's not much information for me to go off of here. It looks like you have two different methods that write a TextMessage out to a stream. Are these methods in the same class or in different classes? Do they work with the same stream and obtain the same lock? Do you have two instances of the class that are talking to the same underlying network stream? I guess what I'm looking for here is if you have a singleton that has a monopoly over this network stream. Everything that goes to that stream therefore has to go through the same object (or through static methods of the same class). Then it's down to a syncronization issue on the methods reading and writing to that stream.

Logifusion[^]

RizwanSharp19-Jul-06 11:35

19-Jul-06 11:35

Forget about all the things and focus only on this code:

public void SendMessage(TextMessage messageToSend)<br />
{<br />
try<br />
{<br />
lock (this.dataStream)<br />
{<br />
byte[] serializedBytes = messageToSend.GetBytes(); // Get bytes from the Message object<br />
byte[] encMessageBytes = encryptorDecryptor.Encrypt(serializedBytes); // Encrypt bytes<br />
serializedBytes = null;<br />
int messageSize = encMessageBytes.Length;<br />
byte[] messageSizeBuffer = new byte[this.messageDataLengthBufferSize];<br />
messageSizeBuffer = BitConverter.GetBytes(messageSize);<br />
this.dataStream.Write(messageSizeBuffer, 0, this.messageDataLengthBufferSize); // Send the size of the message<br />
this.dataStream.Write(encMessageBytes, 0, messageSize); // Send the actual message<br />
this.dataStream.Flush();<br />
messageSizeBuffer = null;<br />
encMessageBytes = null;<br />
}<br />
<br />
}<br />
catch<br />
{<br />
}

Now try to follow me:
Code bock 'A' Makes a call SendMessage(textMessage1);
and SendMessage is in the middle of its work and a Code Block 'B' also call this Method SendMessage(textMessage2);
the second call overwrite the textMessage1 with textMessage2 D'Oh! | :doh:

I think this is the problem May be I'm wrong Cry | :((

I dont know what to do Sigh | :sigh:

Thanks for your help. If you have something more in mind please help me. I really need this so urgent.
Best Regards,
Rizwan

Dustin Metzgar19-Jul-06 13:45

Got the actual problem!!!

19-Jul-06 13:45

Well, you have a lock, which is fine. The lock will only let one thread through this code at a time. This is guaranteed. What's not guaranteed is what I'm asking about. This method is not static, so it must be an instance method. I assume that dataStream is also not static. My question is about where dataStream goes. Does it wrap around an existing stream that other objects can also wrap around? If you have a stream that is shared between two or more objects with this SendMessage method, then the lock is useless. That's why I'm asking if this is a singleton or not.

Logifusion[^]

RizwanSharp19-Jul-06 21:43

Re: Got the actual problem!!!

19-Jul-06 21:43

Sorry guy to put you in problem. And thanks a lot for your help(Really appreciate it Smile | :)

I got the problem on receiving end. Please try to follow me:

lock (this.dataStream) // Lock the NetworkStream
{
byte[] serializedBytes = messageToSend.GetBytes(); // Get bytes from the Message object
byte[] encMessageBytes = encryptorDecryptor.Encrypt(serializedBytes); // Encrypt bytes
serializedBytes = null;
int messageSize = encMessageBytes.Length; // Know the Length of encrypted bytes
byte[] messageSizeBuffer = new byte[this.messageDataLengthBufferSize]; // Make a buffer of 4 bytes to store an integer value
messageSizeBuffer = BitConverter.GetBytes(messageSize); // Convert the messageSize into bytes
this.dataStream.Write(messageSizeBuffer, 0, this.messageDataLengthBufferSize); // Send the size of the message
this.dataStream.Write(encMessageBytes, 0, messageSize); // Send the actual message
this.dataStream.Flush();
messageSizeBuffer = null;
encMessageBytes = null;

what I'm doing in the above code is
1) I lock the NetworkStream
2) Convert the TextMessage's object into bytes (So that I can write them on NetworkStream)
3) Encrypt the Bytes
4) Take the Length of the encrypted bytes and store it into a 4 byte's array
5) Before I send the original message, I write the size of message as a 4 byte array.So that the receiver may know that incoming message includes XXX bytes and i have to read only this much data (not more than that).

Mad | :mad:

But when I send messages too fast then the reading end reads data of more than one message (let me show you how)

Receiving End:

int messageSize = BitConverter.ToInt32(messageDataLengthBuffer, 0);
MemoryStream memstrm = new MemoryStream();

readByteCount = 0;
while (messageSize > 0)
{
readByteCount = dataStream.Read(this.messageDataBuffer, 0, messageDataBuffer.Length);
if (readByteCount < 1)
{
Disconnect();
return;
}
memstrm.Write(this.messageDataBuffer, 0, readByteCount);
messageSize -= readByteCount;
}
memstrm.Position = 0;

byte[] decryptedData = encryptorDecryptor.Decrypt(memstrm.ToArray());
receivedMessage = new TextMessage(decryptedData);

memstrm.Close();
memstrm = null;

1) I read 4 bytes from the Stream and Convert these 4 bytes to integer so I may know that incoming message has XXX number for bytes and I have to reaod only XXX bytes.

2) The Buffer I'm using is of 1024 which is filled up with this statement
readByteCount = dataStream.Read(this.messageDataBuffer, 0, messageDataBuffer.Length);

See problem exist at this statement i.e the dataStream inludes bytes of 3 messages on it which have to be read in 3 rounds but
I read all 3 messages with this statement because I'm reading 1024 bytes which is able to store all 3 messages.

Now suggest me a technique That i donot read more than the size of incoming message Confused | :confused:

Assuming that a message can be of 5000 bytes for which I'm looping on read.

I hope, the problem is very clear now and you can definitely suggest a god solution. I'm also working on this, If i get the solution I'll let you know.

Best Regards,
Rizwan;)

Dustin Metzgar20-Jul-06 5:23

20-Jul-06 5:23

Hmm, I see. I was assuming that since you wrote the size of the message into the stream:

this.dataStream.Write(messageSizeBuffer, 0, this.messageDataLengthBufferSize); // Send the size of the message
this.dataStream.Write(encMessageBytes, 0, messageSize); // Send the actual message

That you were also reading the size of the message first and then reading the message. But you're saying you just blindly take in 1024 bytes at a time assuming that the message will be a multiple of that length. In your loop, you should take into account that your message will not be a multiple of 1024. Either look at your messageSize to figure out how much you can read (1024 bytes or less), or pad the messages you send to multiples of 1024 bytes.

Logifusion[^]

Solved ;)

RizwanSharp20-Jul-06 5:33