Bug 63753 - unnecessary websocket request host header port number checking
Summary: unnecessary websocket request host header port number checking
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 9
Classification: Unclassified
Component: WebSocket (show other bugs)
Version: 9.0.x
Hardware: All All
: P2 trivial (vote)
Target Milestone: -----
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-09-18 08:34 UTC by jongin kim
Modified: 2019-09-19 13:56 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description jongin kim 2019-09-18 08:34:18 UTC
WsWebSocketContainer::createRequestHeaders intends to check whether the port number is -1 or not.

        // Host header
        List<String> hostValues = new ArrayList<>(1);
        if (port == -1) {
            hostValues.add(host);
        } else {
            hostValues.add(host + ':' + port);
        }

However, before createRequestHeaders method is called, the port number is set to 80 and 443 in connectToServerRecursive method. The value of port cannot be -1.

        if (port == -1) {
            if ("ws".equalsIgnoreCase(scheme)) {
                port = 80;
            } else {
                // Must be wss due to scheme validation above
                port = 443;
            }
        }

I think it is side effect of revision 230c1083fd9f10ec50ccd1d15032f2df29e4de2c.
(git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@1780109 13f79535-47bb-0310-9956-ffa450edef68). It is better to remove the code.
Comment 1 Mark Thomas 2019-09-19 13:56:51 UTC
The original purpose of the code was to include the port in the HTTP host header if a non-standard port was used. That requirement still exists but the refactoring to support proxing broke the previous implementation. I have now fixed this. Thanks for reporting this issue.

Fixed in:
- master for 9.0.27 onwards
- 8.5.x for 8.5.47 onwards
- 7.0.x for 7.0.97 onwards