13 ms·

In all my years of writing PHP code I have yet to find a legitimate use case for extract() - PHP should have gotten rid of it generations ago.

• kijin 12y ago
  It's not terribly bad if you use it inside a function or method. Creating global variables is what causes all the issues, but then again, if you're handling global variables, you're asking for trouble anyway. What's really bad about extract() is that the default behavior is to overwrite existing variables. This is just a recipe for disaster, no less than the infamous register_globals. I once wrote a framework where the controller would pass variables to the view as $view->currentUser and the template would access it as $currentUser instead of the more verbose $this->currentUser. This was implemented with extract(), but I went to great lengths to make sure that the scope was clean. PHP probably ain't gonna get rid of extract() any time soon, but at least they should change the default to EXTR_SKIP.
• adamors 12y ago
  Frameworks tend to use it, to create variables in a view that were sent from a controller. For similar usecases it also makes sense (you want a bunch of variables created from an associative array).
• zapt02 12y ago
  Wordpress shortcodes, to extract variables from an array into local scope. Works great and even lets you set default values easily.
• p4bl0 12y ago
  I agree with you, but here the problem is not the extract() call as everyone here seems to imply. The security problem lies in the fact that the authentication code is conditionally called depending on user input. if ($login) { … } is not less safe than if ($_POST['c99shcook']['login']) { … } would have been.
  • 4mnt 12y ago
    I'm pretty sure $login can be set to true/false somewhere on top of the file as 'configuration'. Only it gets overwritten by the call to extract(). It was not meant to be user input
    • p4bl0 12y ago
      I feel stupid. Of course, you are right.
      • heydenberk 12y ago
        Don't feel stupid. This is difficult to reason about, hence the existence of the bug and the inadvisability of PHP's `extract` function.
• ars 12y ago
  You can use it to pass a bunch of variables between functions, without having to constantly get them from an array. function qux() { return compact('foo', 'bar', 'baz'); } And in the other function: extract(qux()); You can have one "preparer" function that works on the data and sets up the variables, then several others that work on it, and all call the same preparer. Or the reverse. Obviously you can do the same thing by just passing arrays but sometimes a simple variable is easier and less cumbersome.
  • kabdib 12y ago
    ... and to think that my contempt for PHP couldn't be increased. Wow. This is the anti-Scheme. It's like they took everything good in language design and decided to do the exact opposite. "Let's make a function that has the side effect of introducing variables into scope. That's a great idea." Oh, Bog....
    • yeahbutbut 12y ago
      > "Let's make a function that has the side effect of introducing variables into scope. That's a great idea." And ``let'' is? ;-) Doing a (let [request] (check-auth ...)) would be equally dumb. Let's not blame the tools.
      • jsn 12y ago
        That's not not how 'let' works, you can't do that with 'let'. You most definitely should blame the tools here.
        • yeahbutbut 12y ago
          You're right, let is a macro so you can't pass binding forms from a variable, my mistake. But surely Scheme has some function for binding variables to the local scope... My point is that trusting user input is the error, not having the ability to play with the scope.
          • 12y ago
• dikei 12y ago
  Exactly, I'm all for getting rid of black magics such as extract() and compact().
• Spoom 12y ago
  Agreed. The sane alternative is to extract specific, expected inputs to scope once they have been filtered and escaped. Any API I've written has this sort of validation built-in.
• yeahbutbut 12y ago
  I've found exactly two: Ghetto templating (using a PHP file as a template). Extract an object from the database into local vars to echo in the "template". Parsing fixed width files, zip the columns with their names, then extract in the processing function. In both cases you know exactly which vars are being replaced. The real WTF is extract on $_{REQUEST,GET,POST,SERVER,...}.
  • ojilles 12y ago
    I wouldn't use the word "exactly" (high chance?): f.ex. where did the fixed width files come from? If it's from some (external) other system you're already have an attack vector.
    • yeahbutbut 12y ago
      > f.ex. where did the fixed width files come from? Encrypted and signed files sent over the wire from a company we do business with. Additionally I was just pairing the values up with local names that I chose (the fixed width file had no column names itself, they sent us a word doc (ugh)). And the function that called extract had exactly one local, the array I built representing a row from the file (unused after the initial call to extract). Extract also lets you prefix the extracted vars, avoid overwriting name collisions, etc. http://php.net/extract http://php.net/extract The code from this article is unsafe because it directly operates on user input, was not explicit about what values were required (you can filter an array by key easily enough...) and doesn't isolate the environment it's extracting in. That's the unsafe behavior.
• thefreeman 12y ago
  I think it is a useful construct for specifically the scenario it is mis-used in here. Overriding default values, or a kwargs polyfill function foo($option1=1, $option2=2, $option3=3, $option4=4) { //do stuff with params } this is unwieldy and difficult to reason about when you see it used, especially if you are trying to use some of the initial default values. ie. foo(null, null, null, 10) vs. function foo($options) { $option1 = 1; $option2 = 2; $option3 = 3; $option4 = 4; extract($options); //do stuff with params } With this, you can just do foo(array('option4'=>10)); obviously its a preference thing, but I think it is a somewhat harmless use case. If there are any params which you do not want to be overriden you move them beneath the call to extract.
  • ceejayoz 12y ago
    We typically handle that by having a $defaults array, then array_merge($defaults, $options) in the function.
    • thefreeman 12y ago
      Yep, totally valid, and like I said basically a preference thing. I personally like to work with explicit variables within the function rather then array indices. With your system I could easily call extract on your resulting array, but I also like defining the variables I want overridden explicitly in code so I my IDE doesn't think they are undefined (though JetBrains does a pretty good job of determining which variables are being created by extract in most scenarios anyway).
• mtdewcmu 12y ago
  Drupal uses it to implement templates. I always got the feeling that it's one of those language features where you want to take a deep breath before using it; sort of like when you type `rm -rf *` and haven't pressed enter yet.
  • derefr 12y ago
    It makes perfect sense when PHP is actually being used as a backend-isolated, controlled-environment templating language. Imagine this setup: 1. you have Nginx at $DOMAIN, with your well-oiled API service (with no HTML rendering logic to be found within it) mounted to /, and a PHP FCGI instance sitting on /views. 2. Nginx is configured such that /views is only accessible from localhost. 3. Your webapp on / mostly sees requests with "Accept: application/json" or some such. It responds to these directly. 4. If your webapp sees "Accept: text/html", it does the same stuff it would to render a text/json response... but then takes that response body, and sends it in an HTTP subrequest back to Nginx, to, say, /views/accounts.php. 5. PHP does this: extract(json_decode($_POST['response'])); ...and then does whatever it does, simply and elegantly, to render your template. 6. Your webapp catches the response from Nginx, and sends it back without modification to the client.
    • dsl 12y ago
      You are setting yourself up for a reflected request forgery attack.
    • mtdewcmu 12y ago
      The way PHP allows fluid, dynamic interaction with the symbol table is pretty nifty, I think, and even if there were no extract() function built into the language, you could implement it yourself out of other primitives. I'm not one that believes that any feature of a language that can be used in dangerous ways should be eliminated (anything can be dangerous). I just think extract() is one that you ought to think carefully about before using. This example ought to reinforce that.