[PEAR-BUG] Bug #20421 [Asn]: Reject invalid path
[email protected] Wed, 8 Oct 2014 11:43:59 +0100 (BST)
| Newsgroups | php.pear.bugs |
|---|---|
| Message-ID | <[email protected]> |
Edit report at https://pear.php.net/bugs/bug.php?id=20421&edit=1 ID: 20421 Updated by: [email protected] Reported By: jan dot prachar at gmail dot com Summary: Reject invalid path Status: Assigned Type: Bug Package: Net_URL2 Package Version: 2.0.7 PHP Version: Irrelevant Assigned To: tkli Roadmap Versions: New Comment: Another point of view. Let's say we want to fullfil following: $u = new Net_URL2($url); $u->setPath($path); $this->assertSame($path, (new Net_URL2($u->getUrl()))->getPath()); For url 'http:g' and path '//h' you get that '//h' is not equal to false For url 'http://example.com' and path 'h' you get that 'h' is not equal to false or (after the fix #20159) 'h' is not equal to '/h' Since there is now way to get this test passing in these two examples I think it is legit to make all setters private. Is there any valid use case for them anyway? But it would be a major bc break... Previous Comments: ------------------------------------------------------------------------ [2014-10-08 01:54:18] tkli -Status: Feedback +Status: Open ------------------------------------------------------------------------ [2014-10-08 01:52:36] tkli thanks for sharing, I did overlook the setPath() call in the actual example code. however the problem still remains: if you set "//h" as path and there is no authority, Net_URL2 actually leaves that up to the user. As it starts with "//" and with no authority, this denotes "h" as the new authority and no path. not saying this is a good way to deal with it, i need to think about this a little. exception throwing is not my preferred option to solve this one. using a method with "path" in it's name to change the authority is not fine, too. as you rightfully say, this is inconsistent. ------------------------------------------------------------------------ [2014-10-08 01:10:44] pracj3am At least now you are inconsistent in handling invalid path in case of present and missing authority. ------------------------------------------------------------------------ [2014-10-08 01:09:01] pracj3am My motivation was seeing this line of code: https://github.com/pear/Net_URL2/blob/master/Net/URL2.php#L728 and this bug report https://pear.php.net/bugs/bug.php?id=20159 Here you are modifying a path in order to get a valid URI. And I think that bug #20159 is not bug at all. Th query and fragment has separators (? and #), but path has not. I think you should compose URI from the parts as they were set and leave the responsiblity to user of the library (or notify him by throwing an exception). Thanks! ------------------------------------------------------------------------ [2014-10-08 00:15:22] tkli -Status: Open +Status: Feedback -Assigned To: +Assigned To: tkli what you write is correct in terms of RFC 3986 but unfortunately Net_URL2 is not about URI validation (in full or in part). therefore throwing an exception while calling Net_URL2::setPath() (or Net_URL2::getUrl() and those by yours correctly named authority counterparts) could be (at least) unexpected or just straight forward introduce a new feature. if you share more about your motivation, it might be possible to find a beautiful way to keep Net_URL2 open for extension but closed for modification offering good options for your validation needs. me seeing more context would also allow me to better understand why you qualified this as a bug. feedback welcome. -- tk ------------------------------------------------------------------------ The remainder of the comments for this report are too long. To view the rest of the comments, please view the bug report online at http://pear.php.net/bugs/bug.php?id=20421 -- Edit this bug report at https://pear.php.net/bugs/bug.php?id=20421&edit=1