« first day (3902 days earlier)      last day (1029 days later) » 
01:00 - 20:0020:00 - 00:00

1:45 AM
@IluTov I actually have another RFC... add a param to the array sort functions so that they return the result instead of sorting in place, they all currently just return true...
 
2:28 AM
PDOStatement::fetchAll() should return array|false ・ PDO Core ・ #81189
 
 
4 hours later…
6:04 AM
\o
 
6:29 AM
@Girgias Arguments to change returning behaviour is usually a red flag.
 
Good Mornig!
@Derick is it expected that src/coverage/branch_info.c is BSD 2 while all other sources of Xdebug are Xdebug 1.01 ?
 
Yes, but I don't remember the reason for it :-)
 
btw, still BSD, thanks
 
fwiw, src/lib/compat.c is also BSD (but 3-clause)
 
yes, and run-test is PHP
/me hates license reviews :(
 
6:39 AM
The headers should really also say "Xdbeug 1.02", which is what is in the LICENSE file
But that's just the xdebug.org to xdebug.org change, so not a ... real issue.
 
6:51 AM
@Girgias As Derick said, I think parameters changing what type is returned is suboptimal. E.g. that can be very annoying for static analysis. We could introduce another variant for each of the ones we have but is that worth it?
 
7:16 AM
@IluTov I suggest that it should be called sort2() :D Actually, we have precedence for a similar naming convention in snmp: e.g. snmpget(), snmp2_get(), snmp3_get() :D
 
7:41 AM
@PatrickAllaert alpha2 will be your ?
 
7:56 AM
i checked in with my named parameter alias RFC again and one of tysons comments let me stumble about a concept i havent investigated before (cache_slot for function arguments). since there is a cache_slot being set on the first call, my code here is probably completly wrong? what are cache_slots for and how do they work? ref: github.com/php/php-src/pull/6522
 
8:34 AM
@RemiCollet 7.4.21 is next week, right?
 
@Derick yes on 8.1.0alpha2 this week
(so, some release every week this summer)
 
that's nice to have it separate - less chances for conflicts. We'll have to fix that for GA though.
 
8:50 AM
indeed, (less work each week), and will have to be synced for GA (if security fix)
 
9:11 AM
must ... resist ... ranting about JSON5 ...
 
.... there's a JSON5? ... oh god why
 
it's the kitchen sink of complicated things you could add to JSON
I love that the authors thank Douglas Crockford for the clear description of JSON, and then present a spec that has to cross-reference the ECMAScript spec to define a bunch of reserved words
 
It's YAML with extra steps?
 
Ah, so there is hope that eventually they all will see their erroneous ways and head back into the light that is XML.
 
might as well go full SGML at this point :P
 
9:20 AM
@Gordon Yes because two completely separate but the same ways of representing data, combined with both meaningful and meaningless whitespace is the way we want to go :P
 
Or just use a programming language.
 
@MarkR hey, it works for Python, too
 
@MateKocsis do you (or anyone) have some tools for bulk updating PHP manual pages where the parameter names need updating?
 
gen-stubs.php has a mode for generating the docbook synopsis, I think; don't know if anyone's written a script to apply that to existing pages
 
9:36 AM
:D
 
10:05 AM
@Danack Yep, I've been using gen_stub indeed. An example: ./build/gen_stub.php --replace-methodsynopses ext/imap ../doc-en/reference/imap
 
10:16 AM
@MateKocsis cool, thanks.
 
10:45 AM
Hey o/
 
11:05 AM
2 crashes(heap-buffer-overflow,SEGV) 5 memory leaks ・ *General Issues ・ #81190
 
11:53 AM
Hey o/
 
12:08 PM
Hello
 
12:23 PM
@beberlei It's been a while since I've looked at the code but from skimming, zend_alloc_cache_slots will increase cache_size of the oparray which determines the additional memory the VM will allocate for each function. The return value is the current offset to whatever slot in the cache was just allocated. The cache is general purpose, what exactly it does depends on the instruction.
Oh and the offset to the cache slot is usually stored in the extended_value of the instruction.
 
Morning
 
1:06 PM
I wonder if using a phar installer is a good idea
 
Mogogmring
 
@Derick do i have to set something special in xdebug.ini to make it work with multiple long running scripts being started with xdebug.start_with_request=yes? i have everything hanging now waiting for something to proceed, PHPStorm shows a checkmark at the breakpoint that i set, so communication between VM and phpstorm did work
 
@PeeHaa Hey Peter o/
 
Yoooooo o/
 
yohaa
 
1:14 PM
Yesterday I found out that Servus means Servant in latin, interestingly enough, Servus is a bavarian salute
LATIN LATIN EVERYWHERE
 
1:34 PM
@RemiCollet Later today, with @ramsey, maybe @JoeWatkins too
 
1:54 PM
Could array to string conversion become a TypeError in 8.1 or does that have to go along with a major-version change?
 
That probably has to be a major, I think.
 
fooey
 
@MatthewBrown wiki.php.net/rfc/engine_warnings Many errors have just been increased. Even if we don't wait for 9.0 I guess it should at least wait 1-2 more years.
@MatthewBrown Although there are no hard rules. You can always suggest it and see what kind of reaction you get.
 
Just got bit by an array-to-string conversion bug is all
 
@IluTov I'd dispute "no hard rules"; although "breaking change" is hard to define, the policy is pretty explicit
 
1:59 PM
@IMSoP Actually, the whole things is a gray area because we have breaking changes in minor versions all the time.
 
we should at least try to follow our own policy though
 
So it's up to the voters to decide if it's ok or not. At least that's how it works in practice. You're probably right that the rules say something else.
 
it's generally agreed that making "too few args" an error in 7.1 was a mistake
 
@beberlei There is a connection limit in PhpStorm, default set to 3.
 
Well there's tons of things that are way less severe and it would be a burden to the project to delay them for 4 years just because of a theoretical BC break. And there are things in between, like adding keywords. If I had to wait 4 years to land something I don't think I'd want to work on that project.
 
2:02 PM
sure; but making a warning an error is very explicitly a breaking change
 
Completely unrelated to this discussion, but PHP Tags matter
 
@beberlei If you check Xdebug's log, you'll likely see that the connections are made, but then there it stops. What PhpStorm ought to do, is to accept the connection and then send a detach with a message that its "concurrentl debugging sessions" window is full.
 
@IMSoP Yes, you're right.
I'm just saying the rules should be there to help us, not cripple us. If the guidelines say no BC break is acceptable in any minor version I'd say let's change the guidelines.
 
2:20 PM
no, the rules are there to help users; they shouldn't "cripple" us, that's true, but they are deliberately limiting what we do
incidentally, we don't have any policy for how often we bump the major version, so releasing 9.0 next year would be more in keeping with our published policy than deliberately including breaking changes in 8.2 just because we were impatient
 
@IMSoP Users don't really care if your version is a major or minor version. Bumping major versions more often won't change their mindset that things should just work. I'm definitely with you we should try to make that happen as much as we can. But we already have a super slow release schedule and I don't want to make contributing even harder.
 
as a maintainer of a sprawling legacy code base, I just don't agree; having an expectation of how much is going to break is huge, and a new release every year is plenty to keep up with
I don't think that's "super slow" at all
the thing that's super slow is adoption by enterprise linux distros on the one side, and cheap web hosts on the other
 
@Derick So you are telling me its phpstorms fault? Do you know of an issue in can upvote?
 
@beberlei I don't want to blame anybody without knowing enough about the issue... but you can just up the number?
Such for "simultaneous" in settings, and change the number from 3 to 100?
I don't think there is an issue
 
@ramsey don't seem to be able to join
whoever has the volume remote for internals, please turn the volume down, then destroy the remote ...
 
2:38 PM
@IMSoP "super slow" ? I think have changed compared to 10 years ago
RHEL 7 was released with 5.4 but 5.5, 5.6, 7.0, 7.1, 7.2,, 7.3 was added less than 1 year after GA
RHEL 8 was released with 7.2 but 7.3 and 7.4 were added.... (and probably 8.0 will be there in the next months)
I don't think this is "super slow" (for an enterprise "stable" distro)
 
@RemiCollet true; you, and Ondrej on the Ubuntu side, do great work making things available, so I am probably being unfair
 
@Derick where can I set this though?
 
of course was speaking of "offirical" packages (not about my repo which of course already have 8.1)
 
@Derick found it, thank you, just needed to know this was a thing
 
@IMSoP I never said we should unnecessarily break things. BC breaks are carefully considered for every RFC. I said limiting all BC breaks to major versions would be bad for the project. And I even said above that I would not support the proposed change. I'm not sure why you're out to argue.
 
2:44 PM
the original comment I responded to was "there are no hard rules"; I would say that there are hard rules, but there can be times when it's justified to break them
 
@beberlei Yeah, does this solve your issue? I can then test this and make them a new ticket.
 
just a different framing, I think: it should always be about making a case for why something needs to be an exception to the rule
 
I think the question hinges on what qualifies as a BC break. "Added a new keyword that is unlikely to be used already as a constant" is a BC break, but so is "we moved the return type of a function to the left instead of the right." They're both BC breaks, but the impact of the former is waaaaaay less than the impact of the latter.
 
@Derick its only possible to set to 20 and my VM is just doing too much. but i can probably find another way, is it possible to start the debugger programatically in code?
 
@MatthewBrown PHP doesn't follow semver. So theoretically it could, but it's really late in the process for rfcs that are likely to be contentious. Maybe try using PHPStan to detect those types of errors?
 
2:47 PM
That requires a judgement call to say if it's a major-level or minor-level or never-level change.
 
@Danack not semver, but we do have a release policy that explicitly says "Backward compatibility must be respected with the same major releases, for example from 5.2 to 5.6."
 
and yes, that absolutely hinges on defining "breaking change", and xkcd.com/1172 applies in spades
 
> ABI and API can be broken (internals), src compatibility should be kept if possible, while breakages are allowed
 
@beberlei alternatively, use a browser extension to only trigger requests when you need to ? Of if this is on the CLI, only set the XDEBUG_SESSION or XDEBUG_TRIGGER env var for the requests you want to debug?
 
2:50 PM
@IMSoP Do you have a link to the policy? This is the only thing I could find: php.net/manual/en/about.phpversions.php "Typically PHP only adds new features to major and minor releases, and fixes bugs in point releases. However, this convention is not always true." That's very vague.
 
@IMSoP Thanks
 
@Derick no i mean in a long running worker script, my idea was to connect / disconnect to the IDE on each worker loop run
i can disable the debugger for my worker scripts for now
 
@beberlei No, I don't think so.
I suppose it's all the same process/script?
 
i made a note to open a feature request in the future
 
2:52 PM
@Danack that's talking about internal and extension changes; the two bullets above very explicitly say that userland code should not break
there are absolutely good reasons not to be overly pedantic about it for things like reserving keywords; but changing a common warning to an error would be against both the letter and the spirit of that policy, and would need some very specific reason it was urgent
 
@IMSoP I just wonder if the policy isn't worth revising if it's regularly disregarded (meaning every minor version). That could be of benefit to you as well (e.g. it needs to be shown that there aren't more than n instances of code breaking in the top 1000 repositories). That automatically puts more emphasis on BC and thus voters will make a more conscious decision.
 
I think having a clearer definition of the kinds of breaking change would be useful, if someone can come up with one
but I don't think scrapping the distinction between minor and major is in anyone's interests
 
3:12 PM
Since when do we do our methodsynopsis multiline?
 
thanks <3
 
@beberlei Can you ping me once you've done with the ID, so I can add my thoughts?
 
@Derick yes i am a bit swamped right now, so it might take a few
 
3:47 PM
@Crell Uh, the query builder discussion desperately needed your reply... :D Thanks!
 
I figure I have rare expertise on the subject, so my ability to laugh at someone's face is useful.
 
Howdy! I've got similar data in two small arrays, and I want to display them in a table side-by-side. I'm not sure how I would go about it, maybe using array_pop? I couldn't find an article covering this topic but maybe I just didn't look hard enough
It makes sense to me to go back and forth between the two arrays, but I'm not sure how I would do it in a loop
 
So you want:

A[0] | B[0]
A[1] | B[1]
A[2] | B[2]

Something like that?
 
Yep, the arrays may not be the same size though
 
Well there's the tricky part. :-) Are they numeric or name-keyed?
 
3:54 PM
Numerically keyed, the keys aren't important either
I was going to use the method shown at the following link, but I'm not how to format a loop structure for two arrays; c-sharpcorner.com/UploadFile/d9da8a/…
 
You can probably use something like this then (pseudo code):

$count max(count($a), count($b));
$a = array_values($a);
$b = array_values($b);
for ($i = 0; $i < $count; ++$i) {
  print $a[$i] ?? '';
  print " | ";
  print $b[$i] ?? '';
}
Modify to format the table as you prefer, of course.
And newlines, etc.
 
That's smart, thanks!
What do the "??" mean? I googled it but I couldn't find anything
 
Remember the array_values() part, because arrays are intrinsically unreliable.
That's the null coallesce operator. It's short hand for is_null($val) ? $default : $val
So if $a[$i] doesn't exist because that array is shorter, use the '' instead.
 
Ohhh okay, thanks for the help, and something new to learn!
 
cmb
4:04 PM
see also 3v4l.org/D4TTj
 
Thats an interesting way to tackle it too, thanks!
 
array_map is a bizarre beast in PHP...
 
cmb
array_map(null) is zip()
 
:exploding head emoji:
 
🤯
 
4:10 PM
array_map makes me wonder why array_walk exists, maybe just to save assignment operators
 
I think you need it for key manipulation.
Or something like that. There's some edge case where you have to walk instead of map, but it's rare.
 
4:24 PM
@Crell did you get any feedback on your blog post ?
 
Levi had a few minor comments that I either tweaked for or responded to. He didn't say if it was actually compelling for him, though.
 
I'm reluctant to suggest any actual changes ... I don't know how well it makes the case to an outsider ... but when I saw this "Complexity and spooky action at a distance are killing us."
 
Your input is also welcome. I feel like I'm struggling to justify partials without leading into pipes, because basically all the use cases I am interested in involve both.
 
it feels like you're drawing attention to the drawbacks of pa ... complexity and spooky action at a distance pretty much describes pa ...
 
I think you're talking about a different kind of complexity.
 
4:26 PM
@Crell I never got a link for feedback, FWIW
 
that's the other thing, I don't know that most of the objection is only because of language complexity ... I don't think that's nikitas main concern for example ... he keeps saying "especially when you look at the implementation" ...
 
You seemed tepid on the idea. :-)
@JoeWatkins It's difficult for me to comment on the implementation, since that's all your territory.
 
anyway, I'm referring to both kinds of complexity there ...
 
@Derick docs.google.com/document/d/… - If you are still interested.
 
I kinda agree with derick and others that tying this to pipes is probably not good for it, pipes is a battle all it's own ... but I don't have good use cases, like I said before, I'm most interested in seeing what people do with it, and I don't know what those things are ... and I don't have too ...
 
4:29 PM
@Crell Also, FWIW what convinced me to say no, was basically Nikita's assessment that there are a lot of edge cases - some known, some unknown.
 
@SlamJammington map creates an array to return. walk doesn't.
 
@Derick fwiw, that's what he wanted it to do, as quick as he pointed them out I fixed them, there are a couple of outstanding issues ... but he made it sound worse than it is ... he wasn't really asking questions about how strict types works, he already knows, but he wanted you to think it might be unknown ...
that's what I see anyway ... which is quite annoying ...
 
It's more the unknown edge cases... which aren't know yet ;-)
 
Is that more true of partials than any other RFC?
We don't know what we don't know about anything.
 
yes
@Derick I'm an optimist, and think we can find them ...
I'm glad everyone is thinking properly whatever ... I wish they'd thought properly in the past, on much more complicated, and much more useless proposals, but whatever ...
 
4:33 PM
Hmm... looiks like alpha2 is broken
 
oh noooo
@ramsey @PatrickAllaert
 
working on a quickj fix
 
they should be around, I was in a chat with them 20 minutes ago, but had to leave them to finish up ...
 
@Derick So, I don't really know how to respond to unknown unknowns. We can say that the use cases are present, and the test coverage is good, but I'm not sure what to say about unknown unknowns. I think Joe already addressed the known complexity questions, as he noted.
 
Only developers will find them, and might send in bug reports :-)
 
4:37 PM
@Crell most rfcs tend to extend some existing functionality ... the engine does not want you to modify functions in the way we're modifying them, almost every branch in code literally exposes edges, it's undeniably a complicated thing ... and I've no doubt there'd be edges left that we can't see, but we'd eventually find and fix them ...
 
@JoeWatkins I still don't know how to address that other than "I trust Joe". :-)
 
not just me, nikita is ready to work on it if he has too, he's already poured over it many times for many hours ... we'd continue to do that ...
 
Point remains, I don't know how else to respond to that other than "yay, smart people."
 
I'm annoyed that this is a problem now, I mean seriously ... we're talking about edges in zend engine code, having just merged whole bunches of code from two other projects, boost and dynasm, with absolutely no clue about how they work - not even aaron understands the boost stuff ... ignorance is bliss I guess ...
 
Bikesheds...
 
4:42 PM
@ramsey @PatrickAllaert @JoeWatkins github.com/php/php-src/pull/7185
 
Still here
 
you'll want to remake that tag between you
 
alpha2 is OK if you build everything static, but will be broken for shared ext (at least phar)
 
I've really gotta go afk now, ping me when resolved, I'll check ... it's okay, this is why we tag early - so remi can tell us we done it wrong :)
 
Will redo the tarballs
I am afraid @ramsey
... is not here anymore
 
4:51 PM
@PatrickAllaert do you take care of re-tag ?
or have we to wait for @ramsey ?
 
@RemiCollet Busy doing it
 
thanks
 
I'm here
@PatrickAllaert, are you re-doing the tarballs?
If not, I can do a new tag and new tarballs
 
New tag already pushed
tarballs almost created
 
great
thanks!
I'll take down my tarballs and the manifest, so there's no confusion
 
5:05 PM
@ramsey, @JoeWatkins, @RemiCollet new tarballs uploaded and mail sent
 
@PatrickAllaert thanks (QA build running)
 
What's up with the failures on ARM? travis-ci.com/github/php/php-src/builds/230424561
 
@RemiCollet I guess we should have some shared ext being built as part of our CI. We only do static builds?
 
yes, I think all CI build only use static ext.
and indeed have 1 build with most ext. build shared make sense IMHO
 
cmb
AppVeyor builds many extensions shared (but not phar)
 
5:20 PM
@ramsey Failed test: ext/standard/tests/file/is_file_variation3.phpt
curiously, in the build related to the commit just before: travis-ci.com/github/php/php-src/jobs/517697606 there is no occurrences to that test at all
 
that's not the only failure I see
Nevermind... maybe I'm also seeing the expected failures
I see the summary at the bottom only shows that as the 1 failure
 
All the other ones that fails are marked "XFAIL"
looks like while running that test, is_file(" ") returned true
that test file pretends testing invalid values, however a file named with a single space is not an invalid value
```
$ php -r 'var_dump(is_file(" "));'
bool(false)
$ touch " "
$ php -r 'var_dump(is_file(" "));'
bool(true)
```
so only explanation is that there might be (random?) cases where a file named " " exists when that test is being triggered
 
5:38 PM
interesting
 
Filed a PR for it => github.com/php/php-src/pull/7186
maybe another test tries to **create** so called invalid files and succeeds?
The fact that the tests run in parallel might bring some randomness.
 
Yeah. Possibly. That was added to the test 13 years ago, so something might have changed in the meantime to introduce a filename with a space (as part of some other test). github.com/php/php-src/commit/…
I've been trying to think through it and figure out whether a space character should still be tested, but if that's a valid filename, then is_file() shouldn't always return false. So, I guess removing it from the test is the right approach.
 
6:09 PM
@Girgias Dan mentioned you started working on function autoloading? Did you get anywhere? @Danack Is my assumption correct that you'd do the specification for this one? Either way, I don't think there's enough time for 8.1.
 
@IluTov Well I looked into it, but Anthony's branch is so outdated that it is kinda unusable, BUT since support for __autoload() has been removed it should be simpler to implement as you could do more of copy/pasting
 
@Girgias Ok cool.
 
There might be time for 8.1 if you put up an RFC and open discussion today ;-)
 
From what I recall, the SPL autoloader already seems to mostly hook into core... but would need to have another look
And offsets have been on my mind currently
 
@ramsey Unless @Danack has already something very concrete written down it's not happening :)
 
6:13 PM
Fair enough
So, the issue here is that the core needs to be changed so that invoking a function that doesn't yet exist in the symbol table causes it to trigger the autoloader?
 
I think also we want it to be a bit more flexible and also have typedefs ready to be hooked into
 
Hi all, I'm building a command line tool, library package. However, my package console isn't available when I install it as a dependancy. if I run vendor/bin/symfonic it returns zsh: no such file or directory: vendor/bin/symfonic any suggestions?
 
@Girgias Out of all the suggestions yours seems like the most realistic (implicit bool -> string coercion deprecation). I'll look into that. But might be too late even for that one.
 
From what i recall from Dan's RFC, there would be "type" argument on what thing this autoloader should autoload, so class, function, and future type defs, so that Types wouldn't need to hook into the class autoloader
 
@KerrialBeckettNewham Did you check whether the file exists?
 
6:17 PM
@KerrialBeckettNewham Is the file there when you look in vendor/bin?
 
@IluTov I can spend some time writting the RFC if you want to. :-)
 
Did you set it up as binary in composer?
 
@Girgias Hey I won't say no to that!
 
Welp, I suppose it's time to open up the text editor
 
6:22 PM
@ramsey I defined a bin inside the composer.json like so"bin": ["bin/symfonic" ] but the files doesn't exist in the vendor/bin folder.
 
@KerrialBeckettNewham And you have bin/symfonic located at that path in your project?
 
I have a manager who thinks end-user test should come before unit test
 
@Girgias Awesome! Thanks for the help. Let me know if you want me to proof-read. Have to go now but I'll start with the implementation later or tomorrow.
 
When I require your package, `vendor/bin/symfonic` exists, but attempting to run it gives me this error:

```
PHP Warning: require_once(/Users/ramsey/Desktop/test/vendor/symfonic/symfonic/bin/../vendor/autoload.php): Failed to open stream: No such file or directory in /Users/ramsey/Desktop/test/vendor/symfonic/symfonic/bin/symfonic on line 5
```
 
6:25 PM
@SalOrozco Does he also think you should put your underwear on after your trousers?
 
lol
 
@KerrialBeckettNewham So, you need to fix how it finds the autoloader
 
That's exactly what I was telling her.
 
@ramsey An update to try and fix it must have broken it. thanks🙏
 
@KerrialBeckettNewham Since it's in the vendor/bin directory when called, __DIR__ gets resolved to that path, so this is having problems: require_once(__DIR__ . '/../vendor/autoload.php');
 
6:28 PM
I have a few options I guess. Try to get the managers to understand why testing is important or find a new job.
 
@KerrialBeckettNewham You might consider looking at how other projects solve this problem, like here: github.com/sebastianbergmann/phpunit/blob/master/…
 
@ramsey Haha, already did, found a solution :)
 
I used to laugh at people constantly re-inventing the wheel in the JS world, forever doomed to ignore and thus repeat the failures of the past
I didn't expect magic quotes 2.0 to come around so quickly in php.
 
That's... not an entirely incorrect summary.
 
6:49 PM
I don't know how I feel about is_trusted()
I think a better option might be to specify that the parameter must be a literal, as part of the type defintion, i.e., function foo(string literal $param) {}
I saw that was mentioned in the RFC as future scope, but I think it's probably the right approach
But I suspect @Stephen might be referring to the "SQL Object Model Parser & Sanitizer" thread, rather than the is_literal thread :-)
 
leaving as future scope eases adoption, and helps it to penetrate the ecosystem ... if we introduce first class support right off the bat, nobody will use it until that version of PHP is in very wide circulation ... by having second class support, we allow drupal/wordpress/whatever to start using it right now ...
 
I think is_trusted() is a bad name for it
 
it's terrible
 
@ramsey actually kind of both, but mostly it stems from the is_literal that isn't literal and is_trusted that shouldn't be trusted.
it's basically like a van in a parking lot with "not a pedo" written on the side.
 
(I'm just writing code, not running the show)
 
6:54 PM
integers can pass int type hints, so I don't know why the function needs to expand pass string literals
but I do see the value of being able to determine whether the value passed was a literal
 
@ramsey so do I, I don't have an issue with the original basic premise
 
is literal is also a bad name
 
but it's less bad than is_trusted
 
im not keen on the name but, what @ramsey said^
 
a lot of people voted for is_trusted in a straw poll and craig was just responding to that feedback ...
 
6:56 PM
I know.
 
I presume the reason for wanting integers included, is so that select ... from $table name limit $limit will still be considered "literal", even if limit came from (int) $_GET['limit']..
 
the ultimate goal is to disallow input from anywhere except values provided by the "developer" of the code
 
no it isn't
 
isn't that the idea of testing for whether it's a literal?
 
@JoeWatkins the first email that introduced the RFC really misleads then:
> Distinguishing strings from a trusted developer from strings that may be
attacker controlled
 
6:58 PM
it's not passed in using a variable, so the only one able to provide it is the writer?
 
I said literal is a bad name ... things that engine concatenates at compile time, and at runtime, are not literal, but they are composed of literals, we can prove they are safe ... sorry for using the word safe ... they're as safe as the programmer wrote them ...
we want that utility, that's how people normally write SQL
 
a programmer didn't write the contents of $_GET['foo']
 
gotcha... but ultimately, compile time is the key... the string cannot be changed at runtime, right?
 
PHP can either get type information from declarations, or from context, when you use an int in a string context, it is a string
 
wellow, I'm the one who created the mess with the trusted/literal name... throw rocks here :-)
 
7:01 PM
@CraigFrancis I will throw Nerf rocks
 
Thanks, appreciated :-)
(for reference, I have lost a lot of sleep over the name)
 
what we want, is that (string) int should produce something safe to use in concat/format, and if we're going to have that, explicit support, we have to have implicit support, and if we're going to have (string) int, we should also have (int) string ... what you see as consistency, I see as purity ... it doesn't make sense to support some casts, in some directions, sometimes ...
 
I still don't really see why you need to concatenate integers in the first place, when parameterised queries are a thing. Are you suggesting that people name their tables 2 ?
 
I propose a new type for that: entish, and then we talk about it for an unreasonably long time before deciding on it ;-)
 
the problem isn't that it's casts one way or another or purity or whatever, its that you're allowing a value that came from the request to suddenly be considered "trusted"
 
7:05 PM
because that is the practical thing to do, that is how people construct strings ... it's fine to say "everyone should do X", but a bit unrealistic ... everyone should be educated to the point that they don't need this kind of guardrail, but they're not ....
 
@Stephen They do things like array_map( 'intval', $ids ) that's used in WHERE id IN... but also some do a sprintf('SELECT * FROM table WHERE id = %d', intval($id))... not ideal, but lots exist.
 
if you're too lazy to bind a variable number of parameters based on the number of IDs, you deserve to be hacked.
it's not fucking rocket science.
the more I hear you try to explain it, the more it sounds like you're designing glass that tastes better for people who lick windows.
 
we don't consider it trusted, we consider it a known quantity, because all integers up to PHP_INT_MAX are, they cannot be mutated by whatever operations you are doing, they cannot lead to an injection, they can lead to mistakes ... that is not a problem we are trying to solve
 
The first implementation was strings only... and I was happy with that, then Matthew from Psalm fame pointed out how much of his code would need to be modified for Vimeo, and it didn't need to be (and Mike has been giving me lots of WordPress examples).
 
My bad joke aside, I'd like to better understand the goal. On the surface, it sounds like something like is_numeric() suffices, or intval(), etc., but I suspect we're focusing on a tiny bit of the problem when discussing int. So, what problem does it solve?
 
7:08 PM
@ramsey Hopefully the RFC explains wiki.php.net/rfc/is_literal
@Stephen It's surprisingly common, but also not an issue (considering they are integers).
 
I'll go back and see what I missed
 
@ramsey Thanks... I would like to make sure the words there are correct (even if you skip most of the FAQs)
 
> check if a variable can be trusted to not contain an Injection Vulnerability
it doesn't really check for injection vulnerabilities, though, right?
 
Erm, you can't make them.
 
how so?
 
7:11 PM
As in, take a look at this example library... intentionally kept short: github.com/craigfrancis/php-is-literal-rfc/blob/main/…
 
something else to mention, is that we have to deal with the compiler the way it is, you probably think that "string" . 2 concatenates the string and number, but it does not
function name: (null)
L1-4 {main}() /opt/src/php-src/test.php - 0x7fddf906a000 + 2 ops
 L2    #0     ECHO                    "literal 2"
 L4    #1     RETURN<-1>              1
 
for the simple one, take a look a lines 90 and 92
 
optimized away, do you want to remove that optimization for all code ? of course not, so you have to try to be compatible with the way the compiler already treats concatenation ...
 
@JoeWatkins That's a good way of explaining it, thanks Joe
 
fwiw if you call it is_trusted you're going to get annihilated at the vote.
 
7:14 PM
@MarkR Yeah, I suspect so... got any better names? :-)
My friend Steve suggested is_potato() because he was about to go peal some potatoes... but Joe has suggested is_known() which I quite like.
 
krakjoe@Fiji:/opt/src/php-src$ cat test.php
<?php
$two = 2;

echo "literal " . $two;
?>
krakjoe@Fiji:/opt/src/php-src$ sapi/phpdbg/phpdbg -n -p* test.php
function name: (null)
L1-6 {main}() /opt/src/php-src/test.php - 0x7fc546e54380 + 4 ops
 L2    #0     ASSIGN                  $two                 2
 L4    #1     CONCAT                  "literal "           $two                 ~1
 L4    #2     ECHO                    ~1
 L6    #3     RETURN<-1>              1
[Script ended normally]
 
Or, we stretch the definition of literal, and keep with the original name.
 
Only is_hardcoded_string or is_source_string come to mind.
 
@MarkR Doesn't handle the integer problem.
 
if we don't do it this way, you get unpredictable strange behaviour, depending on how you concat ... that is not going to be acceptable to anyone ... but they likely don't know, so are focusing on totally the wrong stuff ...
 
7:17 PM
@MarkR As in, because we cannot flag which integers came from the developer, and because integers are used a fair amount, without causing a problem, we are allowing them all in.
 
Oh, interesting... so it can detect whether the value has been manipulated from its literal form? 3v4l.org/IKsY0/rfc#focus=rfc.literals
 
That's it... with the focus being dangerous strings that come from the user being incorrectly included in SQL/HTML/CLI/etc.
 
maybe is_tainted() is better?
 
sorry, taint checking already exists
 
I can appreciate the usage of them e.g. "WHERE foo=" . SOME_ENUM_VALUE, but it immediately breaks down the root premise, unless you have some way to deduce that the number itself came from source.
 
7:20 PM
nevermind... the taint ext already has this
 
Hey guys .. the open job opportunities for PHP developers are getting decreased in our country .. and golang jobs are getting increased .. do you think I have to start learing golang?
I've heared from some CTO that golang is much more powerful (supports over 50k request per sec) and uses less resources than PHP .. that's why they are migrating
 
@MarkR We can't unfortunately... and I think (and it's been requested) it will help adoption is we allow all integers ('cus they cannot cause an injection vulnerability).
 
Stanleyrex ・ *General Issues ・ #81191
 
the main problem I have with using the term "injection vulnerability" is that you can still do stuff like this and it's "trusted": 3v4l.org/EHtkH/rfc#focus=rfc.literals
 
7:23 PM
@CraigFrancis But it's not is_string_that_probably_cant_cause_an_injection_vulnerability
 
@ramsey You wrote that... if it came from $_POST or $_GET, it would be picked up.
 
right... but it's still an injection vulnerability
 
@ramsey How, it didn't come from the user?
 
so, it doesn't "check if a variable can be trusted to not contain an Injection Vulnerability."
 
@ramsey It's not injected if you wrote it
or am I missing something?
 
7:25 PM
semantics... I'm injecting it into the SQL statement
 
For it to be an injection volatility, it has to come from the user.
 
it's being sent as output to the database engine
 
I believe what Ramsey is trying to say is that if the idea is that a string is safe from injection, that's false because a user can still be stupid
 
there is no injection in that code
injection means a specific thing, not "stupid sql" ...
 
okay
 
7:26 PM
"While these values can be trusted to not contain an Injection Vulnerability, they cannot be completely safe from every kind of issue"
 
I think it depends on where you consider the "input" and "output" to live. I don't think of the web server and the database server as a closed system. They are two, separate systems. The web server receives user input, and then it outputs a SQL string to the database server. The database server receives input from the application and outputs results.
 
@ramsey Yep, and it's this functions job to make sure the SQL written on the web server does not contain user values incorrectly (i.e. no mistakes that result in an injection vulnerability).
 
But the application isn't the only thing that can send input to the database, unless you've fully locked it down within a VPC, or something like that
 
@ramsey I can't protect against things happening out side of the PHP script.
 
My point is that it doesn't explicitly check for injection vulnerabilities. That's like a side-effect of what it's doing.
 
7:30 PM
not sure I'm following.
 
@CraigFrancis you should coin a phrase “don’t trust outside input, regardless of type”
 
@Stephen Unfortunately integers are the problem... we can't do that.
 
The obsession with allowing integers is beyond bizarre given what you want to achieve.
 
@IluTov as much as I'd like it, and can have some words ready soon, it's really late for an RFC. It probably needs some feedback from the composer people and there would be 'not clever' tangents about some aspects of it.
 
You think people care enough about injection vulnerabilities that they’re going to check all the input values with this function but not enough to take the time to parameterise their queries
 
7:32 PM
@Stephen If anyone can find a problem with them, I'll happily remove them... but at the moment I can see how they help adoption.
 
Schroedingers security conscious developer.
 
@Stephen We are starting with the libraries... these kinds of mistakes: github.com/craigfrancis/php-is-literal-rfc/blob/main/…
 
@CraigFrancis I don't think I'm explaining myself very well, but it's mainly a semantic issue. I'll try again later.
 
@Stephen Drupageddon was caused by a dynamic placeholder count routine that we THOUGHT was safe. It wasn't, because of the keys.
 
@ramsey I really do appreciate the ideology of no user input, but I'd rather have a usable solution by everyone, than something that's difficult to use.
 
7:34 PM
@Crell yeah developers write bugs all the time. That doesn’t sound like a good reason to tel them “this value might be something you wrote… or something the user wrote”
 
@Crell Oh, that reminds me, Crell, the updated RFC mentions that... I ended up abbreviating it quite a bit, does the words still make sense though.
 
@CraigFrancis I'm not sure what you mean by "ideology of no user input," so I'll try to clarify what I'm saying, but I've got to run
 
@ramsey k, talk later (I'll be back online tomorrow)
 
@CraigFrancis Kind of? I don't think it's inaccurate at least, but I don't see how is_literal() would have fixed the particular issue Drupal had.
 
@Crell Theoretically, if Drupal had used a Query Builder (one of a few options), the SQL and Fields would have to be added separately... github.com/craigfrancis/php-is-literal-rfc/blob/main/…
as in, while that one is a bit primitive, it forces field escaping.
 
7:42 PM
Not really. The issue was caused by an incorrect assumption that an array was packed, not associative.
 
but those go in as fields, and would have been escaped as such?
sorry, I might mean parameters rather than fields ... but same applies, both could have a way of checking.
it requires everyone to apply these fields/parameters via a standard way of adding, which ensures they all get checked/encoded/escaped.
 
you would write:

$db->query("SELECT a FROM b WHERE id IN (:id)", ['id' => [1, 2, 3]]);

That would get expanded out to the equivalent of

$db->query("SELECT a FROM b WHERE id IN (:id_0, :id_1, :id_2)", ['id_0' =>1, 'id_1' => 2, 'id_2' => 3]);

The problem came in if the array had string keys, because then you would end up with the string key in the placeholder name, so you'd get "IN (:id_; DELETE FROM users;" or such, which is, erm, bad.
So the processed query string would have been non-literal no matter what, and the unprocessed would have been literal no matter what. There's nowhere to run is_literal that would have caught that the array keys were non-numeric. (The fix was to toss an array_values() in there, problem solved.)
 
Ok, so assuming we have a library that want's to check itself (I don't imagine many would, and instead would only check the first parameter of query)... but if it did, then it could internally use a query builder, and when it's adding those parameter names, those would be checked as they are being added to the SQL string... :id_0 could pass a regex, but '; DELETE FROM users' would not.
 
I... have no idea what you just said.
 
ok, so you have inputs from the developer... that's the main focus for this RFC, that's where most of the mistakes are made... the example being $db->query("SELECT a FROM b WHERE id IN ($_GET[id])");
but we are looking at the next level, what happens inside the library
internally the library is making up an SQL string... typically this is more along the lines of Doctrine where you append things to it... so as things are being appended, you check the bits as needed.
(sorry, should note I am half asleep)
 
01:00 - 20:0020:00 - 00:00

« first day (3902 days earlier)      last day (1029 days later) »