[GitHub] [tomcat] apptie opened a new pull request, #663: Refactoring the main method of a Tomcat class to improve readability

2023-09-06 Thread via GitHub
apptie opened a new pull request, #663: URL: https://github.com/apache/tomcat/pull/663 Hello! While learning about Tomcat, I see something that I think I can contribute to, so I write this PR. ## Explanation There are two commits here: - Remove the FQCN when

[GitHub] [tomcat] markt-asf commented on a diff in pull request #662: Simplify and enhance charset extraction from content type

2023-09-06 Thread via GitHub
markt-asf commented on code in PR #662: URL: https://github.com/apache/tomcat/pull/662#discussion_r1316892872 ## java/org/apache/coyote/Request.java: ## @@ -16,19 +16,8 @@ */ package org.apache.coyote; -import java.io.IOException; -import

[GitHub] [tomcat] parkmuhyeun commented on pull request #659: Unify constant delimiters and Refactoring for better readability

2023-09-06 Thread via GitHub
parkmuhyeun commented on PR #659: URL: https://github.com/apache/tomcat/pull/659#issuecomment-1707887311 TransferENCODING wasn't mentioned, so I didn't recognize it. I reverted all the stuff about constant names! -- This is an automated message from the Apache Git Service. To respond to

[GitHub] [tomcat] greeng00se commented on a diff in pull request #660: Optimize when call getSessionCookieName & getSessionUriParamName

2023-09-06 Thread via GitHub
greeng00se commented on code in PR #660: URL: https://github.com/apache/tomcat/pull/660#discussion_r1316896601 ## java/org/apache/catalina/util/SessionConfig.java: ## @@ -49,38 +42,30 @@ public static String getSessionCookieName(Context context) { * @return the parameter

[GitHub] [tomcat] greeng00se commented on a diff in pull request #660: Optimize when call getSessionCookieName & getSessionUriParamName

2023-09-06 Thread via GitHub
greeng00se commented on code in PR #660: URL: https://github.com/apache/tomcat/pull/660#discussion_r1316896601 ## java/org/apache/catalina/util/SessionConfig.java: ## @@ -49,38 +42,30 @@ public static String getSessionCookieName(Context context) { * @return the parameter

[GitHub] [tomcat] markt-asf commented on a diff in pull request #660: Optimize when call getSessionCookieName & getSessionUriParamName

2023-09-06 Thread via GitHub
markt-asf commented on code in PR #660: URL: https://github.com/apache/tomcat/pull/660#discussion_r1316890952 ## java/org/apache/catalina/util/SessionConfig.java: ## @@ -49,38 +42,30 @@ public static String getSessionCookieName(Context context) { * @return the parameter

[GitHub] [tomcat] markt-asf commented on a diff in pull request #659: Unify constant delimiters and Refactoring for better readability

2023-09-06 Thread via GitHub
markt-asf commented on code in PR #659: URL: https://github.com/apache/tomcat/pull/659#discussion_r1316887097 ## java/org/apache/coyote/http11/Constants.java: ## @@ -106,7 +106,7 @@ public final class Constants { public static final String KEEP_ALIVE_HEADER_VALUE_TOKEN =

[GitHub] [tomcat] markt-asf commented on a diff in pull request #661: Unify conditional statement format for some constants in http1 package

2023-09-06 Thread via GitHub
markt-asf commented on code in PR #661: URL: https://github.com/apache/tomcat/pull/661#discussion_r1316875467 ## java/org/apache/coyote/http11/filters/ChunkedInputFilter.java: ## @@ -574,7 +574,7 @@ private boolean parseHeader() throws IOException { }

[GitHub] [tomcat] markt-asf merged pull request #657: Eliminating duplicate execution of checkFacade logic in ResponseFacade

2023-09-06 Thread via GitHub
markt-asf merged PR #657: URL: https://github.com/apache/tomcat/pull/657 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail:

[GitHub] [tomcat] wonyongChoi05 opened a new pull request, #662: Simplify and enhance charset extraction from content type

2023-09-05 Thread via GitHub
wonyongChoi05 opened a new pull request, #662: URL: https://github.com/apache/tomcat/pull/662 Hello! Thank you for the PR. Upon reviewing these changes, I've noticed a significant improvement in code readability. This enhanced readability stands out and will make a substantial difference

[GitHub] [tomcat] Jaeyoung22 commented on pull request #658: Unify conditional statement format for some constants in http1 package

2023-09-05 Thread via GitHub
Jaeyoung22 commented on PR #658: URL: https://github.com/apache/tomcat/pull/658#issuecomment-1707535138 This Pull Request UNINTENTIONALLY sended because of github incident :( So I closed this PR and resend. -- This is an automated message from the Apache Git Service. To respond to the

[GitHub] [tomcat] iamjooon2 commented on pull request #652: Remove unnecessary brackets

2023-09-05 Thread via GitHub
iamjooon2 commented on PR #652: URL: https://github.com/apache/tomcat/pull/652#issuecomment-1707507180 @markt-asf I just saw [!This PR](https://github.com/apache/tomcat/pull/650) and removed some brackets too Regret for comment 'Automated tools' -- This is an automated message

[GitHub] [tomcat] iamjooon2 closed pull request #652: Remove unnecessary brackets

2023-09-05 Thread via GitHub
iamjooon2 closed pull request #652: Remove unnecessary brackets URL: https://github.com/apache/tomcat/pull/652 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe,

[GitHub] [tomcat] parkmuhyeun commented on pull request #659: Unify constant delimiters and Refactoring for better readability

2023-09-05 Thread via GitHub
parkmuhyeun commented on PR #659: URL: https://github.com/apache/tomcat/pull/659#issuecomment-1707461772 Oh, sorry. I missed that one! I've fixed it. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go

[GitHub] [tomcat] Jaeyoung22 opened a new pull request, #661: Unify conditional statement format for some constants in http1 package

2023-09-05 Thread via GitHub
Jaeyoung22 opened a new pull request, #661: URL: https://github.com/apache/tomcat/pull/661 Although I totally agree that there could be some unnecessary parentheses to aid the readability of the code, I think that the unity of the code is still important for readability. ###

[GitHub] [tomcat] Jaeyoung22 closed pull request #658: Unify conditional statement format for some constants in http1 package

2023-09-05 Thread via GitHub
Jaeyoung22 closed pull request #658: Unify conditional statement format for some constants in http1 package URL: https://github.com/apache/tomcat/pull/658 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to

[GitHub] [tomcat] Jaeyoung22 closed pull request #658: Unify conditional statement format for some constants in http1 package

2023-09-05 Thread via GitHub
Jaeyoung22 closed pull request #658: Unify conditional statement format for some constants in http1 package URL: https://github.com/apache/tomcat/pull/658 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to

[GitHub] [tomcat] xxeol2 commented on a diff in pull request #657: Eliminating duplicate execution of checkFacade logic in ResponseFacade

2023-09-05 Thread via GitHub
xxeol2 commented on code in PR #657: URL: https://github.com/apache/tomcat/pull/657#discussion_r1316480032 ## java/org/apache/catalina/connector/ResponseFacade.java: ## @@ -111,23 +111,19 @@ public String getCharacterEncoding() { @Override public ServletOutputStream

[GitHub] [tomcat] greeng00se commented on pull request #660: Optimize when call getSessionCookieName & getSessionUriParamName

2023-09-05 Thread via GitHub
greeng00se commented on PR #660: URL: https://github.com/apache/tomcat/pull/660#issuecomment-1707265409 @markt-asf Thanks for the nice comment. I hadn't considered that partial compiler optimisation. I agree, readable code should be a priority and I moved the null-check logic to a

[GitHub] [tomcat] markt-asf commented on pull request #660: Optimize when call getSessionCookieName & getSessionUriParamName

2023-09-05 Thread via GitHub
markt-asf commented on PR #660: URL: https://github.com/apache/tomcat/pull/660#issuecomment-1707256734 Saving a single function call is unlikely to make any difference. I'd expect the compiler to optimise that. Changes that reduce duplication and/or improve the readability of the

[GitHub] [tomcat] greeng00se commented on pull request #656: Optimization when call getSessionCookieName & getSessionUriParamName

2023-09-05 Thread via GitHub
greeng00se commented on PR #656: URL: https://github.com/apache/tomcat/pull/656#issuecomment-1707231871 sorry. Case 3 is not considered. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the

[GitHub] [tomcat] greeng00se commented on pull request #650: Removed unnecessary brackets

2023-09-05 Thread via GitHub
greeng00se commented on PR #650: URL: https://github.com/apache/tomcat/pull/650#issuecomment-1707230523 @markt-asf thx for merge my first contribution  -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above

[GitHub] [tomcat] greeng00se opened a new pull request, #660: Optimize when call getSessionCookieName & getSessionUriParamName

2023-09-05 Thread via GitHub
greeng00se opened a new pull request, #660: URL: https://github.com/apache/tomcat/pull/660 ### Description Optimization when call getSessionCookieName & getSessionUriParamName ### Explanation Existing code calls getConfiguredSessionCookieName even if the context is

[GitHub] [tomcat] parkmuhyeun commented on pull request #659: Unify constant delimiters and Refactoring for better readability

2023-09-05 Thread via GitHub
parkmuhyeun commented on PR #659: URL: https://github.com/apache/tomcat/pull/659#issuecomment-1707218270 I reverted the stage constants back and modified the switch statements to use the constants that were originally there! -- This is an automated message from the Apache Git Service. To

[GitHub] [tomcat] markt-asf commented on a diff in pull request #659: Unify constant delimiters and Refactoring for better readability

2023-09-05 Thread via GitHub
markt-asf commented on code in PR #659: URL: https://github.com/apache/tomcat/pull/659#discussion_r1316303250 ## java/org/apache/catalina/manager/StatusTransformer.java: ## @@ -375,33 +375,33 @@ protected static void writeProcessorState(PrintWriter writer, switch

[GitHub] [tomcat] markt-asf commented on a diff in pull request #657: Eliminating duplicate execution of checkFacade logic in ResponseFacade

2023-09-05 Thread via GitHub
markt-asf commented on code in PR #657: URL: https://github.com/apache/tomcat/pull/657#discussion_r1316282831 ## java/org/apache/catalina/connector/ResponseFacade.java: ## @@ -111,23 +111,19 @@ public String getCharacterEncoding() { @Override public

[GitHub] [tomcat] parkmuhyeun opened a new pull request, #659: Unify constant delimiters And Refactoring

2023-09-05 Thread via GitHub
parkmuhyeun opened a new pull request, #659: URL: https://github.com/apache/tomcat/pull/659 ## Description This PR made the following two modifications - Unify constant delimiters - Refactoring for better readability ## Explanation ### Unify constant delimiters

[GitHub] [tomcat] Jaeyoung22 opened a new pull request, #658: Unify conditional statement format for some constants in http1 package

2023-09-05 Thread via GitHub
Jaeyoung22 opened a new pull request, #658: URL: https://github.com/apache/tomcat/pull/658 Although I totally agree that there could be some unnecessary parentheses to aid the readability of the code, I think that the unity of the code is still important for readability. ###

[GitHub] [tomcat] greeng00se commented on pull request #656: Optimization when call getSessionCookieName & getSessionUriParamName

2023-09-05 Thread via GitHub
greeng00se commented on PR #656: URL: https://github.com/apache/tomcat/pull/656#issuecomment-1706941221 I didn't think about when neither priority 1 nor 2 is present in pr, sorry. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to

[GitHub] [tomcat] xxeol2 opened a new pull request, #657: Eliminating duplicate execution of checkFacade logic in ResponseFacade

2023-09-05 Thread via GitHub
xxeol2 opened a new pull request, #657: URL: https://github.com/apache/tomcat/pull/657 https://github.com/apache/tomcat/pull/654#issuecomment-1706706294 This was discussed in the comments above! -- This is an automated message from the Apache Git Service. To respond to the message,

[GitHub] [tomcat] markt-asf commented on pull request #654: Eliminating duplicate execution of checkFacade logic in ResponseFacade

2023-09-05 Thread via GitHub
markt-asf commented on PR #654: URL: https://github.com/apache/tomcat/pull/654#issuecomment-1706867922 Those refactorings look OK. Please submit another PR for those. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use

[GitHub] [tomcat] greeng00se closed pull request #656: Optimization when call getSessionCookieName & getSessionUriParamName

2023-09-05 Thread via GitHub
greeng00se closed pull request #656: Optimization when call getSessionCookieName & getSessionUriParamName URL: https://github.com/apache/tomcat/pull/656 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go

[GitHub] [tomcat] greeng00se opened a new pull request, #656: Optimization when call getSessionCookieName & getSessionUriParamName

2023-09-05 Thread via GitHub
greeng00se opened a new pull request, #656: URL: https://github.com/apache/tomcat/pull/656 Existing code calls getConfiguredSessionCookieName even if the context is empty. In getConfiguredSessionCookieName, only act to get the session cookie if the context is non-null. I

[GitHub] [tomcat] iamjooon2 closed pull request #652: Remove unnecessary brackets

2023-09-05 Thread via GitHub
iamjooon2 closed pull request #652: Remove unnecessary brackets URL: https://github.com/apache/tomcat/pull/652 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe,

[GitHub] [tomcat] woosung1223 commented on pull request #653: unnecessary work is done before determining if it is a special header

2023-09-05 Thread via GitHub
woosung1223 commented on PR #653: URL: https://github.com/apache/tomcat/pull/653#issuecomment-1706779668 > Those checks were added for performance reasons. They were significantly quicker than calling checkSpecialHeader(). Unless a benchmark (as a unit test) is provided that demonstrates

[GitHub] [tomcat] woosung1223 closed pull request #653: unnecessary work is done before determining if it is a special header

2023-09-05 Thread via GitHub
woosung1223 closed pull request #653: unnecessary work is done before determining if it is a special header URL: https://github.com/apache/tomcat/pull/653 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to

[GitHub] [tomcat] xxeol2 commented on pull request #654: Eliminating duplicate execution of checkFacade logic in ResponseFacade

2023-09-05 Thread via GitHub
xxeol2 commented on PR #654: URL: https://github.com/apache/tomcat/pull/654#issuecomment-1706772038 @markt-asf I was wondering if I shouldn't avoid duplicate calls to `checkFacade()` in the `getOutputStream()` and `getWriter()` methods!

[GitHub] [tomcat] markt-asf commented on pull request #654: Eliminating duplicate execution of checkFacade logic in ResponseFacade

2023-09-05 Thread via GitHub
markt-asf commented on PR #654: URL: https://github.com/apache/tomcat/pull/654#issuecomment-1706766432 Nice catch. Tx for the PR. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific

[GitHub] [tomcat] markt-asf merged pull request #654: Eliminating duplicate execution of checkFacade logic in ResponseFacade

2023-09-05 Thread via GitHub
markt-asf merged PR #654: URL: https://github.com/apache/tomcat/pull/654 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail:

[GitHub] [tomcat] markt-asf commented on pull request #653: unnecessary work is done before determining if it is a special header

2023-09-05 Thread via GitHub
markt-asf commented on PR #653: URL: https://github.com/apache/tomcat/pull/653#issuecomment-1706758159 Those checks were added for performance reasons. They were significantly quicker than calling `checkSpecialHeader()`. Unless a benchmark (as a unit test) is provided that demonstrates

[GitHub] [tomcat] markt-asf merged pull request #655: Fix a comment typo

2023-09-05 Thread via GitHub
markt-asf merged PR #655: URL: https://github.com/apache/tomcat/pull/655 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail:

[GitHub] [tomcat] markt-asf commented on pull request #652: Remove unnecessary brackets

2023-09-05 Thread via GitHub
markt-asf commented on PR #652: URL: https://github.com/apache/tomcat/pull/652#issuecomment-1706738930 We have started to get a lot of PRs removing single instances of unnecessary parentheses. While we are generally in favour of this sort of clean-up, one PR per instance is not scaleable

[GitHub] [tomcat] xxeol2 commented on pull request #654: Eliminating duplicate execution of checkFacade logic in ResponseFacade

2023-09-05 Thread via GitHub
xxeol2 commented on PR #654: URL: https://github.com/apache/tomcat/pull/654#issuecomment-1706706294 I'm curious if following code changes would introduce any side effects. ### Origin ```java public ServletOutputStream getOutputStream() throws IOException { checkFacade();

[GitHub] [tomcat] ingpyo opened a new pull request, #655: Fix a comment typo

2023-09-05 Thread via GitHub
ingpyo opened a new pull request, #655: URL: https://github.com/apache/tomcat/pull/655 Hello, I have fixed a typo in the Tomcat codebase by changing "CookiePreocessor" to "CookieProcessor" for better code clarity and correctness. -- This is an automated message from the Apache Git

[GitHub] [tomcat] xxeol2 opened a new pull request, #654: Eliminating duplicate execution of checkFacade logic in ResponseFacade

2023-09-05 Thread via GitHub
xxeol2 opened a new pull request, #654: URL: https://github.com/apache/tomcat/pull/654 This PR is closely related to https://github.com/apache/tomcat/pull/649. Prevent double execution of the checkFacade() logic when invoking the isCommitted() method. thanks !!  -- This is an

[GitHub] [tomcat] woosung1223 opened a new pull request, #653: remove unnecessary character checking when checking special headers

2023-09-05 Thread via GitHub
woosung1223 opened a new pull request, #653: URL: https://github.com/apache/tomcat/pull/653 Hi there! I found unnecessary code in the Response class. Currently, it seems that only headers of Content-Length or Content-Type are called Special Headers. However, as you can

[GitHub] [tomcat] iamjooon2 opened a new pull request, #652: Deleted needless brackets

2023-09-05 Thread via GitHub
iamjooon2 opened a new pull request, #652: URL: https://github.com/apache/tomcat/pull/652 Hello! i deleted needless brackets for consistency with the other code. Thx :D -- This is an automated message from the Apache Git Service. To respond to the message, please log on to

[GitHub] [tomcat] wonyongChoi05 commented on pull request #651: Refactor SSL certificate population method

2023-09-05 Thread via GitHub
wonyongChoi05 commented on PR #651: URL: https://github.com/apache/tomcat/pull/651#issuecomment-1706354873 > The switch to using a wildcard import would need to be reverted as would the changes to blank lines between methods. The null check is necessary in some form and it is more concise

[GitHub] [tomcat] markt-asf commented on pull request #651: Refactor SSL certificate population method

2023-09-05 Thread via GitHub
markt-asf commented on PR #651: URL: https://github.com/apache/tomcat/pull/651#issuecomment-1706340615 The switch to using a wildcard import would need to be reverted as would the changes to blank lines between methods. The null check is necessary in some form and it is more concise than

[GitHub] [tomcat] markt-asf merged pull request #649: Eliminating duplicate execution of checkFacade logic

2023-09-05 Thread via GitHub
markt-asf merged PR #649: URL: https://github.com/apache/tomcat/pull/649 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail:

[GitHub] [tomcat] markt-asf commented on pull request #649: Eliminating duplicate execution of checkFacade logic

2023-09-05 Thread via GitHub
markt-asf commented on PR #649: URL: https://github.com/apache/tomcat/pull/649#issuecomment-1706282024 The change makes sense to me. It removes a duplicate call to `checkFacade()`. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to

[GitHub] [tomcat] markt-asf merged pull request #650: Removed unnecessary brackets

2023-09-05 Thread via GitHub
markt-asf merged PR #650: URL: https://github.com/apache/tomcat/pull/650 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail:

[GitHub] [tomcat] markt-asf commented on pull request #650: Removed unnecessary brackets

2023-09-05 Thread via GitHub
markt-asf commented on PR #650: URL: https://github.com/apache/tomcat/pull/650#issuecomment-1706269180 The change is trivial but it makes sense to me. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to

[GitHub] [tomcat] aooohan closed pull request #651: Refactor SSL certificate population method

2023-09-05 Thread via GitHub
aooohan closed pull request #651: Refactor SSL certificate population method URL: https://github.com/apache/tomcat/pull/651 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To

[GitHub] [tomcat] aooohan commented on pull request #651: Refactor SSL certificate population method

2023-09-05 Thread via GitHub
aooohan commented on PR #651: URL: https://github.com/apache/tomcat/pull/651#issuecomment-1706113147 This change doesn't make any sense. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the

[GitHub] [tomcat] aooohan commented on pull request #650: Removed unnecessary brackets

2023-09-05 Thread via GitHub
aooohan commented on PR #650: URL: https://github.com/apache/tomcat/pull/650#issuecomment-1706094596 This change doesn't make any sense. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the

[GitHub] [tomcat] aooohan closed pull request #649: Eliminating duplicate execution of checkFacade logic

2023-09-05 Thread via GitHub
aooohan closed pull request #649: Eliminating duplicate execution of checkFacade logic URL: https://github.com/apache/tomcat/pull/649 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific

[GitHub] [tomcat] aooohan commented on pull request #649: Eliminating duplicate execution of checkFacade logic

2023-09-05 Thread via GitHub
aooohan commented on PR #649: URL: https://github.com/apache/tomcat/pull/649#issuecomment-1706095437 This change doesn't make any sense. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the

[GitHub] [tomcat] aooohan closed pull request #650: Removed unnecessary brackets

2023-09-05 Thread via GitHub
aooohan closed pull request #650: Removed unnecessary brackets URL: https://github.com/apache/tomcat/pull/650 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe,

[GitHub] [tomcat] wonyongChoi05 opened a new pull request, #651: Refactor SSL certificate population method

2023-09-05 Thread via GitHub
wonyongChoi05 opened a new pull request, #651: URL: https://github.com/apache/tomcat/pull/651 Simplified the SSL certificate population logic by removing the unnecessary null check and using an ArrayList for dynamic array management. The code is now more concise and readable, and it

[GitHub] [tomcat] greeng00se opened a new pull request, #650: Removed unnecessary brackets

2023-09-05 Thread via GitHub
greeng00se opened a new pull request, #650: URL: https://github.com/apache/tomcat/pull/650 hello  I was looking at the code for the Request class in coyote. Removed unnecessary brackets for consistency with the other code. e.g. `contentLength = (clB == null ||

[GitHub] [tomcat] aooohan commented on pull request #648: Refactor 'getSession' Method in 'Request' Class to Eliminate Code Redundancy

2023-09-05 Thread via GitHub
aooohan commented on PR #648: URL: https://github.com/apache/tomcat/pull/648#issuecomment-1706002731 Thanks for the PR. ; ) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment.

[GitHub] [tomcat] aooohan merged pull request #648: Refactor 'getSession' Method in 'Request' Class to Eliminate Code Redundancy

2023-09-05 Thread via GitHub
aooohan merged PR #648: URL: https://github.com/apache/tomcat/pull/648 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail:

[GitHub] [tomcat] xxeol2 opened a new pull request, #649: Eliminating duplicate execution of checkFacade logic

2023-09-04 Thread via GitHub
xxeol2 opened a new pull request, #649: URL: https://github.com/apache/tomcat/pull/649 In the current implementation of the getSession method in the RequestFacade class, the checkFacade logic was redundantly executed twice. I improved this by eliminating the redundancy. -- This is an

[GitHub] [tomcat] xxeol2 commented on a diff in pull request #648: Refactor 'getSession' Method in 'Request' Class to Eliminate Code Redundancy

2023-09-04 Thread via GitHub
xxeol2 commented on code in PR #648: URL: https://github.com/apache/tomcat/pull/648#discussion_r1315352368 ## java/org/apache/catalina/connector/Request.java: ## @@ -2288,12 +2288,7 @@ public String getServletPath() { */ @Override public HttpSession getSession()

[GitHub] [tomcat] aooohan commented on a diff in pull request #648: Refactor 'getSession' Method in 'Request' Class to Eliminate Code Redundancy

2023-09-04 Thread via GitHub
aooohan commented on code in PR #648: URL: https://github.com/apache/tomcat/pull/648#discussion_r1315332131 ## java/org/apache/catalina/connector/Request.java: ## @@ -2288,12 +2288,7 @@ public String getServletPath() { */ @Override public HttpSession

[GitHub] [tomcat] xxeol2 opened a new pull request, #648: Refactor 'getSession' Method in 'Request' Class to Eliminate Code Redundancy

2023-09-04 Thread via GitHub
xxeol2 opened a new pull request, #648: URL: https://github.com/apache/tomcat/pull/648 This Pull Request aims to reduce code duplication in the `getSession` method of the `Request` class. The refactoring has been inspired by the format used in the `reset` method of

[GitHub] [tomcat-connectors] markt-asf closed pull request #5: Fix bug #65901 to correct HTTP 401 response for HEAD request

2023-09-04 Thread via GitHub
markt-asf closed pull request #5: Fix bug #65901 to correct HTTP 401 response for HEAD request URL: https://github.com/apache/tomcat-connectors/pull/5 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go

[GitHub] [tomcat-connectors] markt-asf commented on pull request #5: Fix bug #65901 to correct HTTP 401 response for HEAD request

2023-09-04 Thread via GitHub
markt-asf commented on PR #5: URL: https://github.com/apache/tomcat-connectors/pull/5#issuecomment-1705720654 The proposed patch does not address the root cause of the bug which can be found at https://github.com/apache/tomcat-connectors/blob/main/native/apache-2.0/mod_jk.c#L3016 I

[GitHub] [tomcat-connectors] markt-asf merged pull request #6: Fix Clang 15/16 compatibility

2023-09-04 Thread via GitHub
markt-asf merged PR #6: URL: https://github.com/apache/tomcat-connectors/pull/6 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail:

[GitHub] [tomcat] markt-asf closed pull request #647: Replaced synchronized with StampedLock

2023-08-22 Thread via GitHub
markt-asf closed pull request #647: Replaced synchronized with StampedLock URL: https://github.com/apache/tomcat/pull/647 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To

[GitHub] [tomcat] markt-asf commented on pull request #647: Replaced synchronized with StampedLock

2023-08-22 Thread via GitHub
markt-asf commented on PR #647: URL: https://github.com/apache/tomcat/pull/647#issuecomment-1689066140 Synchronized blocks only need to be replaced if they contain blocking operations. This one clearly doesn't. -- This is an automated message from the Apache Git Service. To respond to

[GitHub] [tomcat] sergmain commented on pull request #647: Replaced synchronized with StampedLock

2023-08-22 Thread via GitHub
sergmain commented on PR #647: URL: https://github.com/apache/tomcat/pull/647#issuecomment-1689065406 From my point of view there are 2 approaches - analyze every synchronized or remove every synchronized without analyzing. My pull-request is with second approach. -- This is an

[GitHub] [tomcat] markt-asf merged pull request #646: Parameter error handling

2023-08-22 Thread via GitHub
markt-asf merged PR #646: URL: https://github.com/apache/tomcat/pull/646 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail:

[GitHub] [tomcat] markt-asf commented on pull request #647: Replaced synchronized with StampedLock

2023-08-22 Thread via GitHub
markt-asf commented on PR #647: URL: https://github.com/apache/tomcat/pull/647#issuecomment-1688571304 What is the justification for this change? Synchronization is not automatically an issue for virtual threads. I'm not seeing anything in this code that would be unfriendly to virtual

[GitHub] [tomcat] michael-o commented on a diff in pull request #646: Parameter error handling

2023-08-22 Thread via GitHub
michael-o commented on code in PR #646: URL: https://github.com/apache/tomcat/pull/646#discussion_r1301062237 ## java/org/apache/tomcat/util/http/InvalidParameterException.java: ## @@ -0,0 +1,98 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + *

[GitHub] [tomcat-jakartaee-migration] tutufool commented on issue #47: EE package migration

2023-08-21 Thread via GitHub
tutufool commented on issue #47: URL: https://github.com/apache/tomcat-jakartaee-migration/issues/47#issuecomment-1687326690 cool, thanks a lot! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above

[GitHub] [tomcat] sergmain opened a new pull request, #647: Replaced synchronized with StampedLock

2023-08-21 Thread via GitHub
sergmain opened a new pull request, #647: URL: https://github.com/apache/tomcat/pull/647 Replaced synchronized with StampedLock as preparation for virtual threads -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the

[GitHub] [tomcat] markt-asf commented on a diff in pull request #646: Parameter error handling

2023-08-21 Thread via GitHub
markt-asf commented on code in PR #646: URL: https://github.com/apache/tomcat/pull/646#discussion_r1300661077 ## java/org/apache/tomcat/util/http/InvalidParameterException.java: ## @@ -0,0 +1,98 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + *

[GitHub] [tomcat] michael-o commented on a diff in pull request #646: Parameter error handling

2023-08-21 Thread via GitHub
michael-o commented on code in PR #646: URL: https://github.com/apache/tomcat/pull/646#discussion_r1300658108 ## java/org/apache/tomcat/util/http/InvalidParameterException.java: ## @@ -0,0 +1,98 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + *

[GitHub] [tomcat] markt-asf commented on a diff in pull request #646: Parameter error handling

2023-08-21 Thread via GitHub
markt-asf commented on code in PR #646: URL: https://github.com/apache/tomcat/pull/646#discussion_r1300656508 ## java/org/apache/tomcat/util/http/InvalidParameterException.java: ## @@ -0,0 +1,98 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + *

[GitHub] [tomcat] michael-o commented on a diff in pull request #646: Parameter error handling

2023-08-21 Thread via GitHub
michael-o commented on code in PR #646: URL: https://github.com/apache/tomcat/pull/646#discussion_r1300652715 ## java/org/apache/tomcat/util/http/InvalidParameterException.java: ## @@ -0,0 +1,98 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + *

[GitHub] [tomcat] michael-o commented on a diff in pull request #646: Parameter error handling

2023-08-21 Thread via GitHub
michael-o commented on code in PR #646: URL: https://github.com/apache/tomcat/pull/646#discussion_r1300652715 ## java/org/apache/tomcat/util/http/InvalidParameterException.java: ## @@ -0,0 +1,98 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + *

[GitHub] [tomcat] michael-o commented on a diff in pull request #646: Parameter error handling

2023-08-21 Thread via GitHub
michael-o commented on code in PR #646: URL: https://github.com/apache/tomcat/pull/646#discussion_r1300043089 ## java/org/apache/tomcat/util/http/InvalidParameterException.java: ## @@ -0,0 +1,65 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + *

[GitHub] [tomcat] markt-asf commented on a diff in pull request #646: Parameter error handling

2023-08-21 Thread via GitHub
markt-asf commented on code in PR #646: URL: https://github.com/apache/tomcat/pull/646#discussion_r1300034970 ## java/org/apache/tomcat/util/http/InvalidParameterException.java: ## @@ -0,0 +1,65 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + *

[GitHub] [tomcat] michael-o commented on a diff in pull request #646: Parameter error handling

2023-08-21 Thread via GitHub
michael-o commented on code in PR #646: URL: https://github.com/apache/tomcat/pull/646#discussion_r1299969607 ## webapps/docs/config/http.xml: ## @@ -148,20 +148,17 @@ files) obtained from the query string and, for POST requests, the request body if the content

[GitHub] [tomcat] michael-o commented on a diff in pull request #646: Parameter error handling

2023-08-21 Thread via GitHub
michael-o commented on code in PR #646: URL: https://github.com/apache/tomcat/pull/646#discussion_r1299969203 ## java/org/apache/tomcat/util/http/InvalidParameterException.java: ## @@ -0,0 +1,65 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + *

[GitHub] [tomcat] markt-asf commented on a diff in pull request #646: Parameter error handling

2023-08-21 Thread via GitHub
markt-asf commented on code in PR #646: URL: https://github.com/apache/tomcat/pull/646#discussion_r1299936723 ## webapps/docs/config/http.xml: ## @@ -148,20 +148,17 @@ files) obtained from the query string and, for POST requests, the request body if the content

[GitHub] [tomcat] markt-asf commented on a diff in pull request #646: Parameter error handling

2023-08-21 Thread via GitHub
markt-asf commented on code in PR #646: URL: https://github.com/apache/tomcat/pull/646#discussion_r1299935328 ## webapps/docs/changelog.xml: ## @@ -113,6 +113,12 @@ provided error code during error page processing rather than assuming an error code of 500.

[GitHub] [tomcat] markt-asf commented on a diff in pull request #646: Parameter error handling

2023-08-21 Thread via GitHub
markt-asf commented on code in PR #646: URL: https://github.com/apache/tomcat/pull/646#discussion_r1299935009 ## java/org/apache/tomcat/util/http/InvalidParameterException.java: ## @@ -0,0 +1,65 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + *

[GitHub] [tomcat] markt-asf commented on a diff in pull request #646: Parameter error handling

2023-08-21 Thread via GitHub
markt-asf commented on code in PR #646: URL: https://github.com/apache/tomcat/pull/646#discussion_r1299927091 ## java/org/apache/tomcat/util/http/InvalidParameterException.java: ## @@ -0,0 +1,65 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + *

[GitHub] [tomcat] michael-o commented on a diff in pull request #646: Parameter error handling

2023-08-21 Thread via GitHub
michael-o commented on code in PR #646: URL: https://github.com/apache/tomcat/pull/646#discussion_r1299773529 ## java/org/apache/tomcat/util/http/InvalidParameterException.java: ## @@ -0,0 +1,65 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + *

[GitHub] [tomcat-jakartaee-migration] tutufool opened a new issue, #47: EE package migration

2023-08-20 Thread via GitHub
tutufool opened a new issue, #47: URL: https://github.com/apache/tomcat-jakartaee-migration/issues/47 Hi, I'd like to know when a specific package is replaced (like javax.xml.bind replaced by jakarta.xml.bind), shall we need introduce some 3rd party dependency? like:

[GitHub] [tomcat] markt-asf commented on pull request #646: Parameter error handling

2023-08-20 Thread via GitHub
markt-asf commented on PR #646: URL: https://github.com/apache/tomcat/pull/646#issuecomment-1685427252 Thanks for the review. I checked the various getParameter() methods and found a couple I thought needed a try/catch block. The remainder were all used in webapps or in APIs called by

[GitHub] [tomcat] aooohan commented on pull request #646: Parameter error handling

2023-08-18 Thread via GitHub
aooohan commented on PR #646: URL: https://github.com/apache/tomcat/pull/646#issuecomment-1684798995 This change look good for me. But I have found that `org.apache.catalina.valves.ExtendedAccessLogValve.RequestParameterElement` behavior is broken due to this change. Maybe here should

[GitHub] [tomcat] markt-asf opened a new pull request, #646: Parameter error handling

2023-08-18 Thread via GitHub
markt-asf opened a new pull request, #646: URL: https://github.com/apache/tomcat/pull/646 This PR updates the parameter error handling to align with the changes in Jakarta Servlet 6.0 The short version is all getParameterXXX() calls now throw exceptions on invalid parameters.

[GitHub] [tomcat] babyblue94520 closed pull request #645: feat(Http11Processor): Add max value in keepalive.

2023-08-18 Thread via GitHub
babyblue94520 closed pull request #645: feat(Http11Processor): Add max value in keepalive. URL: https://github.com/apache/tomcat/pull/645 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the

[GitHub] [tomcat] aooohan commented on a diff in pull request #645: feat(Http11Processor): Add max value in keepalive.

2023-08-09 Thread via GitHub
aooohan commented on code in PR #645: URL: https://github.com/apache/tomcat/pull/645#discussion_r1289481686 ## java/org/apache/coyote/http11/Http11Processor.java: ## @@ -999,7 +999,12 @@ protected final void prepareResponse() throws IOException { int

[GitHub] [tomcat] babyblue94520 commented on a diff in pull request #645: feat(Http11Processor): Add max value in keepalive.

2023-08-09 Thread via GitHub
babyblue94520 commented on code in PR #645: URL: https://github.com/apache/tomcat/pull/645#discussion_r1289412320 ## java/org/apache/coyote/http11/Http11Processor.java: ## @@ -999,7 +999,12 @@ protected final void prepareResponse() throws IOException { int

[GitHub] [tomcat] markt-asf commented on a diff in pull request #645: feat(Http11Processor): Add max value in keepalive.

2023-08-09 Thread via GitHub
markt-asf commented on code in PR #645: URL: https://github.com/apache/tomcat/pull/645#discussion_r1288767872 ## java/org/apache/coyote/http11/Http11Processor.java: ## @@ -999,7 +999,12 @@ protected final void prepareResponse() throws IOException { int

<    2   3   4   5   6   7   8   9   10   11   >