Live data from Hacker News

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

miguelxpn.com

21–30 of 33 posts

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

#21
post #7
post #3

Earlier quoted context omitted.

Feels like this could be a case where DRY could fix a bug in one place

DRY is a good principle, but sometimes I’d rather repeat myself a little bit than build more bug-prone scaffolding to avoid repetition—especially when the repetition isn’t line-for-line identical. Just remember to always cite the repetition in comments.

Last time this caused memory corruption, I did a rewrite using eacaping-m4 to encode this difference in behavior. Gnu m4 has a changeword feature you can compile in for that.

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

#22
post #7

Earlier quoted context omitted.

DRY is a good principle, but sometimes I’d rather repeat myself a little bit than build more bug-prone scaffolding to avoid repetition—especially when the repetition isn’t line-for-line identical. Just remember to always cite the repetition in comments.

I’ve never come across a bug due to application of DRY, but I have seen many bugs because of “harmless” duplication resulting in inconsistent code changes several months later. Even aside from DRY, extracting a block of code to a function or method gives you an opportunity to name the block of code, which often significantly clarifies the intent of the code so, I’ve always tried to error on the side of overly DRY

I've seen people over-engineering a piece of code in order to make it "flexible" enough to be able to reuse it in like 2-3 scenarios that are not really well aligned - all in the name of making their code super DRY.

Sometimes it's better to have just a few separate, differently shaped pegs, than one flexible peg that fits any hole but is so complex that it might introduce 10 new bugs along the way.

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

#23
post #17

Earlier quoted context omitted.

I have see a dry bug. It happens when you deduplicate code which isn’t an exact duplicate, but only appears to be. For instance, one piece of code is supposed to do the same thing as another, but in context it caused a side effect that some other part of the code was inadvertently relying on.

>> It happens when you deduplicate code which isn’t an exact duplicate, but only appears to be. Embarrassing to admit, but until now I didn't think in this perspective - and on hindsight it should be obvious, sometimes code might look duplicate, but is not

Or it starts as duplicate but diverges over time. I've been bitten by the premature deduplication.

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

#24
post #19
post #16

I feel like this bug should be solved by using a proper parsing, instead of greedily looking for a field...

Headers are just text mixed with body and hard to figure out where they stop

Headers stop when you have 2 line endings consecutively[1], I don't see why it's hard.

[1] https://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4....

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

#25
post #19
post #16

I feel like this bug should be solved by using a proper parsing, instead of greedily looking for a field...

Headers are just text mixed with body and hard to figure out where they stop

Easy. They stop after \r\n\r\n.

There is no other RFC compliant way to end headers.

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

#26

Earlier quoted context omitted.

I haven't seen bugs due to DRY, but I've caused difficulty of adding new features and poor maintainability because of it. When I first took over maintaining Red Moon, I was a very new dev and went a little DRY crazy. In particular, there's a state machine for the different filter states (running, paused, stopped, etc), that had a bunch of classes (one per state) that had a lot of overlap. I pulled some common behavio…

I’ve never really understood this, because undoing DRY is relatively easy: you either copy the new function/class and rename it or you inline it in the mistaken case and adjust the code to match.

It can become much more convoluted than just having a function used in many places. It usually takes the form of multiple levels of inheritance. And it's a pain to deal with, and somewhat of a pain to refactor as well.

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

#27
post #17

Earlier quoted context omitted.

>> It happens when you deduplicate code which isn’t an exact duplicate, but only appears to be. Embarrassing to admit, but until now I didn't think in this perspective - and on hindsight it should be obvious, sometimes code might look duplicate, but is not

Or it starts as duplicate but diverges over time. I've been bitten by the premature deduplication.

Or worse, the natural progression should be that they diverge, but because they now share some kind of "reused" code in a library or class, an abomination is added to one or both users, just to keep the original "shared" feature.

Great fun.

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

#28
post #7

Earlier quoted context omitted.

DRY is a good principle, but sometimes I’d rather repeat myself a little bit than build more bug-prone scaffolding to avoid repetition—especially when the repetition isn’t line-for-line identical. Just remember to always cite the repetition in comments.

I’ve never come across a bug due to application of DRY, but I have seen many bugs because of “harmless” duplication resulting in inconsistent code changes several months later. Even aside from DRY, extracting a block of code to a function or method gives you an opportunity to name the block of code, which often significantly clarifies the intent of the code so, I’ve always tried to error on the side of overly DRY

I’ve seen plenty of DRY bugs due to premature abstraction and an unwillingness to refactor later on.

In many cases, the opportunity for DRY was misleading as the code was only superficially similar, so the implementation became over-complicated to handle various corner cases that otherwise wouldn’t have existed.

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

#29
post #5

Interesting bug, but even better it's nice to see encouragement of people to get involved in open source. The walk through of the process was great.

Humble and didactic, totally agree that we should thank the author. I for one felt compelled to contribute to OS after reading the article.

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

#30

  while ((s = strstr(s, "host:"))) {
    if (s == t || *(s-1) == '\r' || *(s-1) == '\n' ||
        *(s-1) == '\t' || *(s-1) == ' ') {
      have_header |= HTTP_HEADER_HOST;
    }
    s++;
  }
This s++ could be s + sizeof "host:" - 1.

Reason being: if you have just found "host:" at address s, you will not find another one at s+1, s+2, s+3, s+4 or s+4; "host:" supports no overlapped matches.

Actually since, we are really looking for "host:" preceded by whitespace, we can skip by just sizeof "host", (five bytes). Because if s points at "host:host:", the second "host:" is not of interest; it is not preceded by a whitespace character; we won't be setting the HTTP_HEADER_HOST bit for that one.

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 "host:" which doesn'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.

By the way, this test shows how verbose of a language C is:

        *(s-1) == '\r' || *(s-1) == '\n' ||
        *(s-1) == '\t' || *(s-1) == ' '
If we could use Javascript, look how nice it would be:

        strchr("\r\n\t ", s[-1])
Post reply on HN