[Bug 57767] New: Websocket client proprietary configuration

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
23 messages Options
12
Reply | Threaded
Open this post in threaded view
|

[Bug 57767] New: Websocket client proprietary configuration

Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767

            Bug ID: 57767
           Summary: Websocket client proprietary configuration
           Product: Tomcat 9
           Version: unspecified
          Hardware: PC
                OS: Linux
            Status: NEW
          Severity: enhancement
          Priority: P2
         Component: Catalina
          Assignee: [hidden email]
          Reporter: [hidden email]

The Websocket client does not provide the functionality usually found in HTTP
clients. As a result, it cannot do anything except a straight upgrade from
HTTP/1.1 to Websocket.

To handle more than this, it would need proprietary configuration to handle:
- Authentication
- Redirects

For reference about the possibilities:
https://tyrus.java.net/documentation/1.8/user-guide.html#tyrus-proprietary-config
Authentication:
https://tyrus.java.net/documentation/1.8/user-guide.html#d0e1524
Redirects: https://tyrus.java.net/documentation/1.8/user-guide.html#d0e1640

This is not a critical enhancement however as users can use their own websocket
client implementation, they don't have to rely on the one Tomcat provides.

--
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 57767] Websocket client proprietary configuration

Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767

Remy Maucherat <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P2                          |P1
          Component|Catalina                    |WebSocket

--
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 57767] Websocket client proprietary configuration

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767

Mark Thomas <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[hidden email]

--- Comment #1 from Mark Thomas <[hidden email]> ---
*** Bug 58623 has been marked as a duplicate of this bug. ***

--
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 57767] Websocket client proprietary configuration

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767

Remy Maucherat <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[hidden email]

--- Comment #2 from Remy Maucherat <[hidden email]> ---
*** Bug 59191 has been marked as a duplicate of this bug. ***

--
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 57767] Websocket client proprietary configuration

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767

--- Comment #3 from MikeLing <[hidden email]> ---
Hey, I would like to work on this issue if it's ok :) However, as a newbie  to
tomcat, could you tell me where should I look into? BTW, I had clone and set up
tomcat locally and my environment is Ubuntu16.04

Thank you very much!

--
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 57767] Websocket client proprietary configuration

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767

--- Comment #4 from Mark Thomas <[hidden email]> ---
I'd suggest supporting 302 responses as a starting point. The code should
handle both absolute and relative redirects.

There is a ready made test case here:
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/tomcat/websocket/TestWebSocketFrameClient.java?view=annotate#l100
if you restore the commented out request.

The starting point for the client code is here:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/websocket/WsWebSocketContainer.java?view=annotate#l341

--
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 57767] Websocket client proprietary configuration

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767

--- Comment #5 from MikeLing <[hidden email]> ---
(In reply to Mark Thomas from comment #4)

> I'd suggest supporting 302 responses as a starting point. The code should
> handle both absolute and relative redirects.
>
> There is a ready made test case here:
> http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/tomcat/websocket/
> TestWebSocketFrameClient.java?view=annotate#l100
> if you restore the commented out request.
>
> The starting point for the client code is here:
> http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/websocket/
> WsWebSocketContainer.java?view=annotate#l341

Sorry for the late reply. So, how do I know the Websocket client can handle
redirections as expected after I make some changes? It's my first bug in
Tomcat, so please tell me what should I do or where should I look into?  Thank
you very much! :)

--
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 57767] Websocket client proprietary configuration

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767

--- Comment #6 from Mark Thomas <[hidden email]> ---
Re-read my comment #4 regarding a suitable test case and how to activate it.

--
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 57767] Websocket client proprietary configuration

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767

J Fernandez <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[hidden email]

--- Comment #7 from J Fernandez <[hidden email]> ---
Created attachment 35193
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=35193&action=edit
websocket upgrade redirect

Hi, this is my first time submitting any open source contribution. Please let
me know if any changes are needed or if anything was missed.

Regards

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 57767] Websocket client proprietary configuration

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767

Mark Thomas <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|NEW                         |RESOLVED

--- Comment #8 from Mark Thomas <[hidden email]> ---
Thanks for the patch. It has been applied to:
- trunk for 9.0.0.M26 onwards
- 8.5.x for 8.5.20 onwards
- 8.0.x for 8.0.46 onwards
- 7.0.x for 7.0.80 onwards

There were a few minor style things I tweaked. If you enable ChekcStyle and run
the validate target it will catch most of them.

I also moved the configuration properties from System properties the user
properties. Generally, we try to avoid system properties where we can as they
can conflict in some use cases.

I merged the two properties so redirects are disabled by setting the number of
allowed redirects to 0. The default I set to 20 which is consistent with most
current browsers.

It might look like a lot of changes but they were all fairly minor. Thanks
again for the patch.

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 57767] Websocket client proprietary configuration

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767

--- Comment #9 from J Fernandez <[hidden email]> ---
Where can I learn more about CheckStyle? I assume, there is a formatting file
involved. Also, I am interested in adding support for authentication, should I
submit a patch to to this thread? Thanks for your time.

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 57767] Websocket client proprietary configuration

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767

--- Comment #10 from Mark Thomas <[hidden email]> ---
Add

execute.validate=true

to your build.properties file and run

ant validate

The configuration files are in res/checkstyle.

Please open a new bugzilla enhancement for adding authentication support. If
you need any pointers, the dev@ list is the place to ask.

Thanks again for your contribution and we are looking forward to the next one.

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 57767] Websocket client proprietary configuration

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767

Remy Maucherat <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|FIXED                       |---
             Status|RESOLVED                    |REOPENED

--- Comment #11 from Remy Maucherat <[hidden email]> ---
Reopening since authentication is not done and this was for both items, and
possibly more.

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 57767] Websocket client proprietary configuration

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767

Mark Thomas <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|[hidden email]            |

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 57767] Websocket client proprietary configuration

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767

--- Comment #12 from J Fernandez <[hidden email]> ---
Created attachment 35289
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=35289&action=edit
Authentication support

Please find below additional changes.

Added support for Basic and Digest Authentication.
Added support for dynamic loading of other Authentication Schemes.
Cleared redirectSet after invocation to allow for Container reuse.

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 57767] Websocket client proprietary configuration

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767

--- Comment #13 from Remy Maucherat <[hidden email]> ---
Ok, so that's obviously the big item (IMO), that looks good.
I'm not convinced that digest is useful anymore, do you think it is ? On the
plus side, you did it already, on the minus side we'll have to maintain the
feature.

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 57767] Websocket client proprietary configuration

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767

Christopher Schultz <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |PatchAvailable

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 57767] Websocket client proprietary configuration

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767

--- Comment #14 from J Fernandez <[hidden email]> ---
(In reply to Remy Maucherat from comment #13)
> Ok, so that's obviously the big item (IMO), that looks good.
> I'm not convinced that digest is useful anymore, do you think it is ? On the
> plus side, you did it already, on the minus side we'll have to maintain the
> feature.

There is not much need for it nowadays given how ubiquitous SSL is. That being
said, it may prove to be useful in certain circumstances. I don't think it
should require too much maintenance.

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 57767] Websocket client proprietary configuration

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767

--- Comment #15 from Christopher Schultz <[hidden email]> ---
None of the Java classes in the authentication support patch have any Javadoc.
I'm -1 on accepting the patch on that basis alone. I've skimmed the code and it
otherwise looks good, but I have not tested it at all.

For authentication, I wonder how much code can be re-used from Tomcat's
existing HTTP Basic and HTTP Digest authenticators. I'd prefer not to have
competing implementations of WWW-Authenticate handling in Tomcat.

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 57767] Websocket client proprietary configuration

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767

--- Comment #16 from Mark Thomas <[hidden email]> ---
If there is a possibility of reuse ( this is client side and the existing code
is server side) we'd need to be careful about which package / jar we put it in
to avoid adding unwanted dependencies for the WebSocket client JAR.

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

12