Everything
php
Learn Community Releases The PHP Project
Home » Project » Newsgroups » php.internals » fgets()/fgetss() BC break in HEAD »

fgets()/fgetss() BC break in HEAD

php.internals

Antony Dovgal

19 years ago
Hello all. I'd like to continue recent IRC discussion here, to draw more attention to this issue. At the moment functions fgets() and fgetss() are broken in HEAD, as they return <maxlen> characters instead of <maxlen - 1>, as they do in 5.2 (and I'm pretty sure this is the way they used to work since their very first appearance in PHP). So, anybody relying on fgets($fp, 10) to return 9 characters would get 10 characters instead. Yes, this behaviour is not very intuitive for those who are not familiar with fgets() in C, but it's well documented and pretty much expected. I believe this is an unreasonable BC break and I strongly believe it should be fixed. There are 0 reasons to do it (without any discussion) and it can only result in more users' confusion, even if the original purpose was the contrary.
-- Wbr, Antony Dovgal

Andi Gutmans

19 years ago
Sounds like something which indeed isn't worth breaking. Was this intentional?

Antony Dovgal

19 years ago
On 11/14/2006 08:26 AM, Andi Gutmans wrote:
> Sounds like something which indeed isn't worth breaking. Was this > intentional?
Sara says it was intentional, that's why I decided to write to the list. I don't think such intentional breaks should take place in any PHP version.
>> -----Original Message----- >> From: Antony Dovgal [mailto:antony@zend.com] >> Sent: Monday, November 13, 2006 1:39 PM >> To: php-dev >> Subject: [PHP-DEV] fgets()/fgetss() BC break in HEAD >> >> Hello all. >> >> I'd like to continue recent IRC discussion here, to draw more >> attention to this issue. >> >> At the moment functions fgets() and fgetss() are broken in >> HEAD, as they return <maxlen> characters instead of <maxlen - >> 1>, as they do in 5.2 (and I'm pretty sure this is the way >> they used to work since their very first appearance in PHP). >> So, anybody relying on fgets($fp, 10) to return 9 characters >> would get 10 characters instead. >> >> Yes, this behaviour is not very intuitive for those who are >> not familiar with fgets() in C, but it's well documented and >> pretty much expected. >> >> I believe this is an unreasonable BC break and I strongly >> believe it should be fixed. >> There are 0 reasons to do it (without any discussion) and it >> can only result in more users' confusion, even if the >> original purpose was the contrary. >> >> -- >> Wbr, >> Antony Dovgal >> >> -- >> PHP Internals - PHP Runtime Development Mailing List To >> unsubscribe, visit: http://www.php.net/unsub.php >> >
-- Wbr, Antony Dovgal

Edin Kadribasic

19 years ago
Antony Dovgal wrote:
> On 11/14/2006 08:26 AM, Andi Gutmans wrote: >> Sounds like something which indeed isn't worth breaking. Was this >> intentional? > > Sara says it was intentional, that's why I decided to write to the list. > I don't think such intentional breaks should take place in any PHP > version. >
I could not agree more. I think that this change should be reverted. Edin

Sara Golemon

19 years ago
Antony Dovgal wrote:
> On 11/14/2006 08:26 AM, Andi Gutmans wrote: >> Sounds like something which indeed isn't worth breaking. Was this >> intentional? > > Sara says it was intentional, that's why I decided to write to the list. > I don't think such intentional breaks should take place in any PHP version. >
It was quasi-intentional. The functions needed a lot of changes related to PHP6 Unicodiness and I implemented "maxchars" to mean "maxchars" rather than the appearantly expected "maxcharsbutnotreallycausewedonotwantthelastcharacter". As I told tony in IRC, I don't care if the behavior gets changed back to 5.2 style, although I don't think this is a BC we need to worry about keeping. I'll lay money that NOONE is relying on this, and I challenge any of you to prove me wrong on that count. If y'all do want maxcharsminusone behavior for the argument, fine, but rename the arg to something else. maxchars means "I want this many characters or less", not "I want one less than this many characters{or less}". -Sara

Zeev Suraski

19 years ago
I can't prove you wrong, but to me it sounds extremely reasonable that there'd be a lot of lines of code that rely on that behavior exactly. You believe nobody is using fgets() to read, say, 3 bytes? Unless I'm missing something, whenever someone uses this function to read an exact number of bytes (as opposed to reading a full line) - his logic will now be broken. As a quick reference, on the switch from PHP 3 to 4 (when the popularity of PHP was maybe 1/20 of what it is today) every tiniest breakage we thought would bother no one, has been noticed and complained about. Frankly, I think this is a bigger breakage than most of the stuff we broken in PHP 4 (but again, I may be missing something). Re the argument name change, if you can find a better name for argminusone, I'm all for it. I'm doubtful we can come up with something good, but I think it's better to stay with the old behavior and a somewhat bogus argument name than to break compatibility. My 2c. Zeev At 22:59 14/11/2006, Sara Golemon wrote:

Wez Furlong

19 years ago
I'd lay money on someone somewhere having something critical relying on the traditional behavior. IIRC, this came up before and we decided to preserve the way it worked. I don't see any need to change this in 6 or any later version. As crappy as that may be, BC is BC. :-/ --Wez. On 11/14/06, Sara Golemon <pollita@php.net> wrote:

Stanislav Malyshev

19 years ago
> As I told tony in IRC, I don't care if the behavior gets changed back to > 5.2 style, although I don't think this is a BC we need to worry about > keeping. I'll lay money that NOONE is relying on this, and I challenge > any of you to prove me wrong on that count.
How much money? ;) Anyway, if nobody really cares for that, more the reason to leave it alone. If it ain't broken, you know.
> If y'all do want maxcharsminusone behavior for the argument, fine, but > rename the arg to something else. maxchars means "I want this many > characters or less", not "I want one less than this many characters{or > less}".
Call it "limit" if you care for that. Limit can mean anything :)
-- Stanislav Malyshev, Zend Products Engineer stas@zend.com http://www.zend.com/

Ilia A.

19 years ago
On 14-Nov-06, at 3:59 PM, Sara Golemon wrote:
> I'll lay money that NOONE is relying on this, and I challenge any > of you to prove me wrong on that count.
Just how much money are we talking about here? ;-) http://www.google.com/codesearch?as_q=fgets%5C%28%5C%24%5BA-Za-z0-9_% 5D%2B%2C+2%5C%29%3B&btnG=Search+Code&as_lang=php Take a look @ the first result. Ilia Alshanetsky

Steph

19 years ago
It better be in Canadian dollars ;) (Yeah I got inet access again) Sara, more seriously, when Zeev puts in a plea and Edin's saying the same thing, it's likely you're misjudging the situation. It's a rare event :)

Richard Lynch

19 years ago
On Tue, November 14, 2006 2:59 pm, Sara Golemon wrote:
> keeping. I'll lay money that NOONE is relying on this, and I > challenge > any of you to prove me wrong on that count.
Errrr. You may want to re-think that bet... Surely somebody out there has: <?php $field_X = fgets($file, 10); //get the next 9 bytes $field_Y = fgets($file, 4); //get the next 3 bytes ?> even if you and I both know how horrible that code without error-checking is. There's probably somebody who even went to the trouble of error-checking their fgets() calls to be sure they got the right number of bytes. I'm not wasting my time to find code like this to win the bet, but if Vegas was giving odds, I'd put money against ya. If you REALLY wanted this BC-breaking behaviour for a Good Reason, or even a Bad Reason... As it is, seems like it's something to just revert and live with the documented mis-behaviour. Or change the docs and the variable name to something other than maxchars is a perfect solution. :-)
-- Some people have a "gift" link here. Know what I want? I want you to buy a CD from some starving artist. http://cdbaby.com/browse/from/lynch Yeah, I get a buck. So?

Richard Quadling

19 years ago
As we are often told on this list, PHP is not C. It seems utterly ridiculous to return maxlen-1. Whilst this is sure as hell a BC, it should work "properly". If I ask for 10, I want 10. I wouldn't have asked for 10 otherwise. Having to know the "magic" to get things to work is plainly bad magic. In terms of KISS, 10 means 10. Unless PHP !== KISS therefore 10 === 9. Make the break. Deal with it. It was wrong and now it is fixed. On 13/11/06, Antony Dovgal <antony@zend.com> wrote:
> Hello all. > > I'd like to continue recent IRC discussion here, to draw more attention to this issue. > > At the moment functions fgets() and fgetss() are broken in HEAD, as they return <maxlen> > characters instead of <maxlen - 1>, as they do in 5.2 (and I'm pretty sure this is the way > they used to work since their very first appearance in PHP). > So, anybody relying on fgets($fp, 10) to return 9 characters would get 10 characters instead. > > Yes, this behaviour is not very intuitive for those who are not familiar with fgets() in C, > but it's well documented and pretty much expected. > > I believe this is an unreasonable BC break and I strongly believe it should be fixed. > There are 0 reasons to do it (without any discussion) and it can only result in more users' confusion, > even if the original purpose was the contrary. > > -- > Wbr, > Antony Dovgal > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php > >
-- ----- Richard Quadling Zend Certified Engineer : http://zend.com/zce.php?c=ZEND002498&r=213474731 "Standing on the shoulders of some very clever giants!"

Stanislav Malyshev

19 years ago
> It seems utterly ridiculous to return maxlen-1. > > Whilst this is sure as hell a BC, it should work "properly". If I ask > for 10, I want 10. I wouldn't have asked for 10 otherwise. Having to > know the "magic" to get things to work is plainly bad magic.
It always returned length-1, manual says length-1, so suddenly in version 5.2.1 it starts returning length instead. And now imagine somebody who tries to use this function in his scripts. He should start adding version checks to each invocation of this function or what? Doesn't seem very nice behavior to me.
> Make the break. Deal with it. It was wrong and now it is fixed.
Yes, and the way to deal with it is to restore the function to its previous functionality, according to documentation and the way it always worked. If you need exact number, you can always use fread. There's absolutely no reason to break 100% of working scripts (since nobody uses the new semantics and everybody uses the old) just to satisfy someone's sense of purity.
-- Stanislav Malyshev, Zend Products Engineer stas@zend.com http://www.zend.com/

Stanislav Malyshev

19 years ago
> It always returned length-1, manual says length-1, so suddenly in > version 5.2.1 it starts returning length instead. And now imagine
Didn't notice - it is about HEAD not 5.2. It's less critical then but still as bad. People moving to PHP 6 would have enough things to worry without changing functions semantics and things like that is exactly what would make adoption of new versions slow and painful.
-- Stanislav Malyshev, Zend Products Engineer stas@zend.com http://www.zend.com/

Richard Quadling

19 years ago
Just because it has always done it doesn't mean it always will. That's what BC's are about. If this was to be a BC, having it mid version is not a good idea. Maybe for V6 where a whole LOAD of things are going to change, making it another part of the clean up process would be a better option. On 14/11/06, Stanislav Malyshev <stas@zend.com> wrote:
> > It seems utterly ridiculous to return maxlen-1. > > > > Whilst this is sure as hell a BC, it should work "properly". If I ask > > for 10, I want 10. I wouldn't have asked for 10 otherwise. Having to > > know the "magic" to get things to work is plainly bad magic. > > It always returned length-1, manual says length-1, so suddenly in > version 5.2.1 it starts returning length instead. And now imagine > somebody who tries to use this function in his scripts. He should start > adding version checks to each invocation of this function or what? > Doesn't seem very nice behavior to me. > > > Make the break. Deal with it. It was wrong and now it is fixed. > > Yes, and the way to deal with it is to restore the function to its > previous functionality, according to documentation and the way it always > worked. If you need exact number, you can always use fread. There's > absolutely no reason to break 100% of working scripts (since nobody uses > the new semantics and everybody uses the old) just to satisfy someone's > sense of purity. > -- > Stanislav Malyshev, Zend Products Engineer > stas@zend.com http://www.zend.com/ > >
-- ----- Richard Quadling Zend Certified Engineer : http://zend.com/zce.php?c=ZEND002498&r=213474731 "Standing on the shoulders of some very clever giants!"

Stanislav Malyshev

19 years ago
Richard Quadling wrote:
> Just because it has always done it doesn't mean it always will. That's > what BC's are about.
BC, as far as I know, is "backwards compatibility". It is *exactly* about "it has always done so and it always will". And unless there's a *very* good reason not to do it, so it should be.
> If this was to be a BC, having it mid version is not a good idea. > Maybe for V6 where a whole LOAD of things are going to change, making
So let's add one more, just for the fun of it?
> it another part of the clean up process would be a better option.
There's noting more "clean" in new way than in old way - it's a matter of convention. If might be not an ideal one, but having two conventions is much worse than having one non-ideal convention - you can easily deal with latter and you will really hate the guts of the language developers for the former, once your scripts start to break.
-- Stanislav Malyshev, Zend Products Engineer stas@zend.com http://www.zend.com/

Ilia A.

19 years ago
On 14-Nov-06, at 3:34 AM, Stanislav Malyshev wrote:
>> It seems utterly ridiculous to return maxlen-1. >> Whilst this is sure as hell a BC, it should work "properly". If I ask >> for 10, I want 10. I wouldn't have asked for 10 otherwise. Having to >> know the "magic" to get things to work is plainly bad magic. > > It always returned length-1, manual says length-1, so suddenly in > version 5.2.1 it starts returning length instead. And now imagine > somebody who tries to use this function in his scripts. He should > start adding version checks to each invocation of this function or > what? Doesn't seem very nice behavior to me.
The comparison Tony made was between <=5.X and PHP 6, I do not believe anyone is suggesting to modify the 5.2.x behavior. Ilia Alshanetsky

Richard Quadling

19 years ago
On 14/11/06, Ilia Alshanetsky <ilia@prohost.org> wrote:
> > On 14-Nov-06, at 3:34 AM, Stanislav Malyshev wrote: > > >> It seems utterly ridiculous to return maxlen-1. > >> Whilst this is sure as hell a BC, it should work "properly". If I ask > >> for 10, I want 10. I wouldn't have asked for 10 otherwise. Having to > >> know the "magic" to get things to work is plainly bad magic. > > > > It always returned length-1, manual says length-1, so suddenly in > > version 5.2.1 it starts returning length instead. And now imagine > > somebody who tries to use this function in his scripts. He should > > start adding version checks to each invocation of this function or > > what? Doesn't seem very nice behavior to me. > > The comparison Tony made was between <=5.X and PHP 6, I do not > believe anyone is suggesting to modify the 5.2.x behavior.
Absolutely. This should be a V6 tidy-up.
-- ----- Richard Quadling Zend Certified Engineer : http://zend.com/zce.php?c=ZEND002498&r=213474731 "Standing on the shoulders of some very clever giants!"

Stanislav Malyshev

19 years ago
> The comparison Tony made was between <=5.X and PHP 6, I do not believe > anyone is suggesting to modify the 5.2.x behavior.
Yes, I noticed later it's PHP 6, but it does not change a thing - it's still totally unnecessary BC break for the sake of meaningless purism. It adds nothing to the language or functionality and would break a lot of code. It should be fixed to be compatible with previous versions.
-- Stanislav Malyshev, Zend Products Engineer stas@zend.com http://www.zend.com/

Pierre Joye

19 years ago
Hello, On 11/14/06, Stanislav Malyshev <stas@zend.com> wrote:
> > The comparison Tony made was between <=5.X and PHP 6, I do not believe > > anyone is suggesting to modify the 5.2.x behavior. > > Yes, I noticed later it's PHP 6, but it does not change a thing - it's > still totally unnecessary BC break for the sake of meaningless purism. > It adds nothing to the language or functionality and would break a lot > of code. It should be fixed to be compatible with previous versions.
I have to agree, it is purely a cosmetic change. "Renaming" the argument can make the purists happier. However, as a counter argument, a quick search in codesearch/koders show that very few people rely on this behavior. --Pierre

Antony Dovgal

19 years ago
On 11/14/2006 08:56 PM, Pierre wrote:
> Hello, > > On 11/14/06, Stanislav Malyshev <stas@zend.com> wrote: >> > The comparison Tony made was between <=5.X and PHP 6, I do not believe >> > anyone is suggesting to modify the 5.2.x behavior. >> >> Yes, I noticed later it's PHP 6, but it does not change a thing - it's >> still totally unnecessary BC break for the sake of meaningless purism. >> It adds nothing to the language or functionality and would break a lot >> of code. It should be fixed to be compatible with previous versions. > > I have to agree, it is purely a cosmetic change. "Renaming" the > argument can make the purists happier. > > However, as a counter argument, a quick search in codesearch/koders > show that very few people rely on this behavior.
I don't think the number of people relying on this behaviour changes the fact that it's a BC break.
-- Wbr, Antony Dovgal

Pierre Joye

19 years ago
Hello, On 11/14/06, Antony Dovgal <antony@zend.com> wrote:
> On 11/14/2006 08:56 PM, Pierre wrote: > > Hello, > > > > On 11/14/06, Stanislav Malyshev <stas@zend.com> wrote: > >> > The comparison Tony made was between <=5.X and PHP 6, I do not believe > >> > anyone is suggesting to modify the 5.2.x behavior. > >> > >> Yes, I noticed later it's PHP 6, but it does not change a thing - it's > >> still totally unnecessary BC break for the sake of meaningless purism. > >> It adds nothing to the language or functionality and would break a lot > >> of code. It should be fixed to be compatible with previous versions. > > > > I have to agree, it is purely a cosmetic change. "Renaming" the > > argument can make the purists happier. > > > > However, as a counter argument, a quick search in codesearch/koders > > show that very few people rely on this behavior. > > I don't think the number of people relying on this behaviour changes the fact that it's a BC break.
Indeed no, but I only said that the affected users can be small. But I don't really worry about this issue, if we cannot break such things in major versions, when can we do it (this one or another)? For the record, I'm in favour of changing the name of the argument and restore the old behavior, problem solved. --Pierre

Richard Lynch

19 years ago
On Tue, November 14, 2006 11:56 am, Pierre wrote:
> However, as a counter argument, a quick search in codesearch/koders > show that very few people rely on this behavior.
I'm not sure you haven't biased your sample in your test to include only more proficient scripters in the first place... [Or possibly LESS proficient scripters. LOL] The Google results posted previously would seem a bit more balanced for the input sample, imho... Not sure that makes a difference, but worth considering.
-- Some people have a "gift" link here. Know what I want? I want you to buy a CD from some starving artist. http://cdbaby.com/browse/from/lynch Yeah, I get a buck. So?

Richard Lynch

19 years ago
On Tue, November 14, 2006 2:34 am, Stanislav Malyshev wrote:
>> It seems utterly ridiculous to return maxlen-1. >> >> Whilst this is sure as hell a BC, it should work "properly". If I >> ask >> for 10, I want 10. I wouldn't have asked for 10 otherwise. Having to >> know the "magic" to get things to work is plainly bad magic.
It should have been 10, but a zillion scripts are relying on it doing what it does. Changing something this fundamental, and simple, at anything less than a major release point is insane. Even at a major release, it seems like a rather pointless change -- It will only result in millions (literally) choosing not to adopt the new version. Renaming "maxlen" or whatever it is in the docs and variable name to something less clearly defined as "10 means 10" seems like a much better solution.
-- Some people have a "gift" link here. Know what I want? I want you to buy a CD from some starving artist. http://cdbaby.com/browse/from/lynch Yeah, I get a buck. So?

Antony Dovgal

19 years ago
On 11/14/2006 11:24 AM, Richard Quadling wrote:
> As we are often told on this list, PHP is not C. > > It seems utterly ridiculous to return maxlen-1.
It also might seem ridiculous to have strlen() and str_replace(), but there are good reasons to have strlen() instead of str_len().
> Whilst this is sure as hell a BC, it should work "properly". If I ask > for 10, I want 10. I wouldn't have asked for 10 otherwise. Having to > know the "magic" to get things to work is plainly bad magic. > > In terms of KISS, 10 means 10.
In terms of KISS BC means "backward compatibility". If you don't want BC - release your own PHP.
> Unless PHP !== KISS therefore 10 === 9. > > Make the break. Deal with it. It was wrong and now it is fixed.
"It works this way since PHP3" means "it works right" even if there was a bug in PHP3. That's because thousands of people could rely on this behaviour (which is, I repeat, very well documented and pretty much expected).
-- Wbr, Antony Dovgal

Richard Quadling

19 years ago
On 14/11/06, Antony Dovgal <antony@zend.com> wrote:
> "It works this way since PHP3" means "it works right" even if there was a bug in PHP3. > That's because thousands of people could rely on this behaviour (which is, I repeat, > very well documented and pretty much expected).
And so why are we losing register_globals? For a LOT of code they work and removing rg is sure as hell a BC for a lot of code. And we move forward without it (along with magic_quotes and other dead wood). I think this is just another one of those "yeah, well, we made a decision a LONG time ago and now we are changing it" for a more consistent approach. Just because it was written down, doesn't mean it is right. (Bible/koran/etc being perfect examples!)
-- ----- Richard Quadling Zend Certified Engineer : http://zend.com/zce.php?c=ZEND002498&r=213474731 "Standing on the shoulders of some very clever giants!"

Christian Schneider

19 years ago
Richard Quadling wrote:
> And so why are we losing register_globals? For a LOT of code they work > and removing rg is sure as hell a BC for a lot of code. And we move
This was done for *good* reason. You don't gain a lot of security (or even code clearity/brevity) by changing fgets. That's the difference. In favour of reverting, - Chris

Zeev Suraski

19 years ago
Richard, Really, this has been discussed so many times. Backwards compatibility breakage accumulates. It's not a binary. "If we break something we can break everything since it's broken anyway" is not very convincing. The more you break the worse things are, the more work you have to do in order to migrate. Therefore, whatever breakage we have in any one version has absolutely nothing at all whatsoever in any way shape or form in any jurisdiction on this planet or otherwise to do with introducing another BC break. Zeev At 12:19 14/11/2006, Richard Quadling wrote:

bertrand Gugger

19 years ago
Bonsoir, Antony Dovgal wrote:
> Hello all. > > I'd like to continue recent IRC discussion here, to draw more attention > to this issue. > > At the moment functions fgets() and fgetss() are broken in HEAD, as they > return <maxlen> characters instead of <maxlen - 1>, as they do in 5.2 > (and I'm pretty sure this is the way they used to work since their very > first appearance in PHP). > So, anybody relying on fgets($fp, 10) to return 9 characters would get > 10 characters instead. > > Yes, this behaviour is not very intuitive for those who are not familiar > with fgets() in C, but it's well documented and pretty much expected. > > I believe this is an unreasonable BC break and I strongly believe it > should be fixed. > There are 0 reasons to do it (without any discussion) and it can only > result in more users' confusion, even if the original purpose was the > contrary. >
May I say that this thread looks very funny and that poor people wonder what php will be ? Or I am a troll.
-- toggg

Antony Dovgal

19 years ago
The issue is now solved, thank you. EOD On 11/14/2006 12:38 AM, Antony Dovgal wrote:
> Hello all. > > I'd like to continue recent IRC discussion here, to draw more attention to this issue. > > At the moment functions fgets() and fgetss() are broken in HEAD, as they return <maxlen> > characters instead of <maxlen - 1>, as they do in 5.2 (and I'm pretty sure this is the way > they used to work since their very first appearance in PHP). > So, anybody relying on fgets($fp, 10) to return 9 characters would get 10 characters instead. > > Yes, this behaviour is not very intuitive for those who are not familiar with fgets() in C, > but it's well documented and pretty much expected. > > I believe this is an unreasonable BC break and I strongly believe it should be fixed. > There are 0 reasons to do it (without any discussion) and it can only result in more users' confusion, > even if the original purpose was the contrary. >
-- Wbr, Antony Dovgal