4 ms·

+1 to russ. PHP syntax checking on precommit would have caught this. Edit: I'm wrong. It does not catch this.

zaidf 16y ago
Not necessarily. Makes sense for syntax check to only check between php tags. Root of the issue is that by default php outputs anything not between php tags to the browser. Perhaps some sort of buffer control at the top most include can disable output to browser until a later time. That's pretty much how templating systems work except they don't disable output afaik thus leaving possibility of this open.
- dexen 16y ago
  You could do ob_start(); include '...'; ob_end_clean();, but it turns out that die() (say, in a custom error handler) () causes an implicit buffer flush. So it's not a foolproof solution.
  - bluesnowmonkey 16y ago
    You can attach a callback to the buffer to prevent that implicit flush. ob_start(function() {});
    - dexen 16y ago
      Thanks, I'll investingate and probably adopt it for my code :D
  - KevBurnsJr 16y ago
    Or register_shutdown_function - http://blog.kevburnsjr.com/php-fatal-error-500 http://blog.kevburnsjr.com/php-fatal-error-500
- bluesnowmonkey 16y ago
  Exactly right. There's no reason for a config file (or any of the setup code) to be writing output. A good way to force this is by immediately starting an output buffer with a callback that returns nothing, then calling ob_end_clean() after setup is complete.
dexen 16y ago
No chance. The problem was a missing <?php tag -- PHP syntax cheching would NOT have raised alarm, because everything before a <?php tag is not considered PHP. Forcing every file to start with <?php is just PITA for developers working on templates.
- bluesnowmonkey 16y ago
  If the templates are raw PHP, yeah. Constructing HTML in code like that is a PITA and a recipe for errors and XSS vulnerabilities. It's common to use templating engines like Smarty or Mustache. In that case, you can standardize on PHP files starting with an opening tag and having no closing tag. Then syntax checking is a piece of cake.
  - dexen 16y ago
    After using eZ Publish with its templating language (also implemented in PHP) for about a year, I've found `raw' PHP just more expressive and concise -- and my team, coming from various backgrounds, has that preferrence as well. Whaddya know, PHP is a templating language, after all. Also, it takes us less effort to ship product that matches performance requirements with `raw' PHP than with another layer of abstraction.
- russss 16y ago
  > Forcing every file to start with <?php is just PITA for developers working on templates. Yep, our templates were .tpl files, which is probably a good convention to have even if you use raw PHP as your templating language. But you get the drift. This is something you have to deal with when you use PHP.

zaidf 16y ago

Not necessarily. Makes sense for syntax check to only check between php tags. Root of the issue is that by default php outputs anything not between php tags to the browser. Perhaps some sort of buffer control at the top most include can disable output to browser until a later time. That's pretty much how templating systems work except they don't disable output afaik thus leaving possibility of this open.

dexen 16y ago
You could do ob_start(); include '...'; ob_end_clean();, but it turns out that die() (say, in a custom error handler) () causes an implicit buffer flush. So it's not a foolproof solution.
- bluesnowmonkey 16y ago
  You can attach a callback to the buffer to prevent that implicit flush. ob_start(function() {});
  - dexen 16y ago
    Thanks, I'll investingate and probably adopt it for my code :D
- KevBurnsJr 16y ago
  Or register_shutdown_function - http://blog.kevburnsjr.com/php-fatal-error-500 http://blog.kevburnsjr.com/php-fatal-error-500
bluesnowmonkey 16y ago
Exactly right. There's no reason for a config file (or any of the setup code) to be writing output. A good way to force this is by immediately starting an output buffer with a callback that returns nothing, then calling ob_end_clean() after setup is complete.

dexen 16y ago

No chance. The problem was a missing <?php tag -- PHP syntax cheching would NOT have raised alarm, because everything before a <?php tag is not considered PHP. Forcing every file to start with <?php is just PITA for developers working on templates.

bluesnowmonkey 16y ago
If the templates are raw PHP, yeah. Constructing HTML in code like that is a PITA and a recipe for errors and XSS vulnerabilities. It's common to use templating engines like Smarty or Mustache. In that case, you can standardize on PHP files starting with an opening tag and having no closing tag. Then syntax checking is a piece of cake.
- dexen 16y ago
  After using eZ Publish with its templating language (also implemented in PHP) for about a year, I've found `raw' PHP just more expressive and concise -- and my team, coming from various backgrounds, has that preferrence as well. Whaddya know, PHP is a templating language, after all. Also, it takes us less effort to ship product that matches performance requirements with `raw' PHP than with another layer of abstraction.
russss 16y ago
> Forcing every file to start with <?php is just PITA for developers working on templates. Yep, our templates were .tpl files, which is probably a good convention to have even if you use raw PHP as your templating language. But you get the drift. This is something you have to deal with when you use PHP.