I made a PHP script to automate a boring task that i had to do at my internship. Now you get a XML sitemap from a website and paste it in the form. And voila!
You can find it on GitHub here: https://github.com/jeroen1322/xml2htaccess
Thanks in advance.
@ to suppress errors, you can instead just check if file actually exists... that's it. There isn't really a lot of code to review there ... something like 30 lines.
Yeah the messages could use some work.
What is the advantage of using HTTP location header instead of refresh header?
Yeah using @ to suppress the errors was a quick and dirty way to get the job done.
And I know it isn't a lot of code. I wanted to keep it as simple and little as possible! :)
Also thanks for the tips!
don't use variables/indexes when you don't know they exist. Always check first: isset()
as mentioned above, use a Location header instead.
use isset instead of assuming this value exists.
You're taking user input and using it to save + read files on your server. This is horribly unsafe. The way you have this set up will also allow directory traversal attacks; and users can (intentionally or not) overwrite each other's files. Pick your own filename, always.
If you want to allow users to specify a filename, then you store it (e.g., in the session) and use it as the "filename" when you set the download headers. You NEVER use user input as a filename on your server.
As mentioned above, DO NOT USE @ TO IGNORE ERRORS. In this case, you'd want to use libxml_use_internal_errors() to collect errors (instead of issuing warnings) so you can handle them later.
SimpleXML is not simple. Use Dom instead.
instead of using an output buffer, why not simply write to the file as you go? Or (assuming this is short enough) just build the string in a variable?
Otherwise, not too bad. Looking at my comments above, I bet you can pick out the ONE to really pay attention to, even if you ignore all the others : )
Sorry for the late reply!
Thank you very much of pointing some stuff out to me. Very much appreciate it.
I made the script more as a prototype to show my boss. She wanted it to be done quick.
But that is no excuse to make it so unsafe. Users would indeed be able to delete whatever file they want on the server. Major mistake on my end.
I'll fix it as soon as possible! When i'm at it, i'll also take a look at the other modifications you suggested.
Thanks!
no problem.
one thing i like to point out to people is that, in almost all circumstances, you can replace the word "secure" with "works as intended." So, you're absolutely right that there's "no excuse" : )