4 ms·

Can someone please explain this a little more? My basic understanding is that they incorrectly opened a PHP tag and exposed the code. If that's the case, woul

by holdenc 16y ago

Can someone please explain this a little more? My basic understanding is that they incorrectly opened a PHP tag and exposed the code. If that's the case, wouldn't the page have appeared broken in development? Or have been found during testing?

deleted 16y ago
[deleted]
oscardelben 16y ago
By looking at the code !quality, I wouldn't be surprised to learn that they have no tests at all.
- pilif 16y ago
  How can you look at this code and make any statement about general code quality? The stuff that was exposed was pure initialization code that would look the same anywhere you look. Granted. This should move outside of the web root, but just looking at this one file that just sets up configuration values provides no ground to say anything about general code quality
  - jasonlotito 16y ago
    > How can you look at this code and make any statement about general code quality? The stuff that was exposed was pure initialization code that would look the same anywhere you look. No it doesn't. Having to modify live code just to change configuration options is bad. If I want to add a new slave, or change some configuration option, it should have no affect on running code. I shouldn't be changing any software logic, and that's exactly what editing this file does. Their are standard ways to avoid this. But no, if changing your config file means changing what essentially amounts to changing the executable of your code, you're asking for trouble.
    - bluesnowmonkey 16y ago
      It doesn't really matter if the configuration is in an executable form or not. Without testing, you can break it either way. With testing, you probably won't. They're equivalent.
      - jasonlotito 16y ago
        This issue isn't the site breaking. The issue is the leaking of the configuration options. By removing it from the code, you can help avoid that. And you can even go the extra step of making sure that even if you load a broken config, the system will continue to use the old config.
bluesnowmonkey 16y ago
Sure. Their site has a front-end controller (www/dispatch.php) to which all requests get routed (by a mod_rewrite rule, perhaps), rather than having .php files in the web root for each page. This file sets up the environment -- among other things, it registers a custom error handler and includes a config file (config/config.php) that defines a bunch of constants. Then it dispatches the request to the appropriate controller, based on the URL. Someone (probably a sysadmin) edited the config file and accidentally changed the opening tag (<?php). This caused PHP to output its contents, rather than parsing and executing it. Since there was no output buffer active, those contents were sent directly to the user, which caused HTTP response headers also to be sent automatically. That's the big first line you see in the output. Since no error had actually been triggered by this point, execution continued. It tried to set an HTTP header ("P3P: CP="P3P_CP"", whatever that means). However since HTTP headers had already been sent, this did trigger an error, which was passed to the custom error handler, which sent some debug output (the rest of the output you see) and stopped execution.

by holdenc 16y ago

deleted 16y ago

[deleted]

oscardelben 16y ago

By looking at the code !quality, I wouldn't be surprised to learn that they have no tests at all.

pilif 16y ago
How can you look at this code and make any statement about general code quality? The stuff that was exposed was pure initialization code that would look the same anywhere you look. Granted. This should move outside of the web root, but just looking at this one file that just sets up configuration values provides no ground to say anything about general code quality
- jasonlotito 16y ago
  > How can you look at this code and make any statement about general code quality? The stuff that was exposed was pure initialization code that would look the same anywhere you look. No it doesn't. Having to modify live code just to change configuration options is bad. If I want to add a new slave, or change some configuration option, it should have no affect on running code. I shouldn't be changing any software logic, and that's exactly what editing this file does. Their are standard ways to avoid this. But no, if changing your config file means changing what essentially amounts to changing the executable of your code, you're asking for trouble.
  - bluesnowmonkey 16y ago
    It doesn't really matter if the configuration is in an executable form or not. Without testing, you can break it either way. With testing, you probably won't. They're equivalent.
    - jasonlotito 16y ago
      This issue isn't the site breaking. The issue is the leaking of the configuration options. By removing it from the code, you can help avoid that. And you can even go the extra step of making sure that even if you load a broken config, the system will continue to use the old config.

bluesnowmonkey 16y ago

Sure. Their site has a front-end controller (www/dispatch.php) to which all requests get routed (by a mod_rewrite rule, perhaps), rather than having .php files in the web root for each page. This file sets up the environment -- among other things, it registers a custom error handler and includes a config file (config/config.php) that defines a bunch of constants. Then it dispatches the request to the appropriate controller, based on the URL. Someone (probably a sysadmin) edited the config file and accidentally changed the opening tag (<?php). This caused PHP to output its contents, rather than parsing and executing it. Since there was no output buffer active, those contents were sent directly to the user, which caused HTTP response headers also to be sent automatically. That's the big first line you see in the output. Since no error had actually been triggered by this point, execution continued. It tried to set an HTTP header ("P3P: CP="P3P_CP"", whatever that means). However since HTTP headers had already been sent, this did trigger an error, which was passed to the custom error handler, which sent some debug output (the rest of the output you see) and stopped execution.