Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

疑惑和bug? #13

Open
L-admin opened this issue Apr 8, 2018 · 7 comments
Open

疑惑和bug? #13

L-admin opened this issue Apr 8, 2018 · 7 comments

Comments

@L-admin
Copy link

L-admin commented Apr 8, 2018

从知乎而来,看了楼主的代码,感觉受益匪浅。有几个疑惑:
1 hashmap那个结构,第一次见到这种写法,感觉有点意思,这种写法是什么专门的写法吗?
2 有没有可能buffer里面一次读取的是一个完整的包和第二个包的一部分(百度了下,这种是不是叫粘包?),那该怎么处理?
3 在server.c 中124行为什么要调用get_pid()?132行 daemon(1, 0);没有看到这个函数实现?
下面有几个我感觉是bug的地方。
4 get_pid函数中中没有关闭文件。
5 request_handle_uri函数中用了 r->port = atoi(uri->port.data);uri->port.data 并不是用‘\0’结尾的字符串,同理header_handle_content_length函数中int len = atoi(val->data)。
6 parse_uri函数中解析uri,uri->port.data没有设置值,uri->host.len没有设置值。

再次感谢楼主的代码。

@wgtdkp
Copy link
Owner

wgtdkp commented Apr 9, 2018

  1. 是专用的写法,不是通用的;在已知key的总数的时候,这样写可以避免动态内存分配;
  2. 这是可能的,在同一个连接中,一般浏览器在前一个请求的回复没有收到前,不会发送下一个请求;这反之,就是pipelining;pipelining的效率显然更高(但我最开始没有支持),请看issue 5和issue 10的讨论;
  3. g_pid() 是为了在PID文件缺失的情况下新建该文件;daemon() 是unix系统函数;
  4. 确实是bug;
  5. 确实atoi应该接受C-string,但是在这两个场景中,atoi不需要依靠‘\0’来标志convertion的结束,因为这两个digit sequence的后面都接有非digit的字符(否则request会提前返回error,这两处也就不会执行到了),因此atoi会提前结束;我应该写一些注释的;像request.c的322行,就是必须C-string的例子。
  6. 有设置吧,在 case URI_S_PORT: 里面;

@L-admin
Copy link
Author

L-admin commented Apr 9, 2018

1 2 3受教。对于5,6,我还有疑惑。

  • 对于5
    用client模拟发起HTTP请求,当uri为“http://127.0.0.1:8000/” 时。程序会出现段错误,调试core文件发现,atoi炸了。

  • 对于6

     case URI_S_HOST:
         switch (ch) {
         case 'A' ... 'Z':
         case 'a' ... 'z':
         case '0' ... '9':
         case '.':
         case '-':
             break;
         case ':':
             uri->state = URI_S_PORT;
             break;
         case '/':
             uri->abs_path.data = p;
             uri->state = URI_S_ABS_PATH_SLASH;
             break;
         default:
             return ERR_INVALID_URI;
         }
         break;
    
     case URI_S_PORT:
         switch (ch) {
         case ' ':
             // For example: http://localhost
             // We set uri to the first slash after colon(':')
             uri->abs_path.data = uri->host.data - 2;
             uri->abs_path.len = 1;
    
             ++uri->port.data;
             uri->port.len = p - uri->port.data;
             uri->state = URI_S_BEGIN;
             break;
         case '/':
             uri->abs_path.data = p;
    
             ++uri->port.data;
             uri->port.len = p - uri->port.data;
             uri->state = URI_S_ABS_PATH_SLASH;
             break;
         case '0' ... '9':
             break;
         default:
             return ERR_INVALID_URI;
         }
         break;
    

    我没有在case URI_S_PORT中找到
    以解析uri为"http://127.0.0.1:8000/" 为例。
    在case URI_S_PORT:中有++uri->port.data操作,但是没有看到在上一个case URI_S_HOST: 的case ':' 中设置uri->port.data=p。同时我认为还应该在这里设置uri->host.len = p - uri->host.data
    之前我在request_handle_request_line函数中尝试打印解析出来的uri,程序就crash掉了。打印uri.host和uri.port崩了。

  • 附模拟HTTP request

    const char* http_request = "GET http://127.0.0.1:8000/ HTTP/1.1" "\r\n"
                           "host: 127.0.0.1:8080" "\r\n"
                           "user-agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Ubuntu Chromium/65.0.3325.181 Chrome/65.0.3325.181 Safari/537.36" "\r\n"
                           "accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8" "\r\n"
                           "referer: http://127.0.0.1:8000/" "\r\n"
                           "accept-encoding: gzip, deflate, br" "\r\n"
                           "accept-language: zh-CN,zh;q=0.9" "\r\n"
                           "\r\n"
    
                           "hello world!";
    
    send(fd, http_request, strlen(http_request), 0);
    

    5和6的测试都是在此请求下有问题,为了方便显示,我注释掉了设置守护进程代码。
    测试平台:gcc version 4.8.4 (Ubuntu 4.8.4-2ubuntu1~14.04.4)

@wgtdkp
Copy link
Owner

wgtdkp commented Apr 10, 2018

你测5的时候炸掉,应该是6的问题造成的。😓,没想到还有这么明显的bug没有测出来。parse那一段确实有许多问题,我自己也很久没有维护这个project了。你可以尝试改掉这个bug给我PR~

@wgtdkp
Copy link
Owner

wgtdkp commented Apr 10, 2018

或者我晚点fix~

@L-admin
Copy link
Author

L-admin commented Apr 11, 2018

对于5,我增加了一个string_t_to_int的函数。
对于6,目前只增加了我上面提到的两行。但后来测试参数解析那里好像也有点问题。parse那个状态机是个细致活,重在思想吧。
还是很感谢楼主的代码,让我学习了json解析,HTTP长连接实现,堆管理连接,多进程,epoll,状态机,还有非阻塞读取消息的正确姿势。现在再回来看UNP,对很多知识有了进一步理解。

@wgtdkp
Copy link
Owner

wgtdkp commented Apr 11, 2018

确实是应该写一个定制的 string_t_to_int,我太懒了。
状态机的话,严谨的测试需要很多case,这个工作量还是挺多的。
如果对这些感兴趣的话,可以看看nginx的代码。我也是从nginx学过来的,我省去了HTTP的许多逻辑,但是整体的架构、优化的手段,基本都能够在nginx里面找到。推荐你有时间也可以看看,对编程修养的提高还是挺有帮助的(nginx也不过20000 - 30000 loc)~

@zhangshaoxiao
Copy link

正在学习,代码刚看了一部分。回答下你的问题2.
关于粘包问题:TCP协议确实会存在粘包问题:在某个数据到达的时刻,缓冲区主要有以下几种状态:
1.缓冲区为第一个包的不完全包
2.缓冲区为第一个包的全包和后一个包的不完全包
3.在某一个时刻,缓冲区为某个包的不完全包,后续跟着若干个(>=0)全包,而后又跟着一个不完全包
4.so on
解决方法:
HTTP的请求包含3个部分:请求行,包头,包体,其中请求行以\r\n结束,包头以\r\n\r\n结束,
1.使用状态机读取缓冲区,若读到完整请求行+包头,则解析之,并读取其中的包体长度,根据包体长度读取包体,处理完后将处理过的字符串从缓冲区清掉。然后返回步骤1
2.若读不到完整包,则返回again,下次缓冲区有数据到达时从步骤1重新开始
3.若一次到达多个包,按照1的模式处理多个包,将剩下的最后一个不完全包留在缓冲区,等着和剩下的内容进行拼接。下次数据到达时重新返回步骤1
需要注意:为了防止恶意攻击,缓冲区应该有最大长度,超出时需要关闭!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants