Jump to content

requinix

Administrators
  • Posts

    15,229
  • Joined

  • Last visited

  • Days Won

    427

Everything posted by requinix

  1. The problem is that you've constructed an invalid query. If you use PDO's prepared statements then you won't have that problem. ...but it won't be able to do the $searchtype part. You must make sure that it has an acceptable value before you put it into the query. You can do that with code like if (in_array($_POST['search'], array('firstname', 'secondname'))) { $search = $_POST['search']; } else { // either show an error or use a default value for $search }
  2. %y * 12 + %m Great that you solved it, but posting the solution you came up with would be even better.
  3. There's a fair bit of boilerplate code in your factory - particularly in createUserModel. You'll find yourself repeating that in all the factories. Consider any of a) Subclassing the factory, so the parent class handles the general work and the child classes provide the specialized logic for the particular models b) Doing something similiar with the model; a parent class would handle the constructor and setting properties while the children implement any other logic they need c) Using traits to at least move the logic into a centralized place, if you don't want a class hierarchy For DI, you've already changed it up a bit so what I said is less applicable now. Having to pass around various classes and such in constructors is annoying, right? It provides a sort of "perfect" dependency injection because you can customize absolutely everything, but at the cost of increasing complexity and of being a nuisance to the developer. You can take a step back from that and use a sort of (gasp!) registry instead; as a demonstration, I use something like $this->tableGateway = Dependencies::get(userTableGateway::class); final class Dependencies { private static $classes = []; private function __construct() { } public static function get($class) { if (isset(self::$classes[$class])) { $class = self::$classes[$class]; } return new $class(); } public static function register($dependency, $class) { self::$classes[$dependency] = $class; } }But that's one of many ways, and they all have advantages and disadvantages. Use whatever works for you.
  4. Use a backslash to escape, not an apostrophe.
  5. That code is all sorts of messed up. Are you sure you copied it down correctly, either in your own code or into your post?
  6. Are you married to that idea? Because something like array("max_length" => 60, "required" => true)looks a lot nicer to me. To answer the question as asked, you need to break apart $rule on the colon so you can have the first part (the type of rule) separate from the rest (data for the rule). explode() can do it list($type, $data) = explode(":", $rule, 2); // a:b:c:d -> a, b:c:dThen check $type and use $data however is needed.
  7. Being a database model it should not perform the actual task of logging a user into the website - putting stuff into the session and whatnot - but validating credentials is an important job for it. After all, only the User class understands how its passwords work (supposedly) so it is the only one that can do it. Therefore you use the User class to validate the login information provided, then use other code elsewhere to put stuff into the session and whatnot. A LoginUserModel is breaking the limits of what classes mean. A class basically represents an entity. A thing, like a user. It should not represent an action, like the action of logging a user in. (In the real world it's not quite that definitive but it is the rule of thumb you should go for.) So that does leave a question of where to put the login code: it can't go in the User class because that's beyond the scope of the model, and it can't go into its own dedicated class because logging in is an action and not an entity. The answer depends on your application. Most modern sites use MVC, and one answer is intrinsic to the design pattern itself: use a controller. The "M"odel is the User class and it interacts with the database, the "V"iew is the login page itself that the user works with, while the "C"ontroller is the one that actually does things. As a controller - an entity that represents the concept of a login page, if you will - it performs actions based on the page. One action is to present the login page. Another is to process the login information and react accordingly (ie, log in with good credentials or error with bad ones). So the code to log a user in goes in the login controller. Another answer is to use a business model... There are different types of models but they come in basically two flavors: 1. Database model. This object probably mirrors the backing database table, with the table's columns as properties or accessor methods. It does things like load from the database, save a record back to it, and perform basic searches. There's generally not too much logic in there. 2. Business model. This object tries to bridge the gap between what the database model offers and what the application actually needs. For instance, a plain User database model will be mostly about the data itself (eg, find it in the database, change it, save it back) but an application probably needs to do more (eg, log the user in, send a password reset email). 3. A hybrid of the two, which IMO is bad design. Having two classes is only a little more code but you get a clean separation between the representation (database model) and the behavior and usage (business model) and that's more than worth the tradeoff. It is at this point that I realize I made some assumptions that may not be true... I've been assuming we were talking about treating the User as a database model, I'm not sure that's what you were thinking. So, that code you posted is odd. Incomplete and inconsistent. I'm going to make more [assumptions]. Your UserModel looks like... well, it's partly a factory and partly a database model. I've seen that done before and I don't care for it. I'll get back to that in a minute. - It has a constructor that uses DI for the database and encryption stuff. That's fine. [Except that it is using encryption for passwords and not hashing, which is not fine.] - It has a getFromLogin method (fetch from the database using the unique username) and validateLogin method (check the provided password against the actual password). Those are fine too. - getFromLogin has comments talking about getting stuff from $_POST. That's not fine. [but the way the rest of the code is written, let alone there being a $username parameter, suggests the comments are simply wrong.] AuthController looks like a typical MVC controller. The login action - Constructs the UserModel factory with the given database (but no encryption) - Grabs data from $_POST and gives it to the model/factory to find a user - Uses the model to test [the POSTed password] and performs the actual login if successful Back to the factory aspect of UserModel. You're doing two things with an instance of this class: using it to locate user data, and using to represent user data. I've already written enough in this post so I won't get on my soapbox and preach about stuff like statefulness here (unless someone wants to hear it?) so the short version is Don't do that. Either you use a distinct factory class and distinct user class, or (more conveniently) you use static methods for the factory aspect and keep the instance dedicated to being a user model. In other words, // separate factory class UserModelFactory { public function __construct($databaseClass, $encryptionClass) { $this->db = $databaseClass; $this->enc = $encryptionClass; } public function getFromLogin($username) { /* grab data from database */ /* if found { */ return new UserModel($data, $this->db, $this->enc); /* } else { */ return null; // or whatever /* } */ } } class UserModel { public function __construct($data, $databaseClass, $encryptionClass) { $this->data = $data; $this->db = $databaseClass; $this->enc = $encryptionClass; } public function validateLogin($password) { /* validate... */ } } $factory = new UserModelFactory($database, $encryption); $user = $factory->getFromLogin($_POST["username"]); if ($user && $user->validateLogin($_POST["password"])) { // ... } else { // ... } // static factory class UserModel { public function __construct($data, $databaseClass, $encryptionClass) { $this->data = $data; $this->db = $databaseClass; $this->enc = $encryptionClass; } public static function getFromLogin($username, $databaseClass, $encryptionClass) { /* grab data from database */ /* if found { */ return new self($data, $databaseClass, $encryptionClass); /* } else { */ return null; // or whatever /* } */ } public function validateLogin($password) { /* validate... */ } } $user = UserModel::getFromLogin($_POST["username"], $database, $encryption); if ($user && $user->validateLogin($_POST["password"])) { // ... } else { // ... } They're very similar, which is why the factory class is probably overkill for you so I'd say go with static methods. Note that's a separate issue from having database and business models vs. having a hybrid. Not sure if I answered all the questions, or whether I'm creating more. Oh, and you may be starting to feel like the way you're doing DI (by passing around the database and encryption stuff) isn't working very well. I would agree with that.
  8. Composition is the solution to maintaining single responsibility but dealing with the unavoidable necessities of having a, well, functioning codebase: classes should be responsible for a specific set of actions/data/whatever, however they also need to interact with other actions/data/whatever, so they do that by (eg) composing themselves of the classes corresponding to those actions. It's still single responsibility because every class is only responsible for itself, and anything else it needs is handled by a different class. Authentication should not talk to the database. Only database models should ever have to talk to the database (with occasional exceptions). Instead, Authentication should use a "User" model to grab the corresponding user and/or validate their credentials. Like try { $user = \Models\User::getFromLogin($username, $password); } catch (\Models\UserException\UserNotFound $e) { // no matching username/password } $user = \Models\User::get($username); if ($user && $user->validateLogin($password)) { // good } else { // bad }or one of the other million ways of doing that idea. This way Authentication is responsible for performing the action of logging a user into the website, and the User class is responsible for user actions like finding them and checking credentials. Side comment: Authentication is either a page controller or an entity encapsulating credentials for the website, right? It's not like some sort of utility class that has a single method to log a user in, right?
  9. Yes. Yes you do. Did you understand what I said? Do you know what those two loops are doing? I'm confused too because if you knew what that inner foreach did then I'd expect you would understand why you need to remove it. Try what I said, and if it still doesn't work then post your code and explain what's happening.
  10. Seems you're a little confused here. $row is the keys in the user_cards object ("1", "2", etc.), $v is the corresponding object; $key is "num_owned/num_used/is_locked", and $val is the corresponding value. So looking at $val doesn't make sense. Get rid of the inner foreach loop. It gets you $key and $val and those are worthless. Instead, decide what to do with $v if $v['num_owned'] > 0.
  11. If you're trying to make it display a certain way then the database isn't the issue. Moved. What's your current code to display the names?
  12. IIRC some Linux distributions build their phpX-json extension using json-c. [edit] Actually I just don't remember. But I do know that some PHP installations use the ext/json implementation and some don't. jsonc, json-c, and jsond are names that come to mind.
  13. test_input is not right. $data = stripslashes($data);Using stripslashes is old. The reason for doing it in the first place was a (mis)feature called magic_quotes, however that's long since been removed from PHP so don't do this anymore. $data = htmlspecialchars($data);Don't use htmlspecialchars indiscriminately like this. It should happen at the last minute just before you put something into HTML. Before then you need to leave the data alone, with only a couple exceptions (like using trim). Instead your $message line should look like $message = htmlspecialchars($name) . " " . htmlspecialchars($telephone) . " " . htmlspecialchars($email) . " " . htmlspecialchars($details);(though there are fancier ways of doing it) And actually, you're being more paranoid than you need to be. Remember your regex check on $name? You already know it contains only letters and whitespace so there's no risk of XSS. On the other hand, while you did validate $email using filter_var() there's no guarantee that it's not also valid HTML (because email addresses can be weird like that) so you do need to escape it. $message = "{$name} " . htmlspecialchars($telephone) . " " . htmlspecialchars($email) . " " . htmlspecialchars($details);Speaking of names, I sure hope you aren't expecting any feedback from Conan O'Brien because your form would prohibit his. Really, for names you should just allow everything (remembering to add htmlspecialchars back in) because people have weird names.
  14. substr is one of the most basic string functions PHP has so go ahead and start learning about it now - you were using str_replace just fine (except for the repetition, that is).
  15. As long as you're doing this for fun and not for real encryption then, Are you trying to turn "abcdefgh" into "efghijkl"? By adding 4 to each letter? strtr can replace multiple characters at once. To do +4 you could use it like strtr("abcdefgh", "abcdefgh", "efghijkl") // efghijkl // that's not a good example of how it works, so strtr("abcdefgh", "ghefcdab", "klijghef") // g->k, h->l, etc. The second argument to "search" for is just the alphabet, so you need to come up with the third. Start with "abc...xyz" then use string functions like substr to rearrange it so that abcdefghijklmnopqrstuvwxyz /\ 4 abcd efghijklmnopqrstuvwxyz efghijklmnopqrstuvwxyz abcd efghijklmnopqrstuvwxyzabcd
  16. "Grouping by" something means combining all the records with the same values together. That means there will be only one row per Team_catId. You do not want to do a GROUP BY. What kind of results are you trying to get?
  17. How about we try to solve the earlier problem instead? What conflict? What was it doing and what was it supposed to be doing? Any error messages? And most importantly, what was the code?
  18. We use Sphinx here. I won't claim to know much about it, but I am generally pleased with it. A pure PHP solution probably won't be too good. You should have something running in memory to perform searches, and regular databases can't handle that kind of work.
  19. Why not pass the variable in the first form so you don't need the second to retrieve it? Anyway, your form isn't going anywhere. Shouldn't the action be some URL?
  20. Then it's more complicated. Can we get some clarification about exactly how week numbers are supposed to work? Is it ISO 8601 but using Wednesday as the start of the week, so week 1 is - First week with 4+ days in January - First week with the Monday Wednesday closest to Jan 1 - The one with Jan 4 in it - The one with the first working day, not counting Saturday or Sunday (weekends) or Jan 1 (New Year's Day) I suspect so, in which case you'd have to emulate the same logic. Not sure if there's an easy way that works for all days of the year, or else just do a condition on whether the day is December 29 - January 10 for the special logic and the -1 week stuff for the rest of the year.
  21. After four hours of searching, maybe it's time to try to solve the problem rather than continue trying to find somebody who has done it before. Assuming that "W" worked perfectly for you before (because it may do unexpected things at the beginning of the year) then, As documented, "W" starts on a Monday. That means Monday and Tuesday dates will be one week number ahead of where you want them to be. You could just -1 but that will cause problems for the first M/Tu of the year. - If today is a Monday or Tuesday, pick the day one week earlier, otherwise use today - Get the "W"eek number of that date if (date("N") <= 2) { // monday or tuesday $date = strtotime("-1 week"); } else { $date = time(); } echo date("W", $date);
  22. 1. The mysql extension and its mysql_* functions are old and unsupported. You need to start moving to either PDO (recommended) or mysqli. 2. I sure hope $search was escaped before you put it into that query. 3. die()ing with the MySQL error message is dangerous, not to mention a bad user experience. A Refresh value needs to be structured as "seconds;url=path", with the first part being a number of seconds to wait to redirect. 4. A meta refresh (as it's called) is a poor way to move users between pages. Use a header() redirect if you haven't created any output or display a message with a link on the page if you have.
  23. Are the files all constant-rate 128Kbps? Have they been stripped of all ID3 tags? $showsize is in KB so the /1000 would be both wrong (check the man page for what -k means) and unnecessary. 34308 KB * 8 = 274464 Kb / 128 Kbps = 2144.25 sec = 35:44. So there's something else going on.
  24. $saleprice is being outputted into the HTML somewhere. That's where you should add the span. But first look to make sure there isn't already a way to style the element as it currently exists on the page.
  25. Note that what you have will check if there's that 5-4 somewhere inside $input. As in it will quite happily match with $input = "a bunch of stuff then 12345-6789 and then more stuff";If you want to make sure $input has only that 5-4 then you need ^ and $ anchors to mark the beginning and end of the string. '/^\d{5}-\d{4}$/'
×
×
  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.