Jump to content

Jacques1

Members
 View Profile  See their activity

  • Posts

    4,207

  • Joined

  • Last visited

  • Days Won

    209

 Content Type 

Everything posted by Jacques1

  1. Jacques1
    The question is: Why would you want to write all those model classes by hand? It makes perfect sense to use an ORM library which creates the classes automatically. It also makes perfect sense to avoid ORM and just go with classical queries. But your approach sounds like a weird typing exercise. If this is exactly what you want, I won't stop you.
  2. Jacques1
    You can literally copy and paste the example code from the above thread: <?php header('Content-Type: text/html; charset=UTF-8'); // test data $keys = [3, 42, 99]; ?> <!DOCTYPE HTML> <html> <head> <meta charset="utf-8"> <title>JSON</title> <style> .hidden { display: none; } </style> </head> <body> <div id="my-data" class="hidden"><?= htmlspecialchars(json_encode($keys), ENT_QUOTES | ENT_SUBSTITUTE | ENT_HTML5, 'UTF-8'); ?></div> <script> var kArray = JSON.parse(document.getElementById('my-data').innerHTML); alert('Array content: ' + kArray); </script> </body> </html>
  3. Jacques1
    It's wrong and dangerous to execute JSON-encoded data as JavaScript code. Use Ajax or HTML-escape the encoded data and put it into a hidden HTML element where it can be parsed with JavaScript. We'll also need to see the content of $Keys if you want help with that.
  4. Jacques1
    What are those “other cases”? And what do your users really need? Yes, I'm sure Highcharts can be pumped up with thousands of lines of highly dynamic JavaScript code, but unless you're targeting diagram fetishists, I'm sure most people just want to select a chart type, enter their data and be done with it.
  5. Jacques1
    <?php const PASSWORD_MIN_LENGTH = 8; const PASSWORD_MAX_LENGTH = 56; // bcrypt is limited to 56 bytes of input const PASSWORD_HASH_ALGORITHM = PASSWORD_BCRYPT; const PASSWORD_HASH_COST = 10; // the cost factor which determines the hash strength; should be as high as possible /* test: create a new password hash */ $password = 'g3xoc2YJ'; if (strlen($password) < PASSWORD_MIN_LENGTH) { die('Password too short, length must be at least '.PASSWORD_MIN_LENGTH); } if (strlen($password) > PASSWORD_MAX_LENGTH) { die('Password too long, length can be at most '.PASSWORD_MAX_LENGTH); } $passwordHash = password_hash($password, PASSWORD_HASH_ALGORITHM, ['cost' => PASSWORD_HASH_COST]); echo 'Password hash: '.$passwordHash.'<br>'; /* test: verify password */ if (password_verify($password, $passwordHash)) { echo 'The password matches the hash.<br>'; } else { echo 'The password does not match the hash.<br>'; }
  6. Jacques1
    So you want the chart component to execute any JavaScript code? Even if that code steals the session ID, fetches the user password or executes drive-by malware? I hope not. What I see is a plain simple color with fixed boilerplate code for the theme. That's not “I need to store and run arbitrary JavaScript code”. It's storing a plain simple color and then preprocessing the data with the fixed boilerplate code for the theme. You should write a preprocessor or even wrapper for the Highcharts libray where you receive pure data from the database and then add all the dynamic parts. I wonder why Highcharts itself doesn't do that.
  7. Jacques1
    Your terminology is way off. crypt() hashes a password, it has nothing to do with “encryption”. A hash cannot be “decrypted”. And the algorithm you're using is bcrypt, not “Blowfish”. So when you're asking us to “decrypt the password”, this makes absolutely no sense. Besides that, the crypt() code you've appearently copied and pasted from the Internet is insecure and garbage. I suggest you simply throw away the script and start over, this time with the proper password hash API. You should also learn how to use mysqli correctly, particularly how to use prepared statements.
  8. Jacques1
    How is this a solution? The only improvement I see is that you now use null instead of “NONE”. What about all the other problems I've pointed out?
  9. Jacques1
    There's also NOT IN if you want every status except for, say, “COMPLETE” and “CANCELLED”: status NOT IN ('COMPLETE', 'CANCELLED') And if you really need such a complex status system, you should categorize the status values into groups like “error” or “progress”. Then you can, for example, check for the error group instead of going through every single error. I think everything is better than cryptic numbers.
  10. Jacques1
    Why are you trying to bind the statement parameters twice? You either use PDOStatement::bindParam(), or you pass an array to PDOStatement::execute(), not both. The function design is also unfortunate. It's a lot of duplicate code, it uses magic strings like “NONE” to control the behavior, and the error handling is weird (never catch exceptions to print the error message on the screen, avoid magic values like “0” to indicate problems). A better approach would be something like this: public function getUserHistory($userID, $date = null) { // basic query $historySQL = ' SELECT -- always select specific columns, not just "everything" FROM history WHERE uid = :user_id '; $historyParameters = ['user_id' => $userID]; // add date condition if a date is given if ($date) { $historySQL .= ' AND date_ind = :date'; $historyParameters['date'] = $date; } $historyStmt = $this->Db->prepare($historySQL); $historyStmt->execute($historyParameters); return $historyStmt->fetchAll(); } This has no duplicate code (which makes it much shorter), it uses optional parameters instead of magic strings, and it has proper error handling. If the query fails, you get an exception, otherwise you get a valid result set (which may be empty).
  11. Jacques1
    Your error handling is broken (you check the query string $query which is always truthy), and your VALUES have no category. But, yes, do use PDO. It will save you many, many hours or writing useless code and struggling with strange problems.
  12. Jacques1
    To explicitly enumerate the status you're looking for. A condition like WHERE status IN ('NOT_STARTED', 'WORK_IN_PROGRESS') is immediately clear to anybody. There's no ambiguity whatsoever. But when you use WHERE status < 'COMPLETED' it first looks like a typo, then the reader will have to analyze your system, and finally they realize that there isn't really any system at all. The only way to understand your condition is to actually go and look up the numeric values, which is just painful. Another problem is that your numeric status field provides no data integrity. If a buggy query or a confused admin inserts something like “-1” or “999”, the database system happily accepts it, leaves you with this garbage data and may fudge up many processes within the application. A much better approach is to actually enforce a valid status, either with an ENUM type or with a foreign key pointing to a table of all status.
  13. Jacques1
    This is obviously a client-side problem, so we can stare at your code all day long, it won't help. We need specific information. Ask your friend to use a different browser and device. Ask them to record the traffic with the developer tools of their browser. For example, they should provide The complete source code of the page as received by the server The outgoing POST request with all parameters The response from the server That's something we can anaylze. Until then, really all I can say is: pretty strange.
  14. Jacques1
    Besides that, the condition se.status < ? is either wrong or very unfortunate. Even if the status attribute actually happens to have a numeric type which also happens to have meaningful “ranges”, you're relying entirely on side effects of the implementation. A status by itself isn't “smaller” or “bigger” than another status. When you use nonsensical comparisons like this, you'll quickly run into trouble as soon as the implementation changes (a different type, different numeric values, a new status etc.). If you want to check for a single status, use se.status = ? If you want to check for multiple possible status, use se.status IN (?, ?, ...)
  15. Jacques1
    What is this code? I've seen you use prepared statements, now you're suddenly back writing awful vulnerability-infested 90s amateur scripts.
  16. Jacques1
    Several things: Don't clutter your PHP scripts with HTML markup, especially when the markup is divided into countless tiny fragments and further obscured by backslash escape sequences. Use proper external mail templates. Write a helper function for sending mails. Most SMPT options are always the same, so there's no reason to go through the same PHPMailer instantiation process over and over again. Why on earth did you break TLS certificate verification? If there's a problem with your TLS connections, fix it, don't castrate the protocol. Your error handling is weird, to say the least. Why do you print internal error messages on the screen? Who is supposed to read them, and what is the reader supposed to do when all relevant information (line numbers, stack traces etc.) are missing? PHP has an error handler which is far better than any ad-hoc solution. I suggest you enable exceptions both for mysqli and for PHPMailer and let PHP do the rest. What's the purpose of the USE query? mysqli has already selected the database at this point. Are the columns really named column1, ... ?
  17. Jacques1
    Forget about the StackExchange thread, it's nonsense. PHP uses an array-like syntax to denote complex form values: <?php if ($_SERVER['REQUEST_METHOD'] == 'POST') { var_dump($_POST); } ?> <!DOCTYPE html> <html lang="en"> <head> <meta charset="utf-8"> <title>Complex form values</title> </head> <body> <form method="post"> <input type="text" name="test[3]"> <input type="text" name="test[5]"> <input type="text" name="test[10]"> <input type="submit"> </form> </body> </html> When the above form is submitted, $_POST['test'] will be an array which maps the keys 3, 5 and 10 to the corresponding field values in the form. This allows you to easily update any number of posts. Simply use the post ID as the key and then iterate over the array: <?php function html_escape($raw_input, $encoding) { return htmlspecialchars($raw_input, ENT_QUOTES | ENT_COMPAT | ENT_SUBSTITUTE, $encoding); } // test data; load the real post IDs here $post_ids = [12, 45, 69]; if ($_SERVER['REQUEST_METHOD'] == 'POST') { foreach ($_POST['post_title'] as $post_id => $post_title) { echo 'Post '.html_escape($post_id, 'UTF-8').' will get the title: '.html_escape($post_title, 'UTF-8').'<br>'; } } ?> <!DOCTYPE html> <html lang="en"> <head> <meta charset="utf-8"> <title>Complex form values</title> </head> <body> <form method="post"> <?php foreach ($post_ids as $post_id): ?> <fieldset> <legend>Post <?= html_escape($post_id, 'UTF-8') ?></legend> <label>Title: <input type="text" name="post_title[<?= html_escape($post_id, 'UTF-8') ?>]"></label> </fieldset> <?php endforeach; ?> <input type="submit"> </form> </body> </html>
  18. Jacques1
    A message like “No match” is purely presentational and thus belongs into a template, not a query. Cluttering queries with hard-coded text is a bad idea, because whenever the wording changes, you'll be busy finding and updating those strings in the code. And good luck making an application like this multi-lingual. Or rather: Just as bad.
  19. Jacques1
    You'll need to learn the JOIN syntax, particularly left joins. The obsolete syntax you're currently using only works for exact matches, it cannot handle missing rows in the session table. The default value should be implemented in the application, not the query.
  20. Jacques1
    So you're saying he should literally write getters and setters for every single attribute of every single table, plus all the code for loading and saving the data? Are you kidding? If this is your “solution”, I suggest you write the classes for DeX and maintain them for the entire lifetime of the application. Good luck.
  21. Jacques1
    Sorry, but your code is a mess. You need to stop copying and pasting, and you should understand what you want before you write anything. You've produced an impressive amount of code, but none of this really fits together. For example, where do you separate the different orders? Right now, you're collecting all items across all orders. As far as I understand, the task is this: You want to generate a CSV file with one line per order. The items of each order should be divided into two samples according to their item number. Within each sample, the items are further divided into received and completed items. This requires one query to select the order items and one loop to fetch and separate the items. Do not duplicate code when you need to perform similar actions. Whenever you feel the urge to hit ctrl+c/ctrl+v, stop and think. There's a smarter solution. You also need to sort the items by the order ID so that you can switch to a new CSV line whenever all items of one order have been processed. <?php // use constants instead of cluttering your code with hard-coded values coming out of nowhere const ORDER_DATE = '2016-07-20'; // MySQL knows that a day has 24 hours; just give it the date // when multiple tables are involved, always use *qualified* column names (i. e. table.column) so that there are no // name collisions and the reader doesn't have to guess which table the column belongs to $order_items_sql = " SELECT DISTINCT -- are you sure you need DISTINCT? research_queue.item_number, -- I'm guessing the table here order_history.location, -- I'm guessing the table here; is this column even needed? order_history.search_type -- I'm guessing the table here; is this column even needed? research_queue.action LIKE '%COMPLETED FROM REVIEW' AS completed -- Why is there no proper status field? FROM order_history JOIN research_queue ON order_history.order_or_item_number = research_queue.item_number -- you had a LEFT JOIN here, which made no sense WHERE DATE(order_history.modified_date) = ??? AND (research_queue.action LIKE '%SENT TO REVIEW' OR research_queue.action LIKE '%COMPLETED FROM REVIEW') ORDER BY order_history.order_id -- or whatever the order ID is called "; $order_items = DBHelper::execute($order_items_sql, 'test_twin', ORDER_DATE); The code must be adjusted. Since the problem is so poorly documented, I had to guess most of the time.
  22. Jacques1
    Then why didn't you say that? Learn how to issue dynamic queries and try again. It also helps to var_dump() the values right before the query so that you know what's being sent to the database system.
  23. Jacques1
    First off: The script is protected with a password or an equivalent mechanism, right? Because if anybody can send out e-mails simply by visiting the URL, that's obviously a problem. As to your question: You don't fwrite() to the script at all. This is insecure, cumbersome, error-prone and entirely unnecessary. What you do instead is use dynamic e-mail addresses and filenames. So you replace the current hard-coded strings with references like $_POST['email_address'] and $_POST['csv_path']. The script will then use whatever value has been submitted through the form instead of relying on predefined values. The form can and should be in the same file. If you don't fully understand how this works, I suggest you start with a small test script which merely echoes the form input. When this works, you can extend the script and integrate your current code.
  24. Jacques1
    My name is Jacques1. You don't seem to understand the problem at hand. DeX currently doesn't have any infrastructure for handling object-relational mapping, data persistence etc. His idea is to build an ad-hoc ORM with dozens of hand-written getters and setters for the different attributes. This is terrible and plain nonsense, because clearly this task can and should be automated. So the two sensible alternatives are to either forget about ORM altogether or use an established library which provides an actual ORM infrastructure.
  25. Jacques1
    To answer the question: No, you should not write model classes by hand, because then you'll spend an enormous amount of time and code on a trivial problem (passing data to different views). I see two possible approaches: Forget about trying to implement a “pure” version of MVC and simply write data classes which fetch the required information. The methods can return plain associative arrays, they don't have to do anything fancy. Integrate a full-blown library which has already solved the model problem (e. g. an object-relational mapper like Doctrine). The first solution is obviously the quickest and doesn't require you to learn anything new. It makes sense if your application is simple. An ORM is suitable for complex applications. In general, there are no fixed laws for how MVC must be implemented. You'll find different competing approaches, and it's ultimately up to you to choose the right one for your specific project A view should be a template written in a specialized template language like Twig or Smarty. I understand that some programmers think PHP itself is a template engine, but that's hardly true. PHP sucks at templating. It's verbose, it doesn't have any modern features (inheritance, auto-escaping, custom syntax), and it's insecure by design.
×
×
  • 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.