New to PHP and need your input

Coding Critique is the place to post source code for peer review by other members of DevNetwork. Any kind of code can be posted. Code posted does not have to be limited to PHP. All members are invited to contribute constructive criticism with the goal of improving the code. Posted code should include some background information about it and what areas you specifically would like help with.

Popular code excerpts may be moved to "Code Snippets" by the moderators.

Moderator: General Moderators

Post Reply
User avatar
myister
Forum Newbie
Posts: 12
Joined: Tue Sep 28, 2010 7:03 am
Location: Ohio
Contact:

New to PHP and need your input

Post by myister »

Hey guys I am pretty new to PHP and have been learning on my own and the help of others

I seen an idea on the internet while searching and decided to go ahead and add onto the sample code and further test the theory out. The code is based on providing a way to hide the real url of a video that is being played on a website. It seemed not so much on the user friendly side to use so i added some features to it, while experimenting and learning PHP.
The project is for school and I called the project VURL-e
The source is available for criticizing at http://code.google.com/p/vurl-e/downloads/list

I now there is a lot of bugs and what not but you have to remember that This is my first real attempt to write a script other than the standard "Hello World" ones.

Be honest and please give advise to how I can make it better.

Thanks everyone
User avatar
social_experiment
DevNet Master
Posts: 2793
Joined: Sun Feb 15, 2009 11:08 am
Location: .za

Re: New to PHP and need your input

Post by social_experiment »

myister wrote:I now there is a lot of bugs and what not but you have to remember that This is my first real attempt to write a script other than the standard "Hello World" ones.
Don't sweat it, even if you do get negative comments see them as positive reinforcement. It only helps make your scripting better.

I downloaded the script and tested it. Here are some issues you might want to take a look at :

1. Usernames & passwords can only contain letters. Why? Security is important so login information needs to be secure as possible. Also, you are writing this information to the database, you don't need any additional expertise to work with letter AND numbers.
2. Using MD5 for your passwords. Rather go for the SHA family. Look into hash() or if you don't want to venture into that just yet, at least change MD5 to SHA when hashing passwords (change database field accordingly).
3. MySQL Errors. You need to hide them better or display alternative errors that are less informative. I didn't change the password / username details in the config file and got this error
Warning: mysql_connect() [function.mysql-connect]: Access denied for user '******'@'localhost' (using password: YES) in C:\xampp\htdocs\Vurl-e1.2a\config.php on line 22
Could not connect: Access denied for user '******'@'localhost' (using password: YES)

I tested it on my localhost but it gives information regarding the server i am using as well as directory information. It seems trivial, but with security you can't be too sure.

The layout / graphics in the back-end (admin area) looks good, nice job on that :)
“Don’t worry if it doesn’t work right. If everything did, you’d be out of a job.” - Mosher’s Law of Software Engineering
User avatar
myister
Forum Newbie
Posts: 12
Joined: Tue Sep 28, 2010 7:03 am
Location: Ohio
Contact:

Re: New to PHP and need your input

Post by myister »

Thanks for your input

I put the characters only because i did not know for sure yet how to handle information going into a VARCHAR field in MySQL. I know now how that works so I am changing that. I am looking into the SHA and hash now and I am going to start putting that into the script.
I am also looking to add session support for the videostream.php so that when the video is downloaded through direct linking or however, they only get a file saying " Permission Denied ". I am still looking how to work this but I have a good idea how I am going to do that.

I know there is no way to prevent progressive downloading to the computer but this is a attempt to add a little bit more security to a already existing site that don't have a stream media server.

Thanks again for the insite.
jankidudel
Forum Commoner
Posts: 91
Joined: Sat Oct 16, 2010 4:30 pm
Location: Lithuania, Vilnius

Re: New to PHP and need your input

Post by jankidudel »

I don't know, the best suggestion for you is to write clean code, rather than just to write. Try to think what do you need, plan your source, how many actions your script must to handle(i's not so boring) Everything must be in the appropriate place. If you want, i can recreate your whole program just for the sake of idea.

Some notes about your css and html;
1. It would be better for you to write all the css contents in the appropriate place.(Use external files that you created in the CSS folder rather than in some like install.php)
2. HTML :
a) bgcolor => deprecated;
b) width => deprecated;
..........................
<li class="TabbedPanelsTab" tabindex="0">Upload A Video</li>
<li class="TabbedPanelsTab" tabindex="0">Get Links</li>
<li class="TabbedPanelsTab" tabindex="0">Sync</li>
...... and what is <div> for created ?
Post Reply