Merge lp:~vijit.chauhan/beeseek/fix-607076 into lp:beeseek

[vSC]

Fix for bug 607076

[Andrea Corbellini (andrea.corbellini)]

Your code is almost perfect, there are few and easy things to fix.

The major issue is that you are using tabs. You should instead use 4
spaces for indentation, like the rest of the code. This is the cause
of two of the exceptions you are seeing while running the test suite.

On Thu, Jul 29, 2010 at 10:23 PM, vSC <email address hidden> wrote:
> ...
> -                    remaining_bytes = int(remaining_bytes, 16)
> +                   try:
> +                       remaining_bytes = int(remaining_bytes, 16)
> +                   except ValueError:
> +                       raise HttpError('400 BAD REQUEST')

Here it'd be better to use '400 Bad Request' (capitalized, not upper
case), just to be consistent with the rest of the code).

> ...
> +    def test_invalid_chunk_size(self):
> +        client, server = socket.socketpair();
> +        client.sendall(
> +            'GET / HTTP/1.1 \r\n'

You ought to use PUT or POST instead of GET, otherwise the content
will be ignored. Also, you must remove the blank space between
'HTTP/1.1' and '\r\n': this is the cause of the third error in the
test suite.

> +            'Host: localhost\r\n'
> +            'Transfer-Encoding: chunked\r\n'

After this you must put a '\r\n' to indicate that headers are finished.

> +            's\r\n'
> +            '1234')
> +        def callback(handler):
> +            yield handler.parse_request()
> +            try:
> +                yield handler.read()
> +            except HttpError as err:
> +                self.assertEqual(error,('400 Bad Request'))

The right line is:

self.assertEqual(err.args, ('400 Bad Request',))

Note the last comma: it's very important in Python.

> +            else:
> +                self.fail()
> +
> +            conn = HttpConnection(server,callback,())
> +            conn.handle_incoming_bytes()

You should de-indent this last two lines.

These are just minor issues (that I can fix for you if you prefer) so
I'm approving the merge proposal. Great work, thanks!

[Andrea Corbellini (andrea.corbellini)]

review: Approve

[vSC (vijit.chauhan)]

Thanks for the Approval and all the help. :-)

I will keep the code consistency in mind next time.

Thanks,
Vijit

On Fri, Jul 30, 2010 at 2:25 AM, Andrea Corbellini <
<email address hidden>> wrote:

> The proposal to merge lp:~vijit.chauhan/beeseek/fix-607076 into lp:beeseek
> has been updated.
>
> Status: Needs review => Approved
> --
> https://code.launchpad.net/~vijit.chauhan/beeseek/fix-607076/+merge/31325<https://code.launchpad.net/%7Evijit.chauhan/beeseek/fix-607076/+merge/31325>
> You are the owner of lp:~vijit.chauhan/beeseek/fix-607076.
>

=== modified file 'node/bsnode/http.py'
--- node/bsnode/http.py	2010-07-20 16:12:17 +0000
+++ node/bsnode/http.py	2010-07-29 20:23:40 +0000
@@ -300,7 +300,10 @@
                 # chunk size (skipping blank lines).
                 while not remaining_bytes:
                     remaining_bytes = (yield self._raw_read_line()).rstrip()
-                remaining_bytes = int(remaining_bytes, 16)
+		    try:
+                    	remaining_bytes = int(remaining_bytes, 16)
+		    except ValueError:
+		        raise HttpError('400 Bad Request') 
 
                 if not remaining_bytes:
                     # This chunk has a size of zero. It means that the body is
@@ -338,7 +341,10 @@
                     while not remaining_bytes:
                         remaining_bytes = (
                             yield self._raw_read_line()).rstrip()
-                    remaining_bytes = int(remaining_bytes, 16)
+		    try:
+                    	remaining_bytes = int(remaining_bytes, 16)
+		    except ValueError:
+		        raise HttpError('400 BAD REQUEST') 
 
                     if not remaining_bytes:
                         # This chunk has a size of zero. It means that the body

=== modified file 'tests/node/http.py'
--- tests/node/http.py	2010-07-20 16:12:17 +0000
+++ tests/node/http.py	2010-07-29 20:23:40 +0000
@@ -108,6 +108,28 @@
         # Check that the callback has been called the right number of times.
         self.assertEqual(len(calls), 2)
 
+    def test_invalid_chunk_size(self):
+        client, server = socket.socketpair();
+        client.sendall(
+            'GET / HTTP/1.1 \r\n'
+            'Host: localhost\r\n'
+            'Transfer-Encoding: chunked\r\n'
+            's\r\n'
+            '1234')
+        def callback(handler):
+            yield handler.parse_request()
+            try:
+                yield handler.read()
+            except HttpError as err:
+                self.assertEqual(error,('400 Bad Request'))
+            else:
+                self.fail()
+
+            conn = HttpConnection(server,callback,())
+            conn.handle_incoming_bytes()
+		
+
+
 
 class TestRequestParser(TestCase):
     """Check if requests are correctly parsed."""