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

connectionTimeOut and readTimeout not working on UrlResource #28909

Closed
pinkeshsagar-harptec opened this issue Aug 2, 2022 · 4 comments
Closed
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: bug A general bug
Milestone

Comments

@pinkeshsagar-harptec
Copy link

pinkeshsagar-harptec commented Aug 2, 2022

I am trying to terminate a connection if no data is being received or server is just keeping the connection open for a url by setting value for connectionTimeout and readTimeout but it is not working.

Spring boot version 2.7.1

Here is the link to SO https://stackoverflow.com/questions/73165245/setting-connectiontimeout-and-readtimeout-not-working-on-urlresource

Below code reproduces the issue.

try {
  URL url = new URL("http://httpstat.us/200?sleep=20000");
  UrlResource urlResource = new UrlResource(url) {

@Override
    protected void customizeConnection(HttpURLConnection connection) throws IOException {
      super.customizeConnection(connection);
      connection.setConnectTimeout(4000);
      connection.setReadTimeout(2000);
    }
  };

  InputStream inputStream = urlResource.getInputStream();
  InputStreamReader isr = new InputStreamReader(inputStream,
      StandardCharsets.UTF_8);
  BufferedReader br = new BufferedReader(isr);
  br.lines().forEach(line -> System.out.println(line));
} catch (MalformedURLException e) {
  e.printStackTrace();
} catch (IOException e) {
  System.out.println("IO exception");
  e.printStackTrace();
}

Expected:- both connectionTimeout and readTimeout should work as per given value and terminate the connection if time exceeds the timeout value

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Aug 2, 2022
@snicoll
Copy link
Member

snicoll commented Aug 2, 2022

The javadoc of customizeConnection states:

Customize the given HttpURLConnection, obtained in the course of an exists(), contentLength() or lastModified() call.

This override of yours not being called is working as designed. Let's see what the rest of the team thinks on the use case above.

@snicoll snicoll added this to the Triage Queue milestone Aug 2, 2022
@bclozel
Copy link
Member

bclozel commented Aug 2, 2022

While the javadoc for AbstractFileResolvingResource only mentions methods implemented on that class, it could make sense to also call this for getInputStream and update the javadoc accordingly; this looks inconsistent as all other remote calls are using this method.

On the other hand, the default implementation sets the HTTP method to "HEAD", this would not work for getInpustream and changing that would clash with the other methods.

@artembilan
Copy link
Member

The customizeConnection() is also called from the checkReadable() which, in turn, is called from the isReadable().
And this one is not mentioned in the customizeConnection() JavaDoc.

I don't think it is a big deal to set a proper HTTP method in that getInpustream() after calling customizeConnection() to override that default HEAD.

Maybe the pattern is to use an exist() or isReadable() before calling getInpustream() therefore that readTimeout would have some effect?
And perhaps the connection is reused from the cache...

@bclozel bclozel self-assigned this Aug 3, 2022
@bclozel bclozel added type: bug A general bug in: core Issues in core modules (aop, beans, core, context, expression) and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Aug 3, 2022
@bclozel bclozel modified the milestones: Triage Queue, 5.3.23 Aug 3, 2022
@bclozel
Copy link
Member

bclozel commented Aug 3, 2022

Given the conversation above, both AbstractFileResolvingResource and UrlResource have inconsistencies:

  • the customizeConnection(HttpURLConnection) default implementation can only be used by methods that won't need to fetch the actual content of the resource because it is using HTTP HEAD requests.
  • AbstractFileResolvingResource sub-classes (like UrlResource) do not always call the customizeConnection methods which makes the experience inconsistent; depending on the method used, timeouts or request headers will be different

While this behavior is documented in the javadoc, as it lists the methods delegating to the customization method, the developers expectations can be quite different when sub-classing a Resource implementation: the name of the method itself gives a false indication.

We can change this behavior without introducing breaking changes for sub-classes: we can set the HTTP request method after the customization call for each method, as it is its responsibility in the first place.

@bclozel bclozel closed this as completed in 0caa2ac Aug 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: bug A general bug
Projects
None yet
Development

No branches or pull requests

5 participants