Bug 57154 - Failure of TestWsWebSocketContainer when directory %TEMP%\test exists
Failure of TestWsWebSocketContainer when directory %TEMP%\test exists
Status: RESOLVED FIXED
Product: Tomcat 7
Classification: Unclassified
Component: WebSocket
trunk
PC All
: P2 minor (vote)
: ---
Assigned To: Tomcat Developers Mailing List
:
Depends on:
Blocks:
  Show dependency tree
 
Reported: 2014-10-27 22:07 UTC by Konstantin Kolinko
Modified: 2016-01-17 05:17 UTC (History)
1 user (show)



Attachments
TEST-org.apache.tomcat.websocket.TestWsWebSocketContainer.NIO.txt (38.81 KB, text/plain)
2014-10-27 22:10 UTC, Konstantin Kolinko
Details
TEST-org.apache.tomcat.websocket.TestWsWebSocketContainer.NIO2.txt (39.90 KB, text/plain)
2014-10-27 22:10 UTC, Konstantin Kolinko
Details
backport the feature from bug 57155 to Tomcat 7.0.x (110.25 KB, patch)
2015-05-22 01:39 UTC, Huxing Zhang
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Konstantin Kolinko 2014-10-27 22:07:01 UTC
Steps to reproduce:
1) Go to the temporary directory (System.getProperty("java.io.tmpdir")) and create a directory named "test" there.
2) Run org.apache.tomcat.websocket.TestWsWebSocketContainer test case with Nio or Nio 2.

The following two test cases are failing: 
testWriteTimeoutServerEndpoint
testWriteTimeoutServerContainer

Cause of the failure is the following ERROR:
The HTTP response from the server [HTTP/1.1 302 Found
] did not permit the HTTP upgrade to WebSocket
javax.websocket.DeploymentException: The HTTP response from the server [HTTP/1.1 302 Found
] did not permit the HTTP upgrade to WebSocket

This test case configures DefaultServlet and tries to access
"ws://localhost:" + getPort() + "/test".

I suspect that Mapper responds with 302 redirect before the upgrade request reaches web application.

This issue is observed at Buildbot. See
TEST-org.apache.tomcat.websocket.TestWsWebSocketContainer.NIO.txt
TEST-org.apache.tomcat.websocket.TestWsWebSocketContainer.NIO2.txt
files in http://ci.apache.org/projects/tomcat/tomcat8/logs/1634329/
Comment 1 Konstantin Kolinko 2014-10-27 22:10:15 UTC
Created attachment 32152 [details]
TEST-org.apache.tomcat.websocket.TestWsWebSocketContainer.NIO.txt

Log file from testing Tomcat 8 trunk (@r1634690)
Comment 2 Konstantin Kolinko 2014-10-27 22:10:58 UTC
Created attachment 32153 [details]
TEST-org.apache.tomcat.websocket.TestWsWebSocketContainer.NIO2.txt

Log file from testing Tomcat 8 trunk (@r1634690)
Comment 3 Mark Thomas 2014-10-29 15:01:15 UTC
The root cause of this is indeed the redirect from the Mapper.

The redirect is a result of one of the requirements for welcome file processing. The Servlet spec is clear Tomcat can implement these any way it likes so the current Tomcat behaviour is valid.

It looks like the best solution for this bug is a fix for bug 57155.
Comment 4 Mark Thomas 2014-10-29 22:22:02 UTC
Fixed with the fixes for bug 57155.
Comment 5 Konstantin Kolinko 2014-10-31 10:36:21 UTC
This test failure is reproducible with current Tomcat 7.0.x testsuite.
Note that it does not happen by default, but only if one creates a %TEMP%\test directory.

Possible solutions:
a) Backport the feature from bug 57155 (for reference: r1635222).
b) Reconfigure the tests to use some different temporary directory as docBase instead of the system-wide one.
c) Improve Mapper.
d) Close as WONTFIX.

a) is the best one, but requires some work as resource implementations differ between Tomcat 8 and 7.

b) is an alternative way if a) is too hard.

c) unlikely without further analysis.

d) possible. A test configuration issue is not a show stopper.

I am reopening this for Tomcat 7 now, so that it is not forgotten.
Comment 6 Huxing Zhang 2015-05-22 01:39:18 UTC
Created attachment 32749 [details]
backport the feature from bug 57155 to Tomcat 7.0.x

Hi @Konstantin and @Mark, 

I have backported the feature from bug 57155 to Tomcat 7.0.x.
Would you please kindly review the attached fix?
The main idea is borrowed from Mark's implementation in Tomcat 8.0.x.

The changes in the patch mainly includes:
1) an EmptyDirContext implementation which is not backed by a file system.
2) an enhancement in StandardContext that allows an application to be configured to have no docBase, precisely speaking, to have docBase to be null.
3) changes in test cases that replacing unnecessary file system doc base with null doc base.
Comment 7 Huxing Zhang 2015-05-27 01:26:25 UTC
Hi @Konstantin and @Mark, 

Is there any update on this?
Comment 8 Mark Thomas 2015-05-27 10:11:56 UTC
Thanks for the patch. It has been applied (with a few minor tweaks to silence some Eclipse warnings) to 7.0.x and will be included in 7.0.63 onwards.
Comment 9 Konstantin Kolinko 2016-01-17 04:06:04 UTC
(In reply to Mark Thomas from comment #8)
> Thanks for the patch. It has been applied (with a few minor tweaks to
> silence some Eclipse warnings) to 7.0.x and will be included in 7.0.63
> onwards.

For a reference, implementation of this feature in Tomcat 7 is
r1681953 + r1724913

Comments working on backport to Tomcat 6
==========================

1). Currently null is an unsupported value for StandardContext.docBase.

Using the null value is going to fail with a NullPointerException thrown
from a "new File(null)" call.

So implementing support for a null docBase is a new feature
and does not break any existing one.


2). There is a slight difference with Tomcat 7 that ApplicationContext and ReplicatedContext$ReplApplContext constructors accept additional argument basePath.

    public ServletContext getServletContext() {
        if (context == null) {
            context = new ApplicationContext(getBasePath(), this);

This resulted in a stacktrace like this:

  java.lang.NullPointerException
     at java.io.File.<init>(File.java:194)
     at org.apache.catalina.core.StandardContext.getBasePath(StandardContext.java:5179)
     at org.apache.catalina.core.StandardContext.getServletContext(StandardContext.java:1932)
     at org.apache.catalina.core.StandardContext.postWorkDirectory(StandardContext.java:5448)
     at org.apache.catalina.core.StandardContext.start(StandardContext.java:4563)

The basePath value is used to implement methods ApplicationContext.getRealPath(String)
and getResource(String).

(For historic reference: commits that removed basePath field from ApplicationContext
in Tomcat 7 are r784083 + r834220)

In both methods the use of basePath is conditioned on "if (context.isFilesystemBased())"
condition. So if StandardContext is not based on a filesystem, the value of basePath in ApplicationContext does not matter.

Thus I am going to change StandardContext.getBasePath() to return null for a null docBase,
instead of failing with a NullPointerException.
Comment 10 Konstantin Kolinko 2016-01-17 05:17:44 UTC
Implemented in Tomcat 6 by r1725061 and will be in 6.0.45 onwards.