TE
TechEcho
Home24h TopNewestBestAskShowJobs
GitHubTwitter
Home

TechEcho

A tech news platform built with Next.js, providing global tech news and discussions.

GitHubTwitter

Home

HomeNewestBestAskShowJobs

Resources

HackerNews APIOriginal HackerNewsNext.js

© 2025 TechEcho. All rights reserved.

I found and fixed a bug in PHP's standard library

167 pointsby miguelxpnabout 5 years ago

7 comments

glofishabout 5 years ago
Notably, the bugfix made PHP maintainers realize that the header handler had additional behaviors that they did not understand nor had a rationale for, so those behaviors were removed as well:<p><a href="https:&#x2F;&#x2F;github.com&#x2F;php&#x2F;php-src&#x2F;pull&#x2F;5201" rel="nofollow">https:&#x2F;&#x2F;github.com&#x2F;php&#x2F;php-src&#x2F;pull&#x2F;5201</a>
评论 #22460145 未加载
osrecabout 5 years ago
Looking at lines 460-521 in the modified file (<a href="https:&#x2F;&#x2F;github.com&#x2F;miguelxpn&#x2F;php-src&#x2F;blob&#x2F;f4b2089b642d504be358b06bbae81fa688d8bf0e&#x2F;ext&#x2F;standard&#x2F;http_fopen_wrapper.c#L460" rel="nofollow">https:&#x2F;&#x2F;github.com&#x2F;miguelxpn&#x2F;php-src&#x2F;blob&#x2F;f4b2089b642d504be3...</a>), is there not a benefit to `break` out of the while loop in the nested if statements?<p>Otherwise, it looks like it will call strstr() one extra time, even though you may have already determined that the specific header is present.
评论 #22460871 未加载
Lazareabout 5 years ago
Interesting bug, but even better it&#x27;s nice to see encouragement of people to get involved in open source. The walk through of the process was great.
评论 #22460934 未加载
评论 #22464070 未加载
kazinatorabout 5 years ago
<p><pre><code> while ((s = strstr(s, &quot;host:&quot;))) { if (s == t || *(s-1) == &#x27;\r&#x27; || *(s-1) == &#x27;\n&#x27; || *(s-1) == &#x27;\t&#x27; || *(s-1) == &#x27; &#x27;) { have_header |= HTTP_HEADER_HOST; } s++; } </code></pre> This s++ could be s + sizeof &quot;host:&quot; - 1.<p>Reason being: if you have just found &quot;host:&quot; at address s, you will not find another one at s+1, s+2, s+3, s+4 or s+4; &quot;host:&quot; supports no overlapped matches.<p>Actually since, we are really looking for &quot;host:&quot; preceded by whitespace, we can skip by just sizeof &quot;host&quot;, (five bytes). Because if s points at &quot;host:host:&quot;, the second &quot;host:&quot; is not of interest; it is not preceded by a whitespace character; we won&#x27;t be setting the HTTP_HEADER_HOST bit for that one.<p>Also, once we set the HTTP_HEADER_HOST bit, we can break out of the loop; there is no value in continuing it. The point of the loop is not to have a false negative: not to stop on a false match on &quot;host:&quot; which doesn&#x27;t meet the HTTP_HEADER_HOST conditions, and thereby fail to find the real one later in the string. If we find the real one, we are done.<p>By the way, this test shows how verbose of a language C is:<p><pre><code> *(s-1) == &#x27;\r&#x27; || *(s-1) == &#x27;\n&#x27; || *(s-1) == &#x27;\t&#x27; || *(s-1) == &#x27; &#x27; </code></pre> If we could use Javascript, look how nice it would be:<p><pre><code> strchr(&quot;\r\n\t &quot;, s[-1])</code></pre>
kinowabout 5 years ago
Thanks for sharing it! Really interesting, and well done!
Jaxanabout 5 years ago
I feel like this bug should be solved by using a proper parsing, instead of greedily looking for a field...
评论 #22462546 未加载
wolcoabout 5 years ago
That ends suddenly.