michael-o commented on pull request #279:
URL: 
https://github.com/apache/httpcomponents-core/pull/279#issuecomment-814362275


   > 
   > 
   > > From my PoV the entire parsing logic is broken because it tries to read 
the port before host/IP literal. Such an approach does not handle corner cases
   > 
   > This is correct, I updated the parsers to assume all ipv6 addresses are 
correctly bracketed, but I did not update them to fail if that is not the case. 
I think it's tricky because we don't have a specification for these, as they 
don't exactly match URIs. None of these classes handle percent-encoded 
hostnames either, but it's not clear if that should be considered part of the 
same bug (I can file something else for it depending how we want to move 
forward).
   
   Let's ignore percent-encoded hosts for how, this is different topic. We can 
apply common logic and document it: Virtually all software uses the bracketed 
notation when a port comes into play. There is no reason why whe shouldn't. As 
for the case where a URI is read: I would just rely on `URI#parse()` until we 
have our own RFC 3986 compliant impl. At the end it will be used in a URI 
somehow.
   
   > > would be nice to have HC be the canonical code for URI parsing and 
generation. I would expect all that to take place in one location, at least 
under the covers.
   > 
   > Host/HttpHost/URIAuthority don't appear to accept entire URIs, which means 
it may not be obvious that they should use URI bracket encoding for host/port 
separation, or that they must be strict about it. I'm all for reusing logic, 
but I'm worried that we'd inadvertently break edge cases on these other classes 
if we weren't careful.
   
   See above, make documentation explicit and it will leave no doubt.
   
   > I agree that it's very strange these three classes are nearly equivalent 
in many ways, yet have different requirements. For example, Host requires a 
port and fails otherwise, but HttpHost uses default -1.
   
   Host va HttpHost makes sense because the latter implies a specific protocol, 
the former is generic. I believe that HttpHost can be a specialized delegate to 
Host. Every little custom logic.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to