[Bug 57767] Websocket client proprietary configuration
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767 --- Comment #25 from J Fernandez--- 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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 57767] Websocket client proprietary configuration
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767 Remy Maucheratchanged: What|Removed |Added Status|REOPENED|RESOLVED Resolution|--- |FIXED --- Comment #24 from Remy Maucherat --- 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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 57767] Websocket client proprietary configuration
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767 --- Comment #23 from Remy Maucherat--- 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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 57767] Websocket client proprietary configuration
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767 --- Comment #22 from Remy Maucherat--- 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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 57767] Websocket client proprietary configuration
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767 --- Comment #21 from Mark Thomas--- 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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 57767] Websocket client proprietary configuration
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767 --- Comment #20 from Remy Maucherat--- 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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #73: Bug 57767 - Websocket client proprietary configuration
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/73 No objections to back-porting here. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #73: Bug 57767 - Websocket client proprietary configuration
Github user rmaucher commented on the issue: https://github.com/apache/tomcat/pull/73 Well, it looks ok to me overall, so I'll add a bit of javadoc and merge it. Any issue with backporting it ? --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #73: Bug 57767 - Websocket client proprietary configuration
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/73 Chris's original concern with the BZ 57767 patch (lack of Javadoc) still needs to be addressed. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 57767] Websocket client proprietary configuration
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767 --- Comment #19 from J Fernandez--- 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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 57767] Websocket client proprietary configuration
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767 --- Comment #18 from J Fernandez--- 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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 57767] Websocket client proprietary configuration
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767 --- Comment #17 from Remy Maucherat--- 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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 57767] Websocket client proprietary configuration
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767 --- Comment #16 from Mark Thomas--- 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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 57767] Websocket client proprietary configuration
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767 --- Comment #15 from Christopher Schultz--- 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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 57767] Websocket client proprietary configuration
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767 --- Comment #14 from J Fernandez--- (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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 57767] Websocket client proprietary configuration
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767 Christopher Schultzchanged: What|Removed |Added Keywords||PatchAvailable -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 57767] Websocket client proprietary configuration
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767 --- Comment #13 from Remy Maucherat--- 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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 57767] Websocket client proprietary configuration
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767 --- Comment #12 from J Fernandez--- Created attachment 35289 --> https://bz.apache.org/bugzilla/attachment.cgi?id=35289=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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 57767] Websocket client proprietary configuration
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767 Mark Thomaschanged: What|Removed |Added CC|ma...@apache.org| -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 57767] Websocket client proprietary configuration
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767 Remy Maucheratchanged: What|Removed |Added Resolution|FIXED |--- Status|RESOLVED|REOPENED --- Comment #11 from Remy Maucherat --- 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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 57767] Websocket client proprietary configuration
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767 --- Comment #10 from Mark Thomas--- 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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 57767] Websocket client proprietary configuration
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767 --- Comment #9 from J Fernandez--- 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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 57767] Websocket client proprietary configuration
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767 Mark Thomaschanged: What|Removed |Added Resolution|--- |FIXED Status|NEW |RESOLVED --- Comment #8 from Mark Thomas --- 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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 57767] Websocket client proprietary configuration
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767 J Fernandezchanged: What|Removed |Added CC||jfern...@gmail.com --- Comment #7 from J Fernandez --- Created attachment 35193 --> https://bz.apache.org/bugzilla/attachment.cgi?id=35193=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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 57767] Websocket client proprietary configuration
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767 --- Comment #6 from Mark Thomas--- 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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 57767] Websocket client proprietary configuration
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767 --- Comment #5 from MikeLing--- (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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 57767] Websocket client proprietary configuration
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767 --- Comment #4 from Mark Thomas--- 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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 57767] Websocket client proprietary configuration
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767 --- Comment #3 from MikeLing--- 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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 57767] Websocket client proprietary configuration
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767 Remy Maucheratchanged: What|Removed |Added CC||kmcla...@gmail.com --- Comment #2 from Remy Maucherat --- *** 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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 57767] Websocket client proprietary configuration
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767 Mark Thomaschanged: What|Removed |Added CC||ma...@apache.org --- Comment #1 from Mark Thomas --- *** 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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 57767] Websocket client proprietary configuration
https://bz.apache.org/bugzilla/show_bug.cgi?id=57767 Remy Maucherat r...@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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org