[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