Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 353623 - HttpExchange.setURI(String) does nothing
Summary: HttpExchange.setURI(String) does nothing
Status: RESOLVED FIXED
Alias: None
Product: Jetty
Classification: RT
Component: client (show other bugs)
Version: 7.4.5   Edit
Hardware: All All
: P3 major (vote)
Target Milestone: 7.5.x   Edit
Assignee: Michael Gorovoy CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 353619 (view as bug list)
Depends on:
Blocks:
 
Reported: 2011-08-02 13:18 EDT by Joakim Erdfelt CLA
Modified: 2011-08-29 17:31 EDT (History)
3 users (show)

See Also:


Attachments
Proposed patch (16.86 KB, patch)
2011-08-26 03:14 EDT, Michael Gorovoy CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joakim Erdfelt CLA 2011-08-02 13:18:52 EDT
The method HttpExchange.setURI(String) does nothing in so far as setting up the exchange for use.

Example:

URI baseServerURI = URI.create("https://jira.codehaus.org/")
URI uri = baseServerURI.resolve("rest/api/2.0.alpha1/project/JETTY");
HttpClient client = new HttpClient();
client.start();
HttpExchange ex = new HttpExchange();
ex.setMethod(HttpMethods.GET);
ex.setURI(uri.toASCIIString());
client.send(ex);
ex.waitForDone();

This will result in the following ...

java.net.UnknownHostException: Remote socket address cannot be null.
  at org.eclipse.jetty.client.HttpClient.getDestination(HttpClient.java:251)
  at org.eclipse.jetty.client.HttpClient.send(HttpClient.java:177)
  at experiment.JiraVersions.getProjectDetails(JiraVersions.java:121)

This is because HttpExchange.setURI(String) does not perform the same role as HttpExchange.setURL(String).

HttpExchange.setURI(String) should use the java.net.URI object and only use URI's that are absolute and opaque (with a possible test for supported scheme's)
Comment 1 Michael Gorovoy CLA 2011-08-25 13:33:44 EDT
The way this is currently implemented, calls to HttpExchange.setAddress(Address) and HttpExchange.setMethod(String) methods are needed along with a call to HttpExchange.setURI(String) method before the exchange can be sent. 

Alternatively, one can call this method after calling HttpEcxhange.reset() method in order to re-use the exchange for the same address with a different relative URI component.

We could create a method HttpExchange.setURI(URI) that will allow setting address, method, and path+query+uri fields in HttpExchange based on what parts are present in the incoming URI, allowing it to be used instead of HttpExchange.setURL(String) method, as well as set the relative URI field only if the argument is a relative URI. We can also modify HttpExchange.setURI(String) it will work the same way as the new method.

-Michael
Comment 2 Michael Gorovoy CLA 2011-08-26 02:32:16 EDT
The following methods have been added:

HttpExchange.setURL(URI) - sets Scheme, Address, and request URI
HttpExchange.setURI(URI) - sets request URI, disregards Scheme and Address
HttpExchange.setScheme(String) - convenience method

HttpExchange.setURL(String) method has been modified to use HttpExchange.setURL(URI) method.
Comment 3 Michael Gorovoy CLA 2011-08-26 02:52:54 EDT
The reason why setURI(String) had to be kept unchanged, and why setURI(URI) had been  implemented to set the request URI only is due to the way Request-URI is defined in RFC 2616 sec5. Please see http://www.w3.org/Protocols/rfc2616/rfc2616-sec5.html for more information.
Comment 4 Michael Gorovoy CLA 2011-08-26 03:14:52 EDT
Created attachment 202204 [details]
Proposed patch
Comment 5 Greg Wilkins CLA 2011-08-26 03:19:09 EDT
HttpExchange.setURL(URI) is just too confusing.

how about 

rename setURI(String) to setRequestURI(Strig)
leave setURI(String) but deprecated it

add setURI(URI) that does the same as setURL(string)

add lots of javadoc to explain.
Comment 6 Michael Gorovoy CLA 2011-08-26 03:37:21 EDT
(In reply to comment #2)
Following discussion with Greg, implementation has been changed as follows:

HttpExchange.setURI(String) has been deprecated and replaced by HttpExchange.setRequestURI(String). 
HttpExchange.setURI(URI) has been implemented to set scheme, address, and request URI from an absolute URI. 
HttpExchange.setURL(String) has been changed to delegate processing to  HttpExchange.setURI(URI) in order to be able to ensure that the URL is absolute and not opaque.
Comment 7 Michael Gorovoy CLA 2011-08-26 03:50:50 EDT
Pushed the changes as described above to master.
Comment 8 Michael Gorovoy CLA 2011-08-29 17:31:30 EDT
*** Bug 353619 has been marked as a duplicate of this bug. ***