NotionCommotion
Members-
Posts
2,446 -
Joined
-
Last visited
-
Days Won
10
Everything posted by NotionCommotion
-
Understanding Dependency Injection
NotionCommotion replied to NotionCommotion's topic in PHP Coding Help
Wow, is that all there is to Pimple?!? I had assumed I would not see the design pattern through the bloated script, and thus started writing my own. I will spend a little time going over it, and post questions if necessary. Thank you Kicken for your help. Michael -
There are two types of escaping going on, and they are not related. real_escape_string() protects against SQL injection where the bad guy sends content which can compromise your database. See https://www.owasp.org/index.php/SQL_Injection for more. After you escaped it, the data is not changed (unless it was a bad guy attack in which it will store the escaped content, but don't worry about that), and there is nothing to escape back. htmlentities() escapes against cross-site scripting and is to protect the viewer of your site. See https://www.owasp.org/index.php/Cross-site_Scripting_(XSS) for more.
-
I think it is best to store the data in its raw format, but escape when presenting the content. That way whether inputted via the application or directly into the database, the viewer is protected. Of course, escape for SQL injection using PDO prepared statements. To escape, use the following (or better yet, start using a template engine such as Twig or Smarty). htmlentities($yourContentFromTheDB, ENT_QUOTES, 'UTF-8');
-
I am trying to understand when, why, and how to use Dependency Injection. I came up with the following hypothetical script (sorry Jacques, but there is no real script yet). If my script is totally off base, please advise. How should I apply Dependency Injection to it? After I understand how, will also be looking at when and why. Thank you <?php class Container { protected $registry = array(); public function __set($name, $resolver) { $this->registry[$name] = $resolver; } public function __get($name) { return $this->registry[$name](); } // What else should be in the container? } <?php class MyClass { public function __construct($container) { // What should be done here? echo('MyClass::__construct'); var_dump($container); } public function foo($name) { // How do I access the container? } } <?php $c = new Container; $c->pdo = function() { $pdo = new stdClass(); //This is a fictional PDO object $pdo->foo='foo for pdo'; // Do anything else... return $pdo; }; $c->bla = function() { $bla = new stdClass(); $bla->foo='foo for bla'; // Do anything else... return $bla; }; $pdo = $c->pdo; // How do I pass arguments to each? var_dump($pdo); $bla = $c->bla; var_dump($pdo); $pdo = $c->pdo; //How do I prevent creating a new PDO object, and instead use the existing? //$bad=$c->bad; //Will cause error because not defined //$c->bad='BAD'; //$bad=$c->bad; //Will cause error because not a function $myObject=new MyClass($c); //???
-
Is it considered bad practice to pass SQL script?
NotionCommotion replied to NotionCommotion's topic in PHP Coding Help
Thanks Kicken, Still a little confused. I am going to start a new post specific to Dependency Injection. -
Is it considered bad practice to pass SQL script?
NotionCommotion replied to NotionCommotion's topic in PHP Coding Help
Not sure I understand. Mind giving an example? Thank you -
Is it considered bad practice to pass SQL script?
NotionCommotion replied to NotionCommotion's topic in PHP Coding Help
Thanks Jacques, I've typically don't use a PDO wrapper, but probably should do so more often. Do you have many other methods in your wrapper which you use often? EDIT: Do you recommend creating the wrapper in a parent's constructor and assigning it to $this, and then using it as $this->PDOWrapper->execute('SELECT bla bla',[1,2,3]);? The part I was talking about not wanting to duplicate is the below part. Probably shouldn't delegate to a wrapper, right? I suppose I should do the query in the parent (with a wrapper to make it more concise) and pass the results to the child method and if the argument is false, deal with the error. if($rs=$stmt->fetch(\PDO::FETCH_OBJ)) { // Do other tasks } else { // respond with error } From your expression, I didn't think so. -
Is it considered bad practice to pass SQL script?
NotionCommotion replied to NotionCommotion's topic in PHP Coding Help
I thought you'd appreciate it The only purpose is to eliminate duplicated script common to the methods that call it, and the class is not specifically designed to be a PDO wrapper. It seemed a bit wrong to me, but I didn't know why. Why do you think it is? Maybe not very OOPish, but otherMethod() is specifically designed for one task and will only (at least today) accept one argument. I do see your reasoning, however, of using an array, and I will likely do so. Thank you both -
For example, would you frown upon this script regarding how SQL is passed? class Foo { public function method1($value) { $sql='SELECT xyz FROM mytable WHERE id=?'; $this->otherMethod($sql,$value); } public function method2($value) { $sql='SELECT xyz FROM mytable WHERE fk=? LIMIT 1'; $this->otherMethod($sql,$value); } protected function otherMethod($sql,$value) { $stmt = $this->db->prepare($sql); $stmt->execute([$value]); if($rs=$stmt->fetch(\PDO::FETCH_OBJ)) { // Do other taks } else { // respond with error } } }
-
In the top part of your code, you are using $num, but it isn't defined yet, '{$num}' is just a string, and $varWeekNow is thus NULL. Don't use $num as your name of your select menu, but make it the same name for all forms, and put another hidden input in each form with name "num", and value $num. PS. Take steps to prevent SQL injection. PSS. Use PDO.
-
Better approach to extending a class?
NotionCommotion replied to NotionCommotion's topic in PHP Coding Help
Both approaches do update just one field at a time. The potential benefit of the second approach is a bar chart controller object could have a setXAxis() method but a pie chart controller object could not have one. That being said, I am not stuck at all on doing so. I am more concerned on implementing what is typical for a REST API. -
Better approach to extending a class?
NotionCommotion replied to NotionCommotion's topic in PHP Coding Help
Ah, that makes sense. Maybe I should rethink my whole approach? Currently I am doing the following with a single endpoint: curl -i -X PUT -H "Content-Type:application/json" http://localhost:8888 /charts/1 -d '{"name":"title","value":"My Title"}' curl -i -X PUT -H "Content-Type:application/json" http://localhost:8888 /charts/1 -d '{"name":"xAxis","value":"My X Axis Title"}' Maybe I should change to the following and have different endpoints? curl -i -X PUT -H "Content-Type:application/json" http://localhost:8888 /charts/1/title -d '{"value":"My Title"}' curl -i -X PUT -H "Content-Type:application/json" http://localhost:8888 /charts/1/xAxis -d '{"value":"My X Axis Title"}' -
Better approach to extending a class?
NotionCommotion replied to NotionCommotion's topic in PHP Coding Help
I agree that calling "update"+Name methods is way to magical, however, my second (and this) example doesn't do so. What do you mean by IDEs not being able to recognize them? In regards to the unanswered question, I currently handle the values five different ways, but will likely add additional ways in the future, and wish the approach to scale. Thanks for your help and interest! class charts { public function updateParam($name, $value) { /* $this->params must include array elements [name,value] name must be one of the following ['name', 'title', 'subtitle', 'xAxis', 'yAxis', 'type', 'theme'], however, not all chart types support each (i.e. pie doesn't support [xAxis', 'yAxis']). */ $methods=$this->getAllowedUpdateMethods(); $method=isset($methods[$name])?$methods[$name]:'updateInvalid'; return $this->{$method}($value,$name); } /* Used with updateParam(). These are the default updateMethods, and child can add more */ protected function getAllowedUpdateMethods() { return [ 'name'=>'updateName', 'title'=>'updateTitles', 'subtitle'=>'updateTitles', 'type'=>'updateType', 'theme'=>'updateTheme', ]; } /* Used with updateParam(). Available to all. Updates name in DB */ protected function updateName($value,$notUsed) { if(empty($value)) { $error=\MyApp\ErrorResponse::blankValue(['Name']); } else { try { $stmt = $this->db->prepare("UPDATE charts SET name=? WHERE id=? AND accounts_id=?"); $stmt->execute([$value,$this->id,$this->account->id]); } catch(\PDOException $e){ $error=\MyApp\ErrorResponse::duplicatedName(); } } return isset($error)?$error:["status"=>"success"]; } /* Used with updateParam(). Available to all. Updates first level config option */ protected function updateTitles($value,$param) { $config=json_decode($this->config); if(!isset($config->{$param})) { $config->{$param} = new \stdClass(); } $config->{$param}->text=$value; $this->saveConfigDB($config); return ["status"=>"success"]; } /* Used with updateParam(). Available to only some chart types. Updates second level config option */ protected function updateAxis($value,$param) { $config=json_decode($this->config); if(!isset($config->{$param}->title)) { $config->{$param}->title = new \stdClass(); } $config->{$param}->title->text=$value; $this->saveConfigDB($config); return ["status"=>"success"]; } /* Used with updateParam(). Available to all. Updates type */ protected function updateType($value,$notUsed) { $sql="SELECT cth.id, cth.config, cth.type, cty.masterType FROM chart_themes cth INNER JOIN chart_types cty ON cty.type=cth.type WHERE cth.type =? LIMIT 1"; return $this->updateTypeOrTheme($value,$sql); } /* Used with updateParam(). Available to all. Updates theme */ protected function updateTheme($value,$notUsed) { $sql="SELECT cth.id, cth.config, cth.type, cty.masterType FROM chart_themes cth INNER JOIN chart_types cty ON cty.type=cth.type WHERE cth.name =?"; return $this->updateTypeOrTheme($value,$sql); } /* Used with updateParam() to respond to an invalid update method */ protected function updateInvalid($value,$name) { return ErrorResponse::invalidMethod($name); } } class BarCharts extends Charts { protected function getAllowedUpdateMethods() { return array_merge([ 'xAxis'=>'updateAxis', 'yAxis'=>'updateAxis' ],parent::getAllowedUpdateMethods()); } public function someOtherMethod() {} } -
Better approach to extending a class?
NotionCommotion replied to NotionCommotion's topic in PHP Coding Help
I will be using subclasses for other needs, so if they help, I might as well use them. Do you think I am on crack, or does the following make any sense? $charts=new BarCharts(); $charts->updateParam($_POST['value'],$_POST['attrName']); class BarCharts extends Charts { protected $allowedUpdateMethods=[ 'xAxis'=>'updateAxis', 'yAxis'=>'updateAxis' ]; } abstract class Charts { protected $allowedUpdateMethods=[ 'name'=>'updateName', 'title'=>'updateTitles', 'subtitle'=>'updateTitles' ]; protected function getAllowedMethods($name) { $allowedMethods=array_merge($this->allowedUpdateMethods,self::allowedUpdateMethods); //I expect this needs a little work... return isset($allowedMethods($name))?$allowedMethods($name):'updateInvalid'; } public function updateParam($value,$name) { return $this->{$this->allowedUpdateMethods($name)}($value,$name); } protected function updateName($value,$name) { // ... return $rs; } protected function updateTitles($value,$name) { // ... return $rs; } // This method really does not belong in all classes, but the above whitelists and prevents its use on non-applicable classes protected function updateAxis($value,$name) { // ... return $rs; } protected function updateInvalid($value,$name) { return ErrorResponse::invalidMethod(); } } -
Better approach to extending a class?
NotionCommotion replied to NotionCommotion's topic in PHP Coding Help
Maybe a better approach? $charts=new BarCharts(); $charts->updateParam($_POST['value'],$_POST['attrName']); namespace MyApp; class BarCharts extends Charts { protected function getMethod($name) { $allowed=[ 'name'=>'updateType1', 'title'=>'updateType2', 'subtitle'=>'updateType2', 'xAxis'=>'updateType3, 'yAxis'=>'updateType3' ]; return isset($allowed[$name'])?$allowed[$name']:false; } } namespace MyApp; abstract class Charts { abstract public function getMethod($name); public function updateParam($value,$name) { if($method=$this->getMethod($name) { $this->{$method}($value,$name); } else { \MyApp\ErrorResponse::invalidMethod($name); } } protected function __updateType1($value,$name) { if(empty($value)) { $error=\MyApp\ErrorResponse::blankValue([$name]); } else { try { $stmt = $this->db->prepare("UPDATE charts SET $name=? WHERE id=? AND accounts_id=?"); $stmt->execute([$value,$this->id,$this->account->id]); } catch(\PDOException $e){ $error=\MyApp\ErrorResponse::duplicatedName(); } } return isset($error)?$error:null; } protected function __updateType2($value,$name) { if(!$this->setParam($name, $value)) { $error=\MyApp\ErrorResponse::unknown(); } return isset($error)?$error:null; } protected function __updateType3($value,$name) { if(!$this->setSubParam($name, $value)) { $error=\MyApp\ErrorResponse::unknown(); } return isset($error)?$error:null; } } -
Beginner PHP email submit form
NotionCommotion replied to DarraghR's topic in Editor Help (PhpStorm, VS Code, etc)
For real simple, try something like the following. <?php if(!empty($_POST['subject']) || (!empty($_POST['message'])) ) { mail('youremail@example.com', $_POST['subject'], $_POST['message']); } ?> <!DOCTYPE html> <html> <head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> <title>ContactForm</title> </head> <body> <form method="post"> <input type="text" name="subject" /> <textarea rows="4" cols="50" name="message"></textarea> <button type="submit">Submit</button> </form> </body> </html> -
I have a bunch of classes such as BarCharts, PieCharts, etc which extend Charts. While I didn't show them, all of these classes have other methods as well. The client sends a request to update a parameter in them. Originally, I was using a switch statement to ensure the parameter in question was allowed, but found some types of charts allow for different parameters. I don't wish to duplicate the majority of script which is common to all chart types, and came up with the following, but don't really like it (and obviously don't like my use of cryptic method names such as __updateType3). Another option is to create a bunch of new classes such as BarChartsUpdate, PieChartsUpdate along with a parent class ChartsUpdate, but this doesn't seem right either. What would be a better way to do this? Thank you $charts=new BarCharts(); $charts->updateParam($_POST['value'],$_POST['attrName']); namespace MyApp; class BarCharts extends Charts { protected function _updateXAxis($value) { return $this->__updateType3($value,'xAxis'); } protected function _updateYAxis($value) { return $this->__updateType3($value,'yAxis'); } } namespace MyApp; class PieCharts extends Charts { //... } namespace MyApp; abstract class Charts { public function updateParam($value,$name) { $method='_update'.ucfirst($name); if(method_exists($this,$method)) { $error=$this->{$method}($value); } else { $error=$error=\MyApp\ErrorResponse::invalidMethod($name); } return empty($error)?["status"=>"success"]:$error; } protected function _updateName($value) { return $this->__updateType1($value,'name'); } protected function _updateTitle($value) { return $this->__updateType2($value,'title'); } protected function _updateSubtitle($value) { return $this->__updateType2($value,'subtitle'); } protected function __updateType1($value,$name) { if(empty($value)) { $error=\MyApp\ErrorResponse::blankValue([$name]); } else { try { $stmt = $this->db->prepare("UPDATE charts SET $name=? WHERE id=? AND accounts_id=?"); $stmt->execute([$value,$this->id,$this->account->id]); } catch(\PDOException $e){ $error=\MyApp\ErrorResponse::duplicatedName(); } } return isset($error)?$error:null; } protected function __updateType2($value,$name) { if(!$this->setParam($name, $value)) { $error=\MyApp\ErrorResponse::unknown(); } return isset($error)?$error:null; } protected function __updateType3($value,$name) { if(!$this->setSubParam($name, $value)) { $error=\MyApp\ErrorResponse::unknown(); } return isset($error)?$error:null; } }
-
I think I once knew about "--", but failed to remember. I really need to better commit it to memory! No, not called from the command line. Since tried it, and same results. My PHP install was off some YUM repository, and is probably the culprit. Off topic, but do you recommend compiling from source? While I am glad I learned, I don't think this solution will address my immediate needs. I wanted the debugger to magically jump to the second script and allow me to go line-by-line or skip to breakpoints, but I believe it will appear the same as a cURL required (i.e. here is your stuff). Thank you for your help!
-
Ah, much better! By the way, what does the -- do? Granted, you didn't mean to include it, but I suppose it is typically use to do something, and it is one of those things that are impossible to google. Also, any idea why I don't have a value for PHP_BINARY? Thanks!
-
Hi Ignace, Earlier, you said: if (empty($key)) { return ErrorResponseRegistry::missingKey(); // <-- proper response generated } On your latest post, are you indicating that I should do something like the following, and have the various error classes (i.e. MissingKeyErrorResponse and InvalidKeyErrorResponse) throw an exception? Maybe I misunderstood, but I thought that typically one shouldn't throw exceptions (reference http://forums.phpfreaks.com/topic/301769-determining-which-query-caused-exception-in-try-block/) try { $app->add(function(Request $request, Response $response, $next) use ($container) { $key = $request->getHeaderLine('X-MyApp-Key'); if (empty($key)) { return new MissingKeyErrorResponse(); // <-- proper response generated } $account = $this->accounts->findOneByKey($key); if (!$account) { return new InvalidKeyErrorResponse(); } $container['account'] = function() use ($account) { return $account; } return $next($request, $response); }); } catch (Exception $e) { switch ($e->getCode()) { case 400: doHandle400(); break; case 401: doHandle401(); break; case 405: doHandle405(); break; } }
-
Still nothing. One thing I found was that PHP_BINARY is set to an empty string. Any ideas why? Regardless, I hard coded the location of PHP, and tried it again, but still no. file1.php writes to the syslog, but not file2.php. What might be the problem? Thank you <?php openlog("myScriptLog", LOG_PID | LOG_PERROR, LOG_LOCAL1); syslog(LOG_INFO,"File1"); $script=__DIR__.'/file2.php'; $url='hello'; $method='GET'; $data=['a'=>1,'b'=>2, 'c'=>3,'d'=>4]; $query = $method=="GET"?http_build_query($data):''; $input = $method!="GET"?http_build_query($data):''; $args = implode(" ", array( escapeshellarg($method), escapeshellarg($url), escapeshellarg($query) )); $descriptors = array( 0 => array("pipe", "r"), 1 => array("pipe", "w"), 2 => array("file", "/tmp/error-output.txt", "a") ); // Why is PHP_BINARY set to an empty string? var_dump(PHP_BINARY); $command="/usr/bin/php -- " . escapeshellarg($script) . " " .$args; var_dump($command); $p = proc_open($command, $descriptors, $pipes); var_dump($p); var_dump($pipes); fwrite($pipes[0], $input); fclose($pipes[0]); proc_close($p); echo('File 1'); <?php openlog("myScriptLog", LOG_PID | LOG_PERROR, LOG_LOCAL1); syslog(LOG_INFO,"File2"); echo('file2'); Output of file1.php string(0) "" string(95) "/usr/bin/php -- '/var/www/datalogger/src/public/test/file2.php' 'GET' 'hello' 'a=1&b=2&c=3&d=4'" resource(4) of type (process) array(2) { [0]=> resource(2) of type (stream) [1]=> resource(3) of type (stream) } File 1
-
Hey Requinix, I have been trying to implement your solution, but am stuck. What should I witness when proc_open() is executed? Note that the second file never seems to execute. Thanks private function simulateRequest($method, $url, $data, $path_to_second_script, $keys) { syslog(LOG_INFO,"$method | $url | ".json_encode($data)); foreach($keys as $key=>$value){ $_SERVER["HTTP_X_$key"]=$value; } $query = $method=="GET"?http_build_query($data):''; $input = $method!="GET"?http_build_query($data):''; // pass most information through argv $args = implode(" ", array( //escapeshellarg($path_to_second_script), escapeshellarg($method), escapeshellarg($url), escapeshellarg($query) )); $descriptors = array( 0 => array("pipe", "r"), 1 => array("pipe", "w"), 2 => array("file", "/tmp/error-output.txt", "a") ); // launch php with the second script and write to it the post data $p = proc_open(PHP_BINARY . " -- " . escapeshellarg($path_to_second_script) . " " .$args , $descriptors, $pipes); fwrite($pipes[0], $input); fclose($pipes[0]); proc_close($p); } <?php // second script syslog(LOG_INFO,"Got here!"); list(, $method, $url, $query) = $argv; parse_str($query ?: "", $_GET); parse_str(file_get_contents("php://input"), $_POST); $_REQUEST = 123;// really you should evaluate this according to request_order // etc. echo('xxx'); //require('index.php');
-
Passing $this to a sub-function
NotionCommotion replied to NotionCommotion's topic in PHP Coding Help
Sorry for my poor choice of words for "sub-functions". I was trying to imply that these were only used to support another method. I'll go with your optional parameter approach if I think it will be necessary to accept arbitrary IDs.