-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
use .php.cache for all php files generated into the cache dir #511
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
👍 |
Isnt it implicit that the files in the cache dir are caches? And i your ide picks them up and you cannot set some setting to prevent it i would suggest you go find another editor. |
On a related note, is there a reason why bootstrap.php.cache is generated into the |
P.S. I tend to agree with @henrikbjorn |
Sure I can configure it to ignore, so this is for convenience. Where is the harm though in this change? I see none and we already did it for the bootstrap. As for the bootstrap, the reason is that it is intended to have a very different "lifetime". Aka its only updated if you update your vendors. Speaking of which, we should probably clear the cache dir after running vendor.sh. But thats another topic. |
@lsmith77 the reason for not changing is the same as the one with template naming where the last part refers to the filetype. If you add .cache you will not refer to a .php file anymore which is wrong. Also if people have very insecure installations (some does) the .cache files will not be intepreted as php but shown in clear text. Furthermore this is a pull request that will only make it easier for you. And this is only a problem because you choose to use a editor that cant handle ignoreing somefiles for it autocomplete. |
There is little reason to consider these files as "php" files. Very rarely if ever users will look into these files at all, they should never be manually be modified. As for solving a problem "only for me", if by "you", you mean people like me, the first commenter and everybody else infavor of postfixing the bootstrap file with ".cache", then you are right. |
@lsmith77 I disagree, if you get an error message early in the bootstrap process an exception can be thrown referencing the file in the stack trace. |
Yeah, thats the rare case I was referring to for reading the files. Anyway the point of this pull is to be practical. IMHO, and I am not alone here, its more practical to use a php.cache extension. While the file can be hidden manually by configuring the IDE, so can php.cache be added for syntax highlighting if its needed. That being said I think the bulk of the users will not need the syntax highlighting, but appreciate not polluting their class searches in their IDE. However if this isnt merged, then we should also drop the php.cache extension for the bootstrap file. |
@lsmith77 we can both agree that the files contains php code. And therefor they should end with a .php file extension. Why would it be beneficial to change the extension (other than you editor is wierd) ? |
lets just agree that we disagree. but either way its not the end of the world. either we will have a vote or fabien decides. |
Hmm already thought about that and still dunno if it's good or not, here's my thoughts : 👍
👎
After all, i think i'm still 👍 but i'm not against listening more arguments :) |
I'm definitely 👍 - the only use case that it disturbs is people trying to debug issues in the framework itself, which in time should hopefully only affect core devs, and if you really need your syntax highlighting you can disable the bootstrap file in your app_dev.php and just require the autoloader afaik. Of course for looking at compiled DIC and stuff it's not as nice, but again this is a use case that should be less and less common with time. On the other hand, people with no Sf2 experience coming to it with their IDEs and opening cache files by mistake sounds pretty bad to me. I saw how it looked on @lsmith77 machine it's pretty annoying every time you want to jump to a class that's in cache it offers you two choices and you have to select one or the other. Beyond the fact that it's confusing, it's also slower, and it occurs much more often in day to day business. |
There is no argument here listing any benefit other than it would be nice for people using IDE's when doing development. I am a strong believer that the file extension should match the file type, otherwise how would you know what content the file has ? |
IDE support is a strong enough argument. I wouldn't want to program an enterprise scale application in notepad or without accurate code assist ;) |
@schniper i guess that a lot of people on unix / os x / windows will user notepad++ textmate coda vim gvim macvim emac aquamacs sublimetext... The list goes on and on, the list of ide arent very long there is Zend Studio (incl. Eclipse), PHPStorm, Netbeans and i cant think of any other. And saying that you need to have code assist etc to create a "Enterprise scale application" is plain stupid but we can ask the OpenSky guys what they use ? |
Once you reach a certain code size, you will appreciate the "little things" an IDE provides: templating, documentation, autocomplete, integrated debugging, in a word: standardization. var_dump and die doesn't cut it anymore. |
@schniper i dont think that it is true that "when you reach a certain size" that is bullsh** and not even related to the issue and the argument is still the same "we are lazy and dont know how to setup our editor" |
That is exactly the point: why not be lazy with this and use the time needed to setup/figure how to setup your editor/IDE for the actual programming? |
Because the file ending is wrong and insecure for people who does not know how to setup webservers proberly. A php file will be intepreted and show a blank page, a cache file will show the source code. So it is to protect users who dosent know what they are doing and keeping the right filename for files. .cache does now tell you what is inside it a php extension tells you it contains php code. You dont download music files as .jpg because someone thought they liked that file extension better. We could also go the whole way and just name all the files in the frame work with .inc and see how that goes. |
See? There's a config misunderstanding :D Only the web folder should be web-visible. |
I also believe it is wrong for the bootstrap files and if you cant figure out how to setup your editor then you should learn the tool you are using. And if you want to be lazy and do programming why are you then using an editor where you have to setup projects, use your mouse to click to open files etc? Also sounds like a sucky editor if you cant set it to ignore some file globally based on a project root. Yes it is a config misunderstanding but it will inevitable happen sometime when a new user trying out the framework fails to setup his/hers test enviroment up wrong and in my book it is more important to have people more safe even when they fail to setup webservers wrong than being lazy. And your best argument is still "I am lazy and dont want to change editor settings." |
Right. And I think it will stick :) Anyway, we've wasted enough scrollbar on this. Let's wait for other opinions. |
Maybe the answer is to have this as a configuration option? Then we could argue about which default to use. :) I'm in favor of naming them .cache. It seems more useful to most users. |
I'm also +1 on this, especially for the compiled class files. @henrikbjorn, surely you can tell your editor to display these files with the proper markup even if they do not have the .php extension. |
@schmittjoh surely you can configure your editor to ignore these files so they dont show up in your code assit. |
C'moon :D The question is: which one of these 2 cases mostly impedes starting up your development: inconsistent code assist throughout your project or lack of syntax highlighting for some files you don't need to view anyway? |
FYI: related PR PR525 is focusing on more debugging issue. |
I'm +1 on this. |
I think that hardcodng this decision in the proposed way is even worse than the decision itself. If I want to make a Symfony2 app available (with php config) that has no security issues by default when deployed wherever, then I now have to replace various components and copy&paste large methods into my subclasses to be able to change this back. So if this has to happen, at least make it more easy to configure back to the old way. |
The security argument simply holds no water. If your cache dir is exposed, so is your config dir. Its a non argument. The only somewhat valid argument is lack of syntax highlighting if you do need to read those files, but as stated before this requires a one time setting in your editor/ide vs. a per project setting. Making this configurable doesnt seem feasible or necessary. |
@henrikbjorn, we have people using stuff ranging from from vi to eclipse. I've personally had no luck excluding these files from my build path. Although my project settings have:
Eclipse continues to call up 4-5 copies of core classes with its auto-complete. One of those is the real file and the rest are duplicates from the multiple cache files. I'm unsure if this is the result of my own error with the configuration or some fault of eclipse. |
@lsmith77 You did see how I mentioned that any project which needs this as a security measure would also need PHP configuration? To me personally it's just not acceptable to deliver a project that requires more than dropping it into the webroot. The security problem is very real in this situation. I understand your points, and thus all I'm asking for is a way to configure this for such projects. |
If you want to be able to drop a Symfony2 app into a webroot you have to do add an .htaccess file (and enable .htaccess) into all subdirs preventing access, which again means one .htaccess for your app dir. Meaning once again this issue is not relevant to the topic discussed here because the cache dir is inside the app dir. |
I can neither assume that Apache is being used nor can I assume that .htaccess files even work. So yes, it is relevant. There are certainly a number of things to look out for, but this is one of them. |
Its not relevant because you need to secure the entire app dir. "Securing" just a few files from the cache dir by insisting to keep the .php extension gets you no where. So its irrelevant because you will need a more complete solution for this and this more complete solution isnt a patch work of small solutions. Its having to provide some mechanism to convince the server in question to only serve stuff that isnt in a number of directories (app, src, bin, vendor ..). It has nothing to do with the cache dir, let alone a certain type of file in the cache dir. |
There is simply no way to reliably accomplish that with all webservers, so the only viable solution is to make sure that critical information remains hidden in PHP files - be that in the app dir or the cache.. |
This is security by luck. Obviously you will then have to use php for configuration too. Furthermore you have to hope that no Bundle gets the idea of caching anything without a .php extension etc, which means you will need to educate your users what Bundles they are allowed to use etc. I think if this is your solution, then you have a problem aka this is not a solution. |
There simply is no other solution. Of course you ship an .htaccess file and try to educate users about server configuration. But neither of those is guaranteed to work so going this extra step is vital. If you don't it'll be your fault for providing them with insecure software. It's not like anyone cares if you told them to configure their webserver ... |
Maybe as a compromise we can have .php.cache in dev environment, and On Thu, Apr 21, 2011 at 3:31 PM, naderman <
|
Well at that point its cleaner to make it configurable. However what we are talking about here is not real security. Its patch work security where you try to make it harder. But its not security in the sense that the final solution is actually secure since there is plenty of stuff that doesnt end in .php still. That being said my motivation to work on fixing such a use case is pretty low and I think the issue has been demonstrated. So if you think its an acceptable solution to "security" I invite you to write a patch to make the extension configurable. |
OK, I want this fixed, so I have started working on making this configurable. I am mostly done, however before I create a pull I have a question: In other words the question:
The other question is should this parameter be defined inside the Kernel? If we go with 1) the parameter would only be needed inside |
I have implemented 1) in #633 |
similar to the bootstrap.php.cache this prevents IDE's from indexing these files