Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: wrong timeout implementation #404

Merged
merged 4 commits into from
Aug 20, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -115,11 +115,24 @@ This defaults to `urlencoded`. You can also override the default content type he

### setRequestTimeout
Set how long to wait for a request to respond, in seconds.
For Android, this will set both [connectTimeout](https://developer.android.com/reference/java/net/URLConnection#getConnectTimeout()) and [readTimeout](https://developer.android.com/reference/java/net/URLConnection#setReadTimeout(int))
For iOS, this will set [timeout interval](https://developer.apple.com/documentation/foundation/nsmutableurlrequest/1414063-timeoutinterval)
```js
cordova.plugin.http.setRequestTimeout(5.0);
```

### setConnectTimeout (Android Only)
Set connect timeout for Android
```js
cordova.plugin.http.setRequestTimeout(5.0);
```

### setReadTimeout (Android Only)
Set read timeout for Android
```js
cordova.plugin.http.setReadTimeout(5.0);
```

### setFollowRedirect<a name="setFollowRedirect"></a>
Configure if it should follow redirects automatically. This defaults to true.

Expand Down Expand Up @@ -237,7 +250,6 @@ cordova.plugin.http.sendRequest('https://google.com/', options, function(respons
}, function(response) {
// prints 403
console.log(response.status);

//prints Permission denied
console.log(response.error);
});
Expand Down
18 changes: 11 additions & 7 deletions src/android/com/silkimen/cordovahttp/CordovaHttpBase.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,34 +37,37 @@ abstract class CordovaHttpBase implements Runnable {
protected String responseType;
protected Object data;
protected JSONObject headers;
protected int timeout;
protected int connectTimeout;
protected int readTimeout;
protected boolean followRedirects;
protected TLSConfiguration tlsConfiguration;
protected CallbackContext callbackContext;

public CordovaHttpBase(String method, String url, String serializer, Object data, JSONObject headers, int timeout,
boolean followRedirects, String responseType, TLSConfiguration tlsConfiguration,
public CordovaHttpBase(String method, String url, String serializer, Object data, JSONObject headers, int connectTimeout,
int readTimeout, boolean followRedirects, String responseType, TLSConfiguration tlsConfiguration,
CallbackContext callbackContext) {

this.method = method;
this.url = url;
this.serializer = serializer;
this.data = data;
this.headers = headers;
this.timeout = timeout;
this.connectTimeout = connectTimeout;
this.readTimeout = readTimeout;
this.followRedirects = followRedirects;
this.responseType = responseType;
this.tlsConfiguration = tlsConfiguration;
this.callbackContext = callbackContext;
}

public CordovaHttpBase(String method, String url, JSONObject headers, int timeout, boolean followRedirects,
public CordovaHttpBase(String method, String url, JSONObject headers, int connectTimeout, int readTimeout, boolean followRedirects,
String responseType, TLSConfiguration tlsConfiguration, CallbackContext callbackContext) {

this.method = method;
this.url = url;
this.headers = headers;
this.timeout = timeout;
this.connectTimeout = connectTimeout;
this.readTimeout = readTimeout;
this.followRedirects = followRedirects;
this.responseType = responseType;
this.tlsConfiguration = tlsConfiguration;
Expand Down Expand Up @@ -121,7 +124,8 @@ protected HttpRequest createRequest() throws JSONException {

protected void prepareRequest(HttpRequest request) throws JSONException, IOException {
request.followRedirects(this.followRedirects);
request.readTimeout(this.timeout);
request.connectTimeout(this.connectTimeout);
request.readTimeout(this.readTimeout);
request.acceptCharset("UTF-8");
request.uncompress(true);
HttpRequest.setConnectionFactory(new OkConnectionFactory());
Expand Down
6 changes: 3 additions & 3 deletions src/android/com/silkimen/cordovahttp/CordovaHttpDownload.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@
class CordovaHttpDownload extends CordovaHttpBase {
private String filePath;

public CordovaHttpDownload(String url, JSONObject headers, String filePath, int timeout, boolean followRedirects,
TLSConfiguration tlsConfiguration, CallbackContext callbackContext) {
public CordovaHttpDownload(String url, JSONObject headers, String filePath, int connectTimeout, int readTimeout,
boolean followRedirects, TLSConfiguration tlsConfiguration, CallbackContext callbackContext) {

super("GET", url, headers, timeout, followRedirects, "text", tlsConfiguration, callbackContext);
super("GET", url, headers, connectTimeout, readTimeout, followRedirects, "text", tlsConfiguration, callbackContext);
this.filePath = filePath;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,16 @@

class CordovaHttpOperation extends CordovaHttpBase {
public CordovaHttpOperation(String method, String url, String serializer, Object data, JSONObject headers,
int timeout, boolean followRedirects, String responseType, TLSConfiguration tlsConfiguration,
int connectTimeout, int readTimeout, boolean followRedirects, String responseType, TLSConfiguration tlsConfiguration,
CallbackContext callbackContext) {

super(method, url, serializer, data, headers, timeout, followRedirects, responseType, tlsConfiguration,
super(method, url, serializer, data, headers, connectTimeout, readTimeout, followRedirects, responseType, tlsConfiguration,
callbackContext);
}

public CordovaHttpOperation(String method, String url, JSONObject headers, int timeout, boolean followRedirects,
public CordovaHttpOperation(String method, String url, JSONObject headers, int connectTimeout, int readTimeout, boolean followRedirects,
String responseType, TLSConfiguration tlsConfiguration, CallbackContext callbackContext) {

super(method, url, headers, timeout, followRedirects, responseType, tlsConfiguration, callbackContext);
super(method, url, headers, connectTimeout, readTimeout, followRedirects, responseType, tlsConfiguration, callbackContext);
}
}
4 changes: 2 additions & 2 deletions src/android/com/silkimen/cordovahttp/CordovaHttpUpload.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,11 @@ class CordovaHttpUpload extends CordovaHttpBase {
private JSONArray uploadNames;
private Context applicationContext;

public CordovaHttpUpload(String url, JSONObject headers, JSONArray filePaths, JSONArray uploadNames, int timeout,
public CordovaHttpUpload(String url, JSONObject headers, JSONArray filePaths, JSONArray uploadNames, int connectTimeout, int readTimeout,
boolean followRedirects, String responseType, TLSConfiguration tlsConfiguration,
Context applicationContext, CallbackContext callbackContext) {

super("POST", url, headers, timeout, followRedirects, responseType, tlsConfiguration, callbackContext);
super("POST", url, headers, connectTimeout, readTimeout, followRedirects, responseType, tlsConfiguration, callbackContext);
this.filePaths = filePaths;
this.uploadNames = uploadNames;
this.applicationContext = applicationContext;
Expand Down
34 changes: 19 additions & 15 deletions src/ios/CordovaHttpPlugin.m
Original file line number Diff line number Diff line change
Expand Up @@ -151,13 +151,14 @@ - (void)executeRequestWithoutData:(CDVInvokedUrlCommand*)command withMethod:(NSS

NSString *url = [command.arguments objectAtIndex:0];
NSDictionary *headers = [command.arguments objectAtIndex:1];
NSTimeInterval timeoutInSeconds = [[command.arguments objectAtIndex:2] doubleValue];
bool followRedirect = [[command.arguments objectAtIndex:3] boolValue];
NSString *responseType = [command.arguments objectAtIndex:4];
NSTimeInterval connectTimeout = [[command.arguments objectAtIndex:2] doubleValue];
NSTimeInterval readTimeout = [[command.arguments objectAtIndex:3] doubleValue];
bool followRedirect = [[command.arguments objectAtIndex:4] boolValue];
NSString *responseType = [command.arguments objectAtIndex:5];

[self setRequestSerializer: @"default" forManager: manager];
[self setRequestHeaders: headers forManager: manager];
[self setTimeout:timeoutInSeconds forManager:manager];
[self setTimeout:readTimeout forManager:manager];
[self setRedirect:followRedirect forManager:manager];
[self setResponseSerializer:responseType forManager:manager];

Expand Down Expand Up @@ -205,13 +206,14 @@ - (void)executeRequestWithData:(CDVInvokedUrlCommand*)command withMethod:(NSStri
NSDictionary *data = [command.arguments objectAtIndex:1];
NSString *serializerName = [command.arguments objectAtIndex:2];
NSDictionary *headers = [command.arguments objectAtIndex:3];
NSTimeInterval timeoutInSeconds = [[command.arguments objectAtIndex:4] doubleValue];
bool followRedirect = [[command.arguments objectAtIndex:5] boolValue];
NSString *responseType = [command.arguments objectAtIndex:6];
NSTimeInterval connectTimeout = [[command.arguments objectAtIndex:4] doubleValue];
NSTimeInterval readTimeout = [[command.arguments objectAtIndex:5] doubleValue];
bool followRedirect = [[command.arguments objectAtIndex:6] boolValue];
NSString *responseType = [command.arguments objectAtIndex:7];

[self setRequestSerializer: serializerName forManager: manager];
[self setRequestHeaders: headers forManager: manager];
[self setTimeout:timeoutInSeconds forManager:manager];
[self setTimeout:readTimeout forManager:manager];
[self setRedirect:followRedirect forManager:manager];
[self setResponseSerializer:responseType forManager:manager];

Expand Down Expand Up @@ -338,12 +340,13 @@ - (void)uploadFiles:(CDVInvokedUrlCommand*)command {
NSDictionary *headers = [command.arguments objectAtIndex:1];
NSArray *filePaths = [command.arguments objectAtIndex: 2];
NSArray *names = [command.arguments objectAtIndex: 3];
NSTimeInterval timeoutInSeconds = [[command.arguments objectAtIndex:4] doubleValue];
bool followRedirect = [[command.arguments objectAtIndex:5] boolValue];
NSString *responseType = [command.arguments objectAtIndex:6];
NSTimeInterval connectTimeout = [[command.arguments objectAtIndex:4] doubleValue];
NSTimeInterval readTimeout = [[command.arguments objectAtIndex:5] doubleValue];
bool followRedirect = [[command.arguments objectAtIndex:6] boolValue];
NSString *responseType = [command.arguments objectAtIndex:7];

[self setRequestHeaders: headers forManager: manager];
[self setTimeout:timeoutInSeconds forManager:manager];
[self setTimeout:readTimeout forManager:manager];
[self setRedirect:followRedirect forManager:manager];
[self setResponseSerializer:responseType forManager:manager];

Expand Down Expand Up @@ -398,11 +401,12 @@ - (void)downloadFile:(CDVInvokedUrlCommand*)command {
NSString *url = [command.arguments objectAtIndex:0];
NSDictionary *headers = [command.arguments objectAtIndex:1];
NSString *filePath = [command.arguments objectAtIndex: 2];
NSTimeInterval timeoutInSeconds = [[command.arguments objectAtIndex:3] doubleValue];
bool followRedirect = [[command.arguments objectAtIndex:4] boolValue];
NSTimeInterval connectTimeout = [[command.arguments objectAtIndex:3] doubleValue];
NSTimeInterval readTimeout = [[command.arguments objectAtIndex:4] doubleValue];
bool followRedirect = [[command.arguments objectAtIndex:5] boolValue];

[self setRequestHeaders: headers forManager: manager];
[self setTimeout:timeoutInSeconds forManager:manager];
[self setTimeout:readTimeout forManager:manager];
[self setRedirect:followRedirect forManager:manager];

if ([filePath hasPrefix:@"file://"]) {
Expand Down
22 changes: 22 additions & 0 deletions test/js-specs.js
Original file line number Diff line number Diff line change
Expand Up @@ -142,12 +142,32 @@ describe('Advanced HTTP public interface', function () {
it('configures global timeout value correctly with given valid value', () => {
http.setRequestTimeout(10);
http.getRequestTimeout().should.equal(10);
http.getConnectTimeout().should.equal(10);
http.getReadTimeout().should.equal(10);
});

it('throws an Error when you try to configure global timeout with a string', () => {
(() => { http.setRequestTimeout('myString'); }).should.throw(messages.INVALID_TIMEOUT_VALUE);
});

it('configures connect timeout value correctly with given valid value', () => {
http.setConnectTimeout(10);
http.getConnectTimeout().should.equal(10);
})

it('throws an Error when you try to configure connect timeout with a string', () => {
(() => { http.setConnectTimeout('myString'); }).should.throw(messages.INVALID_TIMEOUT_VALUE);
})

it('configures read timeout value correctly with given valid value', () => {
http.setReadTimeout(10);
http.getReadTimeout().should.equal(10);
})

it('throws an Error when you try to configure connect timeout with a string', () => {
(() => { http.setReadTimeout('myString'); }).should.throw(messages.INVALID_TIMEOUT_VALUE);
})

it('sets global option for following redirects correctly', () => {
http.setFollowRedirect(false);
http.getFollowRedirect().should.equal(false);
Expand Down Expand Up @@ -375,6 +395,8 @@ describe('Common helpers', function () {
serializer: 'urlencoded',
followRedirect: true,
timeout: 60.0,
connectTimeout: 30.0,
readTimeout: 30.0
}

it('adds missing "followRedirect" option correctly', () => {
Expand Down
2 changes: 2 additions & 0 deletions www/global-configs.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ var globalConfigs = {
serializer: 'urlencoded',
followRedirect: true,
timeout: 60.0,
connectTimeout: 60.0,
readTimeout: 60.0
};

module.exports = globalConfigs;
4 changes: 3 additions & 1 deletion www/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,9 @@ module.exports = function init(global, jsUtil, cookieHandler, messages, base64,
params: checkParamsObject(options.params || {}),
responseType: checkResponseType(options.responseType || validResponseTypes[0]),
serializer: checkSerializer(options.serializer || globals.serializer),
timeout: checkTimeoutValue(options.timeout || globals.timeout),
connectTimeout: checkTimeoutValue(options.connectTimeout || globals.connectTimeout),
readTimeout: checkTimeoutValue(options.readTimeout || globals.readTimeout),
timeout: checkTimeoutValue(options.timeout || globals.timeout)
};
}
};
33 changes: 28 additions & 5 deletions www/public-interface.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ module.exports = function init(exec, cookieHandler, urlUtil, helpers, globalConf
setRequestTimeout: setRequestTimeout,
getFollowRedirect: getFollowRedirect,
setFollowRedirect: setFollowRedirect,
// @Android Only
getConnectTimeout: getConnectTimeout,
setConnectTimeout: setConnectTimeout,
getReadTimeout: getReadTimeout,
setReadTimeout: setReadTimeout,
// @DEPRECATED
disableRedirect: disableRedirect,
// @DEPRECATED
Expand Down Expand Up @@ -95,6 +100,24 @@ module.exports = function init(exec, cookieHandler, urlUtil, helpers, globalConf

function setRequestTimeout(timeout) {
globalConfigs.timeout = helpers.checkTimeoutValue(timeout);
globalConfigs.connectTimeout = helpers.checkTimeoutValue(timeout);
globalConfigs.readTimeout = helpers.checkTimeoutValue(timeout);
YouYue123 marked this conversation as resolved.
Show resolved Hide resolved
}

function getConnectTimeout() {
return globalConfigs.connectTimeout;
}

function setConnectTimeout(timeout) {
globalConfigs.connectTimeout = helpers.checkTimeoutValue(timeout);
}

function getReadTimeout() {
return globalConfigs.readTimeout;
}

function setReadTimeout(timeout) {
globalConfigs.readTimeout = helpers.checkTimeoutValue(timeout);
}

function getFollowRedirect() {
Expand Down Expand Up @@ -154,18 +177,18 @@ module.exports = function init(exec, cookieHandler, urlUtil, helpers, globalConf
case 'post':
case 'put':
case 'patch':
return helpers.processData(options.data, options.serializer, function(data) {
exec(onSuccess, onFail, 'CordovaHttpPlugin', options.method, [url, data, options.serializer, headers, options.timeout, options.followRedirect, options.responseType]);
return helpers.processData(options.data, options.serializer, function (data) {
exec(onSuccess, onFail, 'CordovaHttpPlugin', options.method, [url, data, options.serializer, headers, options.connectTimeout, options.readTimeout, options.followRedirect, options.responseType]);
});
case 'upload':
var fileOptions = helpers.checkUploadFileOptions(options.filePath, options.name);
return exec(onSuccess, onFail, 'CordovaHttpPlugin', 'uploadFiles', [url, headers, fileOptions.filePaths, fileOptions.names, options.timeout, options.followRedirect, options.responseType]);
return exec(onSuccess, onFail, 'CordovaHttpPlugin', 'uploadFiles', [url, headers, options.connectTimeout, options.readTimeout, fileOptions.filePaths, fileOptions.names, options.timeout, options.followRedirect, options.responseType]);
case 'download':
var filePath = helpers.checkDownloadFilePath(options.filePath);
var onDownloadSuccess = helpers.injectCookieHandler(url, helpers.injectFileEntryHandler(success));
return exec(onDownloadSuccess, onFail, 'CordovaHttpPlugin', 'downloadFile', [url, headers, filePath, options.timeout, options.followRedirect]);
return exec(onDownloadSuccess, onFail, 'CordovaHttpPlugin', 'downloadFile', [url, headers, filePath, options.connectTimeout, options.readTimeout, options.followRedirect]);
default:
return exec(onSuccess, onFail, 'CordovaHttpPlugin', options.method, [url, headers, options.timeout, options.followRedirect, options.responseType]);
return exec(onSuccess, onFail, 'CordovaHttpPlugin', options.method, [url, headers, options.connectTimeout, options.readTimeout, options.followRedirect, options.responseType]);
}
}

Expand Down