verify my code please

General PHP Help
#1

hi I have some code written I just need someone please look over it if there are some coding practices that are better than what I have written. please I have it working but I just need verification.

[php]

<?php if(isset($_POST['submit'])){ $user = isset($_POST['username']) ? $_POST['username'] : ""; $password = isset($_POST['password']) ? $_POST['password'] : ""; } else { $user = ""; } $logins = ($user == "umar" )&& ($password == "machi") ? 1 : 0; ?>

<!doctype html>

<?php if (!$logins): ?> Username:
Password:

<?php endif; if($logins) { echo ("Welcome, " . $user); } ?>

[/php]

#2

[php]$user = isset($_POST[‘username’]) ? trim($_POST[‘username’]) : NULL;
$password = isset($_POST[‘password’]) ? trim($_POST[‘password’]) : NULL;

$logins = (($user === “umar” ) && ($password === “machi”)) ? TRUE : FALSE;[/php]

The above is how I would do it, but there are plenty of other ways of doing it.

#3

Dont ever use if(isset($_POST[‘submit’]))

There are issues with it in Internet Explorer. You can look up what it is.

#4

The be honest, the code is rather poor:

[ul][li]Never store plaintext passwords, not even in PHP code. There’s a huge risk that they will be exposed through a server misconfiguration or an attack. Hard-coded passwords buried somewhere in the code are generally bad, because changing them is inconvenient, and that means it probably won’t happen. Instead, use the HTTP Basic Authentication mechanism provided by your webserver (I’m assuming it’s Apache). If you insist on implementing your own authentication mechanism, store a password hash (not the password itself!) in a configuration script outside of the document root.[/li]
[li]What’s the point of setting the username and password to an empty string by default? If the user hasn’t provided all necessary data, that’s an error. Tell the user about it![/li]
[li]Your use of the ternary operator is weird. Why not use actual booleans to indicate true/false values? What’s the point of mapping a boolean to the number 0 or 1?[/li]
[li][tt]htmlspecialchars()[/tt] requires a character encoding (the third parameter), and it’s strongly recommended that you set the [tt]ENT_QUOTES[/tt] flag (in the second parameter). Right now, the function may not do anything.[/li]
[li]You print the [tt]$user[/tt] variable without escaping it, potentially leaving your application wide open to cross-site scripting attacks. It’s not enough to occasionally use HTML-escaping. You need to use it religiously.[/li]
[li]Your formatting is chaotic, which makes the code hard to read and understand. Use consistent spacing so that the formatting actually reflects the program structure. You also shouldn’t randomly jump in and out of PHP mode.[/li][/ul]

So the main weaknesses are poor security and readability. Try to be more careful when you write code, and do some research before implementing critical features (like password authentication). Proper formatting may not seem important to you right now, but it’s actually very important.

#5

Well, Machi, you appear to be a beginner, therefore some of the comments so far might be over your head.
Here is a nice tutorial that follows getting posted data, validating it and other areas that may help you to learn
how to handle this type of project. Just press the green Next-Chapter links to walk thru the tutorial. I think this
will help you to understand what these comments are talking about…
http://www.w3schools.com/php/php_forms.asp

[member=46186]Kevin Rubio[/member] , that issue is only for IE versions before IE9, right? The current usage of IE6-7-8 combined is only
about 3% of all browsers used in the world. I use "if(isset($_POST[‘submit’])) on lots of sites and will stop using it
if it is really not good. But, I can’t find any of the knowledge you mention on the net. Can you post more info
on why you don’t suggest using this? I am interested in more info on this subject…

#6

Kevin and Pretty Homepages have decided it is poor practice to rely on it. I use if for anything not API related and work in several systems that do so as well to verify a form was submitted. One of those systems was designed by a CompSci PhD with a background in Security. In other words, it is fine to use, they just don’t think so.

#7

Astonecipher, you are so wrong. We didnt “decide” anything and it has absolutely nothing to do with what we think. It is easily demonstrated as fact. I could have sworn I posted the details that prove it before. May have been on another forum. I wouldn’t call code that will fail on cue a bad coding practice. Any other reasons not to use it may be.

It is a provable fact that in certain older versions of Internet Explorer (< IE9), the submit button is NOT submitted, so relying on it to be submitted will fail every time. Apparently you haven’t done your homework on this subject.

#8

Astonecipher, I agree with you. I researched this and only the old IE’s have this issue. It was fixed in IE9 and
works 100% as far as I know on all of IE9-IE12. I researched how many <IE9’s are around and only 3 percent
of ALL browsers in use are IE6-IE7-IE8, so I feel it is fine to use this code. Since, <IE9’s will not work on most
secured sites such as banks, credit card companies and others, anyone who would ever be on my sites would
not have that old of a browser…

Just my 2 cents…

#9

[member=43746]ErnieAlex[/member] ,

What are you going to do if for some reason someone submits your form using cURL? They are not going to send “submit”. Can you say that 100% of users will 100% of the time use YOUR form to post data?

The correct way is if ($_SERVER[‘REQUEST_METHOD’] == ‘POST’)

#10

astonecipher, try to understand the reasons for that statement.

First off: This weird pattern of checking submit button values to determine the request method is a PHPism used almost exclusively in low-level applications. When you look into other languages or more advanced PHP applications (which have routers or complete frameworks), you won’t see this technique, because it simply doesn’t make a lot of sense. If you want to know the request method, you can simply ask the webserver (e. g. by reading [tt]$_SERVER[‘REQUEST_METHOD’][/tt]).

Secondly, an HTTP request does not always come with the value of some submit button. Why should it? There are no buttons in a machine-oriented API, an Ajax request or in any kind of automated request. In other words, you’d either have to “simulate” a submit button just to make your code work, or you’d have to process the same requests in two entirely different ways depending on the origin. Both is obviously ridiculous.

And third, making your entire application rely on the presence and name of one specific submit button is far too fragile to be used in production. Why on earth should a whole website break down just because some silly button has a different name? Why should buttons even have names? Their sole purpose is to be clicked on.

I understand that bad habits are hard to break. But why not try it?

#11

Yes, Kevin, I agree! But, I do not want anyone on my site(s) unless they log in. So, no CURL runs on those
pages. I do have a couple of sites I monitor that needs to allow CURL, so those have a different way of taking
in and handling post’s. (But, we were not discussing the use of curl posting…)

#12

You do know you can login with cURL too right? I have used it before to bypass a sites login form before. Dont remember why, but it was necessary for what I was doing.

Try this on ANY browser:

[php]<?php
var_dump($_POST);

if(isset($_POST[‘submit’]))
{
echo ‘The check says: POST request’;
}
else
{
echo ‘The check says: no POST request’;
}

?>

[/php]

Then, try this in any browser: (Only line 4 is changed)

[php]<?php
var_dump($_POST);

if ($_SERVER[‘REQUEST_METHOD’] == ‘POST’)
{
echo ‘The check says: POST request’;
}
else
{
echo ‘The check says: no POST request’;
}

?>

[/php]
#13

[member=76351]Pretty Homepages[/member] As I said, when doing API things I don’t check. The appications I work on are million dollar systems that the government uses and I can be held criminally negligent on a breach. I am quite sure you are not in the same situation. And the brain power I am around is far higher than your single competency level.

Until you can point to something reputable, ie other than your personal OPINION, it’s not factual. If the http requests does not come from the form, where is it coming from? Do I want to allow that kind of process to occur? No, I want it from the form that I provided or it would not be there.

As for this BS statement,

This weird pattern of checking submit button values to determine the request method is a PHPism used almost exclusively in low-level applications. When you look into other languages or more advanced PHP applications (which have routers or complete frameworks), you won't see this technique, because it simply doesn't make a lot of sense.

Let’s take a look at one of the largest frameworks:

After adding the AlbumForm to the use list, we implement addAction(). Let’s look at the addAction() code in a little more detail:

[php] $form = new AlbumForm();
$form->get(‘submit’)->setValue(‘Add’);
[/php]
We instantiate AlbumForm and set the label on the submit button to “Add”. We do this here as we’ll want to re-use the form when editing an album and will use a different label.


Taken from :ZF2 form handling

Hmm the creators and maintainers of not only the language, but the largest framework, use a submit button with the label ‘Add’. Tell me again how frameworks don’t do this? Or how it is only low level applications?

As I said you are full of it and trying to impose your thoughts, however meaningless they are.

#14
I work on are million dollar systems that the government uses

When did what the government does become a qualifier of what is good or right? I would say that in itself is the perfect DISqualifier no matter what the subject is.

I have seen a few government systems. Massive cluster echo str_rot13(“Shpx!”);

If the http requests does not come from the form, where is it coming from? Do I want to allow that kind of process to occur? No, I want it from the form that I provided or it would not be there.

Lets put all our “opinions” aside. [member=72272]astonecipher[/member], put up a simple test login form that you would expect someone to use and I will give you the answer. Unless you do something like NONCE nobody “has” to use “your” form to login.

#15

I didn’t say I work for the government, I said they use our systems. I also said, I can be criminally and financially responsible if my code fails. Do either of you have something more than, “but but in older IE browsers it won’t work”? cURL can be lobbed off of this as we are talking form submission, not api integration.

#16

Under what guise? You want simple html?
[php]

My App


  • First Name

  • Last Name

  • Email
























[/php]
#17

Have at it Kev.

#18

Why can cURL be lobbed off? It is perfect for form submission. It is also not the only other way to post data. We can keep going back and forth. Just put a simple demo login form on your server and we can put it to the test. So yes, we/I have something more than old IE. Put up the form and I will show you.

#19

I might be confused with all this. Can’t you just make sure it is your own server calling your own code?
Wouldn’t that just lock out CURL completely? How would an outside server be able to post to your own code if
the code self-checks itself???

#20

“Somebody” post a simple working login form on their server and we can start proving the answers. There is no point for me to put it on my own server.

Sponsor our Newsletter | Privacy Policy | Terms of Service