Fixes issue #1345; calling XHR.open() with null arguments for user and password would cause a "null"/"null" login to be sent to the server on Opera. The fix is to only pass exactly as many arguments to open() as matter. I made three different overloads for the JSNI methods because in most cases (no auth) the compiler will be able to optimize out the 4 and 5 arg versions.
Found by: joerg.baumann
Review by: mmendez
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@1262 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/user/src/com/google/gwt/http/client/RequestBuilder.java b/user/src/com/google/gwt/http/client/RequestBuilder.java
index 1daefaa..5d57570 100644
--- a/user/src/com/google/gwt/http/client/RequestBuilder.java
+++ b/user/src/com/google/gwt/http/client/RequestBuilder.java
@@ -157,12 +157,26 @@
*/
public Request sendRequest(String requestData, RequestCallback callback)
throws RequestException {
- JavaScriptObject xmlHttpRequest = httpRequest.createXmlHTTPRequest();
- String openError = XMLHTTPRequest.open(xmlHttpRequest, httpMethod, url,
- true, user, password);
+ if (user == null && password != null) {
+ throw new IllegalStateException("A password is set, but no user is set");
+ }
+
+ JavaScriptObject xmlHttpRequest = httpRequest.createXmlHTTPRequest();
+ String openError;
+ if (password != null) {
+ openError = XMLHTTPRequest.open(xmlHttpRequest, httpMethod, url, true,
+ user, password);
+ } else if (user != null) {
+ openError = XMLHTTPRequest.open(xmlHttpRequest, httpMethod, url, true,
+ user);
+ } else {
+ openError = XMLHTTPRequest.open(xmlHttpRequest, httpMethod, url, true);
+ }
if (openError != null) {
- throw new RequestPermissionException(url);
+ RequestPermissionException requestPermissionException = new RequestPermissionException(url);
+ requestPermissionException.initCause(new RequestException(openError));
+ throw requestPermissionException;
}
setHeaders(xmlHttpRequest);
diff --git a/user/src/com/google/gwt/http/client/XMLHTTPRequest.java b/user/src/com/google/gwt/http/client/XMLHTTPRequest.java
index 48e7374..53898e7 100644
--- a/user/src/com/google/gwt/http/client/XMLHTTPRequest.java
+++ b/user/src/com/google/gwt/http/client/XMLHTTPRequest.java
@@ -168,6 +168,49 @@
* @param httpMethod the method to use for open call
* @param url the URL to use for the open call
* @param async true if we should do an asynchronous open
+ * @return error message if an exception is thrown or null if there is none
+ */
+ static native String open(JavaScriptObject xmlHttpRequest, String httpMethod,
+ String url, boolean async) /*-{
+ try {
+ xmlHttpRequest.open(httpMethod, url, async);
+ return null;
+ } catch (e) {
+ return e.message;
+ }
+ }-*/;
+
+ /**
+ * Opens the request and catches any exceptions thrown. If an exception is
+ * caught, its string representation will be returned. This is the only signal
+ * that an error has occurred.
+ *
+ * @param xmlHttpRequest JavaScript <code>XmlHttpRequest</code> object
+ * @param httpMethod the method to use for open call
+ * @param url the URL to use for the open call
+ * @param async true if we should do an asynchronous open
+ * @param user user to use in the URL
+ * @return error message if an exception is thrown or null if there is none
+ */
+ static native String open(JavaScriptObject xmlHttpRequest, String httpMethod,
+ String url, boolean async, String user) /*-{
+ try {
+ xmlHttpRequest.open(httpMethod, url, async, user);
+ return null;
+ } catch (e) {
+ return e.message;
+ }
+ }-*/;
+
+ /**
+ * Opens the request and catches any exceptions thrown. If an exception is
+ * caught, its string representation will be returned. This is the only signal
+ * that an error has occurred.
+ *
+ * @param xmlHttpRequest JavaScript <code>XmlHttpRequest</code> object
+ * @param httpMethod the method to use for open call
+ * @param url the URL to use for the open call
+ * @param async true if we should do an asynchronous open
* @param user user to use in the URL
* @param password password to use in the URL
* @return error message if an exception is thrown or null if there is none
@@ -178,7 +221,7 @@
xmlHttpRequest.open(httpMethod, url, async, user, password);
return null;
} catch (e) {
- return e.toString();
+ return e.message;
}
}-*/;
@@ -199,7 +242,7 @@
return null;
} catch (e) {
xmlHttpRequest.onreadystatechange = @com.google.gwt.user.client.impl.HTTPRequestImpl::nullFunc;
- return e.toString();
+ return e.message;
}
}-*/;
@@ -209,7 +252,7 @@
xmlHttpRequest.setRequestHeader(header, value);
return null;
} catch (e) {
- return e.toString();
+ return e.message;
}
}-*/;
diff --git a/user/test/com/google/gwt/http/client/RequestBuilderTest.java b/user/test/com/google/gwt/http/client/RequestBuilderTest.java
index b8e1ff8..7e42770 100644
--- a/user/test/com/google/gwt/http/client/RequestBuilderTest.java
+++ b/user/test/com/google/gwt/http/client/RequestBuilderTest.java
@@ -19,7 +19,7 @@
import com.google.gwt.junit.client.GWTTestCase;
/**
- * HTTPRequestBuilder tests
+ * HTTPRequestBuilder tests.
*/
public class RequestBuilderTest extends GWTTestCase {
private static final int TEST_FINISH_DELAY = 10000;
@@ -55,9 +55,9 @@
* <li>url == "www.freebsd.org" - violates same source
* </ul>
*/
- public void testRequestBuilderStringString() {
+ public void testRequestBuilderStringString() throws RequestException {
try {
- RequestBuilder builder = new RequestBuilder((RequestBuilder.Method) null,
+ new RequestBuilder((RequestBuilder.Method) null,
null);
fail("NullPointerException should have been thrown for construction with null method.");
} catch (NullPointerException ex) {
@@ -65,14 +65,14 @@
}
try {
- RequestBuilder builder = new RequestBuilder(RequestBuilder.GET, null);
+ new RequestBuilder(RequestBuilder.GET, null);
fail("NullPointerException should have been thrown for construction with null URL.");
} catch (NullPointerException ex) {
// purposely ignored
}
try {
- RequestBuilder builder = new RequestBuilder(RequestBuilder.GET, "");
+ new RequestBuilder(RequestBuilder.GET, "");
fail("IllegalArgumentException should have been throw for construction with empty URL.");
} catch (IllegalArgumentException ex) {
// purposely ignored
@@ -96,8 +96,6 @@
// purposely ignored
} catch (RequestPermissionException ex) {
// this is the type of exception that we expect
- } catch (RequestException e) {
- fail(e.getMessage());
}
}
@@ -106,70 +104,58 @@
* {@link com.google.gwt.http.client.RequestBuilder#RequestBuilder(java.lang.String, java.lang.String)}. *
*/
public void testRequestBuilderStringString_HTTPMethodRestrictionOverride() {
- RequestBuilder builder = new RequestBuilder(RequestBuilder.GET, "FOO");
+ new RequestBuilder(RequestBuilder.GET, "FOO");
- try {
- class MyRequestBuilder extends RequestBuilder {
- MyRequestBuilder(String httpMethod, String url) {
- super(httpMethod, url);
- }
- };
+ class MyRequestBuilder extends RequestBuilder {
+ MyRequestBuilder(String httpMethod, String url) {
+ super(httpMethod, url);
+ }
+ };
- builder = new MyRequestBuilder("HEAD", "FOO");
- // should reach here without any exceptions being thrown
- } catch (IllegalArgumentException ex) {
- fail(ex.getMessage());
- }
+ new MyRequestBuilder("HEAD", "FOO");
+ // should reach here without any exceptions being thrown
}
/**
* Test method for
* {@link com.google.gwt.http.client.RequestBuilder#sendRequest(java.lang.String, com.google.gwt.http.client.RequestCallback)}.
*/
- public void testSendRequest_GET() {
+ public void testSendRequest_GET() throws RequestException {
delayTestFinish(TEST_FINISH_DELAY);
- try {
- RequestBuilder builder = new RequestBuilder(RequestBuilder.GET,
- getTestBaseURL() + "sendRequest_GET");
- builder.sendRequest(null, new RequestCallback() {
- public void onError(Request request, Throwable exception) {
- fail();
- }
+ RequestBuilder builder = new RequestBuilder(RequestBuilder.GET,
+ getTestBaseURL() + "sendRequest_GET");
+ builder.sendRequest(null, new RequestCallback() {
+ public void onError(Request request, Throwable exception) {
+ fail();
+ }
- public void onResponseReceived(Request request, Response response) {
- assertEquals(200, response.getStatusCode());
- finishTest();
- }
- });
- } catch (RequestException e) {
- fail(e.getMessage());
- }
+ public void onResponseReceived(Request request, Response response) {
+ assertEquals(200, response.getStatusCode());
+ finishTest();
+ }
+ });
}
/**
* Test method for
* {@link com.google.gwt.http.client.RequestBuilder#sendRequest(java.lang.String, com.google.gwt.http.client.RequestCallback)}.
*/
- public void testSendRequest_POST() {
+ public void testSendRequest_POST() throws RequestException {
delayTestFinish(TEST_FINISH_DELAY);
- try {
- RequestBuilder builder = new RequestBuilder(RequestBuilder.POST,
- getTestBaseURL() + "sendRequest_POST");
- builder.sendRequest("method=test+request", new RequestCallback() {
- public void onError(Request request, Throwable exception) {
- fail("HTTPRequest timed out");
- }
+ RequestBuilder builder = new RequestBuilder(RequestBuilder.POST,
+ getTestBaseURL() + "sendRequest_POST");
+ builder.sendRequest("method=test+request", new RequestCallback() {
+ public void onError(Request request, Throwable exception) {
+ fail("HTTPRequest timed out");
+ }
- public void onResponseReceived(Request request, Response response) {
- assertEquals(200, response.getStatusCode());
- finishTest();
- }
- });
- } catch (RequestException e) {
- fail(e.getMessage());
- }
+ public void onResponseReceived(Request request, Response response) {
+ assertEquals(200, response.getStatusCode());
+ finishTest();
+ }
+ });
}
public void testSetPassword() {
@@ -201,7 +187,7 @@
* <li>value == ""
* </ul>
*/
- public void testSetRequestHeader() {
+ public void testSetRequestHeader() throws RequestException {
RequestBuilder builder = new RequestBuilder(RequestBuilder.GET,
getTestBaseURL() + "setRequestHeader");
@@ -235,25 +221,21 @@
delayTestFinish(TEST_FINISH_DELAY);
- try {
- builder = new RequestBuilder(RequestBuilder.GET, getTestBaseURL()
- + "setRequestHeader");
- builder.setHeader("Foo", "Bar");
- builder.setHeader("Foo", "Bar1");
+ builder = new RequestBuilder(RequestBuilder.GET, getTestBaseURL()
+ + "setRequestHeader");
+ builder.setHeader("Foo", "Bar");
+ builder.setHeader("Foo", "Bar1");
- builder.sendRequest(null, new RequestCallback() {
- public void onError(Request request, Throwable exception) {
- fail("HTTPRequest timed out");
- }
+ builder.sendRequest(null, new RequestCallback() {
+ public void onError(Request request, Throwable exception) {
+ fail("HTTPRequest timed out");
+ }
- public void onResponseReceived(Request request, Response response) {
- assertEquals(200, response.getStatusCode());
- finishTest();
- }
- });
- } catch (RequestException e) {
- fail(e.getMessage());
- }
+ public void onResponseReceived(Request request, Response response) {
+ assertEquals(200, response.getStatusCode());
+ finishTest();
+ }
+ });
}
/**
@@ -267,26 +249,22 @@
* <li>Timeout is less than the server's response time
* </ul>
*/
- public void testSetTimeout_noTimeout() {
+ public void testSetTimeout_noTimeout() throws RequestException {
delayTestFinish(TEST_FINISH_DELAY);
- try {
- RequestBuilder builder = new RequestBuilder(RequestBuilder.GET,
- getTestBaseURL() + "setTimeout/noTimeout");
- builder.setTimeoutMillis(10000);
- builder.sendRequest(null, new RequestCallback() {
- public void onError(Request request, Throwable exception) {
- fail("Test timed out");
- }
+ RequestBuilder builder = new RequestBuilder(RequestBuilder.GET,
+ getTestBaseURL() + "setTimeout/noTimeout");
+ builder.setTimeoutMillis(10000);
+ builder.sendRequest(null, new RequestCallback() {
+ public void onError(Request request, Throwable exception) {
+ fail("Test timed out");
+ }
- public void onResponseReceived(Request request, Response response) {
- assertEquals(200, response.getStatusCode());
- finishTest();
- }
- });
- } catch (RequestException e) {
- fail(e.getMessage());
- }
+ public void onResponseReceived(Request request, Response response) {
+ assertEquals(200, response.getStatusCode());
+ finishTest();
+ }
+ });
}
/**
@@ -300,26 +278,22 @@
* <li>Timeout is less than the server's response time
* </ul>
*/
- public void testSetTimeout_timeout() {
+ public void testSetTimeout_timeout() throws RequestException {
delayTestFinish(TEST_FINISH_DELAY);
- try {
- RequestBuilder builder = new RequestBuilder(RequestBuilder.GET,
- getTestBaseURL() + "setTimeout/timeout");
- builder.setTimeoutMillis(2000);
- builder.sendRequest(null, new RequestCallback() {
- public void onError(Request request, Throwable exception) {
- finishTest();
- }
+ RequestBuilder builder = new RequestBuilder(RequestBuilder.GET,
+ getTestBaseURL() + "setTimeout/timeout");
+ builder.setTimeoutMillis(2000);
+ builder.sendRequest(null, new RequestCallback() {
+ public void onError(Request request, Throwable exception) {
+ finishTest();
+ }
- public void onResponseReceived(Request request, Response response) {
- assertEquals(200, response.getStatusCode());
- fail("Test did not timeout");
- }
- });
- } catch (RequestException e) {
- fail(e.getMessage());
- }
+ public void onResponseReceived(Request request, Response response) {
+ assertEquals(200, response.getStatusCode());
+ fail("Test did not timeout");
+ }
+ });
}
public void testSetUser() {
