You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by bu...@apache.org on 2015/03/27 15:53:32 UTC
[Bug 57767] New: Websocket client proprietary configuration
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: dev@tomcat.apache.org
Reporter: remm@apache.org
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: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 57767] Websocket client proprietary configuration
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767
--- Comment #22 from Remy Maucherat <re...@apache.org> ---
Yes, that would be a big problem with my "simplification" then. Ooops. I will
restore the separate client class, it's a good solution for the issue.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 57767] Websocket client proprietary configuration
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767
--- Comment #3 from MikeLing <sa...@gmail.com> ---
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: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 57767] Websocket client proprietary configuration
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767
--- Comment #17 from Remy Maucherat <re...@apache.org> ---
Yes, I would rather integrate it (if it works) then see about reuse. I also
don't think javadoc is a big issue either for this kind of code.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 57767] Websocket client proprietary configuration
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767
--- Comment #5 from MikeLing <sa...@gmail.com> ---
(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: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 57767] Websocket client proprietary configuration
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767
Mark Thomas <ma...@apache.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |markt@apache.org
--- Comment #1 from Mark Thomas <ma...@apache.org> ---
*** 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: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 57767] Websocket client proprietary configuration
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767
--- Comment #20 from Remy Maucherat <re...@apache.org> ---
I committed the patch to trunk, with a few changes:
- Adding javadocs
- Merged all client code back to WsWebSocketContainer (the new client class was
taking over nearly all its code so I didn't really see the point)
If nobody complains about other mandatory improvements, I will proceed with
backporting the feature addition.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 57767] Websocket client proprietary configuration
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767
--- Comment #16 from Mark Thomas <ma...@apache.org> ---
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: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 57767] Websocket client proprietary configuration
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767
--- Comment #10 from Mark Thomas <ma...@apache.org> ---
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: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 57767] Websocket client proprietary configuration
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767
--- Comment #4 from Mark Thomas <ma...@apache.org> ---
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: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 57767] Websocket client proprietary configuration
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767
Remy Maucherat <re...@apache.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|REOPENED |RESOLVED
Resolution|--- |FIXED
--- Comment #24 from Remy Maucherat <re...@apache.org> ---
This should make the enhancement "done" as it adds the most important features.
This is included in 9.0.2, 8.5.24, 8.0.48, 7.0.83.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 57767] Websocket client proprietary configuration
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767
--- Comment #15 from Christopher Schultz <ch...@christopherschultz.net> ---
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: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 57767] Websocket client proprietary configuration
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767
--- Comment #9 from J Fernandez <jf...@gmail.com> ---
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: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 57767] Websocket client proprietary configuration
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767
--- Comment #19 from J Fernandez <jf...@gmail.com> ---
Are there any additional proposed changes for this patch? I would like to
leverage some of the functionality for
https://bz.apache.org/bugzilla/show_bug.cgi?id=59758.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 57767] Websocket client proprietary configuration
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767
J Fernandez <jf...@gmail.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |jfern096@gmail.com
--- Comment #7 from J Fernandez <jf...@gmail.com> ---
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: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 57767] Websocket client proprietary configuration
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767
--- Comment #14 from J Fernandez <jf...@gmail.com> ---
(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: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 57767] Websocket client proprietary configuration
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767
--- Comment #25 from J Fernandez <jf...@gmail.com> ---
I believe that there are additional benefits for separating the websocket
client from the container. For example, we could enhance the redirect flow when
behind a proxy by caching the SocketChannel for a connected host:port. We can
always, pass the structure as a method argument, but I am not sure that can be
manageable long term. In my opinion, as it stands, the container limits
flexibility when adding similar features.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 57767] Websocket client proprietary configuration
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767
--- Comment #12 from J Fernandez <jf...@gmail.com> ---
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: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 57767] Websocket client proprietary configuration
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767
Remy Maucherat <re...@apache.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |kmclaugh@gmail.com
--- Comment #2 from Remy Maucherat <re...@apache.org> ---
*** 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: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 57767] Websocket client proprietary configuration
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767
Remy Maucherat <re...@apache.org> 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: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 57767] Websocket client proprietary configuration
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767
--- Comment #6 from Mark Thomas <ma...@apache.org> ---
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: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 57767] Websocket client proprietary configuration
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767
--- Comment #23 from Remy Maucherat <re...@apache.org> ---
I prefer getting rid of the field instead, the GC savings are minimal and not
worth it IMO.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 57767] Websocket client proprietary configuration
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767
Remy Maucherat <re...@apache.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Resolution|FIXED |---
Status|RESOLVED |REOPENED
--- Comment #11 from Remy Maucherat <re...@apache.org> ---
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: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 57767] Websocket client proprietary configuration
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767
--- Comment #21 from Mark Thomas <ma...@apache.org> ---
Wasn't the point of the new class that the redirectSet wasn't thread safe?
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 57767] Websocket client proprietary configuration
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767
Mark Thomas <ma...@apache.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC|markt@apache.org |
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 57767] Websocket client proprietary configuration
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767
--- Comment #13 from Remy Maucherat <re...@apache.org> ---
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: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 57767] Websocket client proprietary configuration
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767
Mark Thomas <ma...@apache.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Resolution|--- |FIXED
Status|NEW |RESOLVED
--- Comment #8 from Mark Thomas <ma...@apache.org> ---
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: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 57767] Websocket client proprietary configuration
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767
--- Comment #18 from J Fernandez <jf...@gmail.com> ---
I have spent some time looking for opportunities to reuse but did not find
many. We could replace the WWWAuthenticate parser for digest with
org.apache.tomcat.util.http.parser, but we will still need the one I added for
basic. At that point I am not sure if it's worth it if we can't replace it for
both schemes.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 57767] Websocket client proprietary configuration
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767
Christopher Schultz <ch...@christopherschultz.net> changed:
What |Removed |Added
----------------------------------------------------------------------------
Keywords| |PatchAvailable
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org