Skip to content
This repository

Fix a varios bugs in fapws3 #19

Closed
wants to merge 7 commits into from

2 participants

Alexander Shigin William
Alexander Shigin
  • memleak in py_rfc1123_date;
  • segfault if handler return type isn't list, file or iterator;
  • proper handling the closing of a socket by client;
  • add example of DoS client.
William

Thanks shigin for your cooperation.

Except the "do not segfault on wrong type", I'll merge your changes. Indeed, this one has also been corrected on my local repository (with a bit more changes).

I have to see how I can merge all your changes except that one ;-(.

William

William

merged in master.

Thanks for your contribution

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
0  INSTALL 100755 → 100644
File mode changed
0  README.markdown 100755 → 100644
Source Rendered
File mode changed
9  fapws/_evwsgi.c
@@ -399,11 +399,14 @@ PyObject *py_defer_queue_size(PyObject *self, PyObject *args)
399 399
 PyObject *py_rfc1123_date(PyObject *self, PyObject *args)
400 400
 {
401 401
     time_t t;
  402
+    PyObject *result;
  403
+    char *rfc_string = NULL;
402 404
     if (!PyArg_ParseTuple(args, "L", &t))
403 405
         return NULL;
404  
-    char *res=NULL;
405  
-    res=time_rfc1123(t);
406  
-    return PyString_FromString(res);
  406
+    rfc_string = time_rfc1123(t);
  407
+    result = PyString_FromString(rfc_string);
  408
+    free(rfc_string);
  409
+    return result;
407 410
 }
408 411
 
409 412
 
2  fapws/extra.h
@@ -11,5 +11,5 @@ char *cur_time(char * fmt);
11 11
 
12 12
 char *time_rfc1123(time_t t);
13 13
 
14  
-char *cur_time_rfc1123();
  14
+char *cur_time_rfc1123(void);
15 15
 
20  fapws/mainloop.c
@@ -397,6 +397,10 @@ Procedure that will write "len" bytes of "response" to the client.
397 397
 */
398 398
 int write_cli(struct client *cli, char *response, size_t len,  int revents)
399 399
 {
  400
+    /* XXX The design of the function is broken badly: after the first EAGAIN
  401
+       error we should exit and wait for an EV_WRITE event. You can think about
  402
+       slow client or some client which holds a socket but don't read from one.
  403
+     */
400 404
     size_t r=0, sent_len=MAX_BUFF;
401 405
     int c=0;
402 406
     if (revents & EV_WRITE){
@@ -414,13 +418,18 @@ int write_cli(struct client *cli, char *response, size_t len,  int revents)
414 418
                 printf("host=%s,port=%i write_cli:uri=%s,r=%i,len=%i,c=%i\n", cli->remote_addr, cli->remote_port, cli->uri, (int)r, (int)len,c);
415 419
             if (((int)r<0) & (errno != EAGAIN))
416 420
             {
  421
+                if (errno == EPIPE || errno == ECONNRESET) {
  422
+                    // The client closes the socket. We can log the error.
  423
+                    return 0;
  424
+                }
417 425
                 cli->retry++;
418 426
                 fprintf(stderr,"Failed to write to the client:%s:%i, #:%i.\n", cli->remote_addr, cli->remote_port, cli->retry);
419 427
                 if (cli->retry>MAX_RETRY) 
420 428
                 {
421 429
                     fprintf(stderr, "Connection closed after %i retries\n", cli->retry);
422 430
                     return 0; //stop the watcher and close the connection
423  
-                    }
  431
+                }
  432
+                // XXX We shouldn't sleep in an event-base server.
424 433
                 usleep(100000);  //failed but we don't want to close the watcher
425 434
             }
426 435
             if ((int)r==0)
@@ -432,9 +441,9 @@ int write_cli(struct client *cli, char *response, size_t len,  int revents)
432 441
                 response+=(int)r;
433 442
                 len -=r ;
434 443
             }
435  
-         }
436  
-         //p==len
437  
-         return 1;
  444
+        }
  445
+        //p==len
  446
+        return 1;
438 447
     }
439 448
     else {
440 449
         printf("write callback not ended correctly\n");
@@ -554,8 +563,9 @@ void write_cb(struct ev_loop *loop, struct ev_io *w, int revents)
554 563
             }
555 564
             //free(buff);
556 565
         } 
557  
-        else if (PyIter_Check(cli->response_content_obj)) //we treat Iterator object
  566
+        else if (cli->response_content_obj != NULL && PyIter_Check(cli->response_content_obj))
558 567
         {
  568
+            //we treat Iterator object
559 569
             cli->response_iter_sent++;
560 570
             PyObject *pyelem = cli->response_content;
561 571
             if (pyelem == NULL) 
67  sample/test/slow_client.py
... ...
@@ -0,0 +1,67 @@
  1
+"""The sample slow + fast client.
  2
+
  3
+The client can work fine if server is localhost.  Otherwise time_request
  4
+client usually takes the same time as long_request.
  5
+"""
  6
+
  7
+import sys
  8
+import socket
  9
+import time
  10
+import threading
  11
+
  12
+if len(sys.argv) > 2:
  13
+    print "usage: %s [host[:port]]" % sys.argv[0]
  14
+    sys.exit(1)
  15
+
  16
+hostname = 'localhost'
  17
+port = 8080
  18
+if len(sys.argv) == 2:
  19
+    if ':' in sys.argv[1]:
  20
+        hostname, port = sys.argv[1].split(':')
  21
+        port = int(port)
  22
+    else:
  23
+        hostname = sys.argv[1]
  24
+headers = '\r\nHost: %s\r\n\r\n' % hostname
  25
+
  26
+def timed(func):
  27
+    def wrapper(*args, **kwargs):
  28
+        start = time.time()
  29
+        try:
  30
+            return func(*args, **kwargs)
  31
+        finally:
  32
+            print '%s took %.2f sec' % (repr(func), time.time() - start)
  33
+    return wrapper
  34
+
  35
+lock = threading.Lock()
  36
+@timed
  37
+def long_request():
  38
+    s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
  39
+    s.connect((hostname, port))
  40
+    s.send('GET /file HTTP/1.0' + headers)
  41
+    for i in range(1):
  42
+        s.recv(80)
  43
+        time.sleep(1)
  44
+    lock.release()
  45
+    print 'release'
  46
+    for i in range(10):
  47
+        s.recv(80)
  48
+        time.sleep(1)
  49
+    s.close()
  50
+
  51
+@timed
  52
+def time_request():
  53
+    lock.acquire()
  54
+    print 'acquire'
  55
+    s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
  56
+    s.connect((hostname, port))
  57
+    s.send('GET /time HTTP/1.0' + headers)
  58
+    s.recv(900)
  59
+    s.close()
  60
+
  61
+lock.acquire()
  62
+fl = threading.Thread(target=long_request)
  63
+tl = threading.Thread(target=time_request)
  64
+fl.start()
  65
+tl.start()
  66
+tl.join()
  67
+fl.join()
35  sample/test/test.py
... ...
@@ -0,0 +1,35 @@
  1
+#!/usr/bin/env python
  2
+# -*- coding: utf-8 -*-
  3
+
  4
+from time import time
  5
+import fapws._evwsgi as evwsgi
  6
+from fapws import base
  7
+
  8
+def start():
  9
+    evwsgi.start("0.0.0.0", "8080")
  10
+    evwsgi.set_base_module(base)
  11
+
  12
+    def return_file(environ, start_response):
  13
+        start_response('200 OK', [('Content-Type','text/html')])
  14
+        return open('big-file')
  15
+
  16
+    def return_tuple(environ, start_response):
  17
+        start_response('200 OK', [('Content-Type','text/plain')])
  18
+        return ('Hello,', " it's me ", 'Bob!')
  19
+
  20
+    def return_rfc_time(environ, start_response):
  21
+        start_response('200 OK', [('Content-Type','text/plain')])
  22
+        return [evwsgi.rfc1123_date(time())]
  23
+
  24
+    evwsgi.wsgi_cb(("/file", return_file))
  25
+    evwsgi.wsgi_cb(("/tuple", return_tuple))
  26
+    evwsgi.wsgi_cb(("/time", return_rfc_time))
  27
+
  28
+    evwsgi.run()
  29
+
  30
+if __name__=="__main__":
  31
+    try:
  32
+        open('big-file')
  33
+    except IOError:
  34
+        open('big-file', 'w').write('\n'.join('x'*1024 for i in range(1024)))
  35
+    start()
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.