You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by GitBox <gi...@apache.org> on 2020/01/10 05:08:58 UTC

[GitHub] [tomcat] bkahlert opened a new pull request #232: Fix handling of query parameters with no value, like `?foo`

bkahlert opened a new pull request #232: Fix handling of query parameters with no value, like `?foo`
URL: https://github.com/apache/tomcat/pull/232
 
 
   Currently undefined values are returned as an empty string by `Parameters#getParameter(String)`, though `null` should be returned.
   
   **Background:**
   A query string can contain defined and undefined variable values.
   A variable with a defined value is followed by a `=` (e.g. `?foo=abc` where `foo` is the string `abc`, or `bar=` where the string is empty).
   A variable with an undefined value is not followed by `=`.
   In Java such a value is coded with `null` and not an empty string because the latter is already the defined value of a string with zero length.
   
   See also https://tools.ietf.org/html/rfc6570#section-2.3 and https://github.com/OpenFeign/feign/issues/872#issuecomment-452390655

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] markt-asf commented on issue #232: Fix handling of query parameters with no value, like `?foo`

Posted by GitBox <gi...@apache.org>.
markt-asf commented on issue #232: Fix handling of query parameters with no value, like `?foo`
URL: https://github.com/apache/tomcat/pull/232#issuecomment-573140542
 
 
   > I recall a discussion and changes in this area a few years ago. I'd like to find that discussion and remind myself of the details before deciding on whether to apply this PR.
   
   The discussion was about cookie parameters, not query parameters, so it does not apply here. 
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] bitcod3r commented on a change in pull request #232: Fix handling of query parameters with no value, like `?foo`

Posted by GitBox <gi...@apache.org>.
bitcod3r commented on a change in pull request #232: Fix handling of query parameters with no value, like `?foo`
URL: https://github.com/apache/tomcat/pull/232#discussion_r367052401
 
 

 ##########
 File path: res/tomcat-maven/README.md
 ##########
 @@ -39,7 +39,7 @@ mvn clean; mvn package
 ```
 docker build -t apache/tomcat-maven:1.0 -f ./Dockerfile .
 ```
-Docker build arguments include `namepsace` (default is `tomcat`) and `port` which should match the Tomcat port in `server.xml` (default is `8080`). Other ports that need to be exposed can be added in the `Dockerfile` as needed. Webapps should be added to the `webapps` folder where they will be auto deployed by the host if using the defaults. Otherwise, the `Dockerfile` command line can be edited like below to include the necesary resources and command line arguments to run a single or multiple hardcoded web applications.
+Docker build arguments include `namespace` (default is `tomcat`) and `port` which should match the Tomcat port in `server.xml` (default is `8080`). Other ports that need to be exposed can be added in the `Dockerfile` as needed. Webapps should be added to the `webapps` folder where they will be auto deployed by the host if using the defaults. Otherwise, the `Dockerfile` command line can be edited like below to include the necesary resources and command line arguments to run a single or multiple hardcoded web applications.
 
 Review comment:
   Thanks for the clarification. :)

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] ChristopherSchultz commented on a change in pull request #232: Fix handling of query parameters with no value, like `?foo`

Posted by GitBox <gi...@apache.org>.
ChristopherSchultz commented on a change in pull request #232: Fix handling of query parameters with no value, like `?foo`
URL: https://github.com/apache/tomcat/pull/232#discussion_r365254773
 
 

 ##########
 File path: test/org/apache/catalina/core/TestApplicationHttpRequest.java
 ##########
 @@ -354,4 +364,11 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp)
             }
         }
     }
+
+    private static boolean objectsEqual(String object1, String object2) {
 
 Review comment:
   I agree that `getParameter` should remain: a parameter present with no value should return `""` and not `null`.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] bkahlert commented on issue #232: Fix handling of query parameters with no value, like `?foo`

Posted by GitBox <gi...@apache.org>.
bkahlert commented on issue #232: Fix handling of query parameters with no value, like `?foo`
URL: https://github.com/apache/tomcat/pull/232#issuecomment-573089178
 
 
   I think `?foo&foo=1&foo` leading to `{null, "1", null}` is totall valid and imho the expected behaviour.
   Asking for a parameter's value `getParameter(String)` should return its value.
   Doing so for a Parameter, that was not provided has no defined behavior to my mind.
   Therefor I advocate a `isParameter(String)` or similar method that checks, if a parameter was provided at all.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] bkahlert commented on issue #232: Fix handling of query parameters with no value, like `?foo`

Posted by GitBox <gi...@apache.org>.
bkahlert commented on issue #232: Fix handling of query parameters with no value, like `?foo`
URL: https://github.com/apache/tomcat/pull/232#issuecomment-572976739
 
 
   1. Spring Boot is handling query parameters as described. Example: `@RequestParam MultiValueMap<String, Object> params` in all types of Tests that don't involve Tomcat.
   2. https://github.com/OpenFeign/feign/issues/872#issuecomment-452390655 works that way.
   3. https://tools.ietf.org/html/rfc6570#section-2.3 describes it that way.
   4. The HTTP Verb PATCH could can't be used to its full extends or can harm data, since the called side can't distinguish between null and empty.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] bkahlert commented on issue #232: Fix handling of query parameters with no value, like `?foo`

Posted by GitBox <gi...@apache.org>.
bkahlert commented on issue #232: Fix handling of query parameters with no value, like `?foo`
URL: https://github.com/apache/tomcat/pull/232#issuecomment-575914702
 
 
   I fear we won't make progress here.
   To anyone who suffers from the same problem (I spent hours with it),
   I have created a workaround as a [gist](https://gist.github.com/bkahlert/47884a44edcbbff8da5e79696f428eff) that makes Spring Boot always parse the query string not just the same way as during the tests but also as you might expect it; `undefined` / `?foo`  as `null` and `defined` / `?bar=&baz=%F0%9F%90%BB` as what it is (empty string and 🐻).
   
   
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] markt-asf commented on a change in pull request #232: Fix handling of query parameters with no value, like `?foo`

Posted by GitBox <gi...@apache.org>.
markt-asf commented on a change in pull request #232: Fix handling of query parameters with no value, like `?foo`
URL: https://github.com/apache/tomcat/pull/232#discussion_r365102120
 
 

 ##########
 File path: res/tomcat-maven/README.md
 ##########
 @@ -39,7 +39,7 @@ mvn clean; mvn package
 ```
 docker build -t apache/tomcat-maven:1.0 -f ./Dockerfile .
 ```
-Docker build arguments include `namepsace` (default is `tomcat`) and `port` which should match the Tomcat port in `server.xml` (default is `8080`). Other ports that need to be exposed can be added in the `Dockerfile` as needed. Webapps should be added to the `webapps` folder where they will be auto deployed by the host if using the defaults. Otherwise, the `Dockerfile` command line can be edited like below to include the necesary resources and command line arguments to run a single or multiple hardcoded web applications.
+Docker build arguments include `namespace` (default is `tomcat`) and `port` which should match the Tomcat port in `server.xml` (default is `8080`). Other ports that need to be exposed can be added in the `Dockerfile` as needed. Webapps should be added to the `webapps` folder where they will be auto deployed by the host if using the defaults. Otherwise, the `Dockerfile` command line can be edited like below to include the necesary resources and command line arguments to run a single or multiple hardcoded web applications.
 
 Review comment:
   Look again at the first argument, namespace. A typo is fixed.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] ChristopherSchultz commented on issue #232: Fix handling of query parameters with no value, like `?foo`

Posted by GitBox <gi...@apache.org>.
ChristopherSchultz commented on issue #232: Fix handling of query parameters with no value, like `?foo`
URL: https://github.com/apache/tomcat/pull/232#issuecomment-576357046
 
 
   I stiil don't understand the ask, here. It looks like you are taking a situation where you CAN tell whether a parameter exists and turning it into a situation where you CANNOT tell that it exists. That's a net loss in functionality IMO.
   
   It also violates a part of the servlet specification IMO: `getParameter` states that it returns `null` if a parameter does not exist, and I believe that, implicitly, it is stating that the method will return non-`null` if the parameter *does* exist. This PR violates that implicit requirement and is therefore rejected.
   
   You may publish your own code if you choose to, but this PR will not be merged.
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] bkahlert edited a comment on issue #232: Fix handling of query parameters with no value, like `?foo`

Posted by GitBox <gi...@apache.org>.
bkahlert edited a comment on issue #232: Fix handling of query parameters with no value, like `?foo`
URL: https://github.com/apache/tomcat/pull/232#issuecomment-573089178
 
 
   I think `?foo&foo=1&foo` leading to `{null, "1", null}` is totally valid and imho the expected behavior.
   Asking for a parameter's value `getParameter(String)` should return its value.
   Doing so for a Parameter, that was not provided has no defined behavior to my mind.
   Therefore I advocate for a `isParameter(String)` or similar method that checks if a parameter was provided at all.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] rmaucher commented on issue #232: Fix handling of query parameters with no value, like `?foo`

Posted by GitBox <gi...@apache.org>.
rmaucher commented on issue #232: Fix handling of query parameters with no value, like `?foo`
URL: https://github.com/apache/tomcat/pull/232#issuecomment-573465491
 
 
   I don't understand how this can work either, but I guess something is missing. Unfortunately there's no real solution.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] ChristopherSchultz commented on issue #232: Fix handling of query parameters with no value, like `?foo`

Posted by GitBox <gi...@apache.org>.
ChristopherSchultz commented on issue #232: Fix handling of query parameters with no value, like `?foo`
URL: https://github.com/apache/tomcat/pull/232#issuecomment-573055772
 
 
   I agree that `getParameter` should remain: a parameter present with no value should return `""` and not `null`. `getParameterValues` (and also `getParameterMap`) should return `new String[0]` if there are no values but the parameter is present, not `null`.
   
   Otherwise, there is no way to tell the difference between parameters which are present (e.g. `foo` in your example) and those that are not (e.g. `bar`, which isn't in your example URL).
   
   I think this means I disagree 100% with the premise of this 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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] ChristopherSchultz commented on a change in pull request #232: Fix handling of query parameters with no value, like `?foo`

Posted by GitBox <gi...@apache.org>.
ChristopherSchultz commented on a change in pull request #232: Fix handling of query parameters with no value, like `?foo`
URL: https://github.com/apache/tomcat/pull/232#discussion_r365254773
 
 

 ##########
 File path: test/org/apache/catalina/core/TestApplicationHttpRequest.java
 ##########
 @@ -354,4 +364,11 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp)
             }
         }
     }
+
+    private static boolean objectsEqual(String object1, String object2) {
 
 Review comment:
   I agree that `getParameter` should remain: a parameter present with no value should return `""` and not `null`.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] panchenko commented on a change in pull request #232: Fix handling of query parameters with no value, like `?foo`

Posted by GitBox <gi...@apache.org>.
panchenko commented on a change in pull request #232: Fix handling of query parameters with no value, like `?foo`
URL: https://github.com/apache/tomcat/pull/232#discussion_r365189403
 
 

 ##########
 File path: test/org/apache/catalina/core/TestApplicationHttpRequest.java
 ##########
 @@ -354,4 +364,11 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp)
             }
         }
     }
+
+    private static boolean objectsEqual(String object1, String object2) {
 
 Review comment:
   The code here fails if only the first parameter is `null`

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] bkahlert commented on a change in pull request #232: Fix handling of query parameters with no value, like `?foo`

Posted by GitBox <gi...@apache.org>.
bkahlert commented on a change in pull request #232: Fix handling of query parameters with no value, like `?foo`
URL: https://github.com/apache/tomcat/pull/232#discussion_r365162630
 
 

 ##########
 File path: res/tomcat-maven/README.md
 ##########
 @@ -39,7 +39,7 @@ mvn clean; mvn package
 ```
 docker build -t apache/tomcat-maven:1.0 -f ./Dockerfile .
 ```
-Docker build arguments include `namepsace` (default is `tomcat`) and `port` which should match the Tomcat port in `server.xml` (default is `8080`). Other ports that need to be exposed can be added in the `Dockerfile` as needed. Webapps should be added to the `webapps` folder where they will be auto deployed by the host if using the defaults. Otherwise, the `Dockerfile` command line can be edited like below to include the necesary resources and command line arguments to run a single or multiple hardcoded web applications.
+Docker build arguments include `namespace` (default is `tomcat`) and `port` which should match the Tomcat port in `server.xml` (default is `8080`). Other ports that need to be exposed can be added in the `Dockerfile` as needed. Webapps should be added to the `webapps` folder where they will be auto deployed by the host if using the defaults. Otherwise, the `Dockerfile` command line can be edited like below to include the necesary resources and command line arguments to run a single or multiple hardcoded web applications.
 
 Review comment:
   Correct. `namepsace` -> `namespace`

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] bkahlert commented on a change in pull request #232: Fix handling of query parameters with no value, like `?foo`

Posted by GitBox <gi...@apache.org>.
bkahlert commented on a change in pull request #232: Fix handling of query parameters with no value, like `?foo`
URL: https://github.com/apache/tomcat/pull/232#discussion_r365292432
 
 

 ##########
 File path: test/org/apache/catalina/core/TestApplicationHttpRequest.java
 ##########
 @@ -354,4 +364,11 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp)
             }
         }
     }
+
+    private static boolean objectsEqual(String object1, String object2) {
 
 Review comment:
   Indeed. I did not use `Objects.equals` because of backward compatibility.
   I will solve that possible NPE.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] ChristopherSchultz edited a comment on issue #232: Fix handling of query parameters with no value, like `?foo`

Posted by GitBox <gi...@apache.org>.
ChristopherSchultz edited a comment on issue #232: Fix handling of query parameters with no value, like `?foo`
URL: https://github.com/apache/tomcat/pull/232#issuecomment-576357046
 
 
   I still don't understand the ask, here. It looks like you are taking a situation where you CAN tell whether a parameter exists and turning it into a situation where you CANNOT tell that it exists. That's a net loss in functionality IMO.
   
   It also violates a part of the servlet specification IMO: `getParameter` states that it returns `null` if a parameter does not exist, and I believe that, implicitly, it is stating that the method will return non-`null` if the parameter *does* exist. This PR violates that implicit requirement and is therefore rejected.
   
   You may publish your own code if you choose to, but this PR will not be merged.
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] panchenko commented on a change in pull request #232: Fix handling of query parameters with no value, like `?foo`

Posted by GitBox <gi...@apache.org>.
panchenko commented on a change in pull request #232: Fix handling of query parameters with no value, like `?foo`
URL: https://github.com/apache/tomcat/pull/232#discussion_r365164921
 
 

 ##########
 File path: test/org/apache/catalina/core/TestApplicationHttpRequest.java
 ##########
 @@ -354,4 +364,11 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp)
             }
         }
     }
+
+    private static boolean objectsEqual(String object1, String object2) {
 
 Review comment:
   `java.util.Objects.equals(Object a, Object b)` ?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] ChristopherSchultz commented on issue #232: Fix handling of query parameters with no value, like `?foo`

Posted by GitBox <gi...@apache.org>.
ChristopherSchultz commented on issue #232: Fix handling of query parameters with no value, like `?foo`
URL: https://github.com/apache/tomcat/pull/232#issuecomment-573438973
 
 
   > I think `?foo&foo=1&foo` leading to `{null, "1", null}` is totally valid and imho the expected behavior.
   
   Thanks for the original comment about this @panchenko. I think this should return `{"", "1", ""}` to be consistent with the other methods. That means that `?foo` should not return `new String[0]` but instead `new String[] { "" }`.
   
   > Asking for a parameter's value `getParameter(String)` should return its value.
   > Doing so for a Parameter, that was not provided has no defined behavior to my mind.
   
   The only way to tell the difference between the parameter being present and not is to check the value you get back from `getParameter`. Returning `null` is indistinguishable from the parameter not being present, so `null` is not acceptable.
   
   > Therefore I advocate for a `isParameter(String)` or similar method that checks if a parameter was provided at all.
   
   You cannot change the servlet API, so this is not possible. Also, it has to be backward-compatible, so, again, this cannot be changed.
   
   I am now officially a -1 on this 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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] michael-o commented on a change in pull request #232: Fix handling of query parameters with no value, like `?foo`

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #232: Fix handling of query parameters with no value, like `?foo`
URL: https://github.com/apache/tomcat/pull/232#discussion_r365179774
 
 

 ##########
 File path: test/org/apache/catalina/core/TestApplicationHttpRequest.java
 ##########
 @@ -354,4 +364,11 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp)
             }
         }
     }
+
+    private static boolean objectsEqual(String object1, String object2) {
 
 Review comment:
   That is problematic if this will be backported to Tomcat 7.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] bkahlert edited a comment on issue #232: Fix handling of query parameters with no value, like `?foo`

Posted by GitBox <gi...@apache.org>.
bkahlert edited a comment on issue #232: Fix handling of query parameters with no value, like `?foo`
URL: https://github.com/apache/tomcat/pull/232#issuecomment-575914702
 
 
   I fear we won't make progress here.
   To anyone who suffers from the same problem (I spent hours with it),
   I have created a workaround as a [gist - UndefinedToNullMappingWebMvcRegistrationsConfiguration.java](https://gist.github.com/bkahlert/47884a44edcbbff8da5e79696f428eff) that makes Spring Boot always parse the query string not just the same way as during the tests but also as you might expect it; `undefined` / `?foo`  as `null` and `defined` / `?bar=&baz=%F0%9F%90%BB` as what it is (empty string and 🐻).
   
   ```
   @Override
   public Map<String, String[]> getParameterMap() {
       MultiValueMap<String, String> parameterMap = UriComponentsBuilder
           .fromHttpRequest(new ServletServerHttpRequest(getRequest())).build().getQueryParams();
       return parameterMap.entrySet().stream().collect(Collectors.toMap(
           Map.Entry::getKey, entry -> entry.getValue().toArray(new String[0])));
   }
   ```

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] markt-asf commented on issue #232: Fix handling of query parameters with no value, like `?foo`

Posted by GitBox <gi...@apache.org>.
markt-asf commented on issue #232: Fix handling of query parameters with no value, like `?foo`
URL: https://github.com/apache/tomcat/pull/232#issuecomment-572909953
 
 
   Seems reasonable.
   
   I recall a discussion and changes in this area a few years ago. I'd like to find that discussion and remind myself of the details before deciding on whether to apply this PR.
   
   Do you have any specification references to support the behaviour you are proposing?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] bitcod3r commented on a change in pull request #232: Fix handling of query parameters with no value, like `?foo`

Posted by GitBox <gi...@apache.org>.
bitcod3r commented on a change in pull request #232: Fix handling of query parameters with no value, like `?foo`
URL: https://github.com/apache/tomcat/pull/232#discussion_r365080879
 
 

 ##########
 File path: res/tomcat-maven/README.md
 ##########
 @@ -39,7 +39,7 @@ mvn clean; mvn package
 ```
 docker build -t apache/tomcat-maven:1.0 -f ./Dockerfile .
 ```
-Docker build arguments include `namepsace` (default is `tomcat`) and `port` which should match the Tomcat port in `server.xml` (default is `8080`). Other ports that need to be exposed can be added in the `Dockerfile` as needed. Webapps should be added to the `webapps` folder where they will be auto deployed by the host if using the defaults. Otherwise, the `Dockerfile` command line can be edited like below to include the necesary resources and command line arguments to run a single or multiple hardcoded web applications.
+Docker build arguments include `namespace` (default is `tomcat`) and `port` which should match the Tomcat port in `server.xml` (default is `8080`). Other ports that need to be exposed can be added in the `Dockerfile` as needed. Webapps should be added to the `webapps` folder where they will be auto deployed by the host if using the defaults. Otherwise, the `Dockerfile` command line can be edited like below to include the necesary resources and command line arguments to run a single or multiple hardcoded web applications.
 
 Review comment:
   @bkahlert Seems like there's no diff in this file. Please, revert and checkout to the original to keep the PR clear of exact files changed. 

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] ChristopherSchultz closed pull request #232: Fix handling of query parameters with no value, like `?foo`

Posted by GitBox <gi...@apache.org>.
ChristopherSchultz closed pull request #232: Fix handling of query parameters with no value, like `?foo`
URL: https://github.com/apache/tomcat/pull/232
 
 
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] panchenko commented on issue #232: Fix handling of query parameters with no value, like `?foo`

Posted by GitBox <gi...@apache.org>.
panchenko commented on issue #232: Fix handling of query parameters with no value, like `?foo`
URL: https://github.com/apache/tomcat/pull/232#issuecomment-573059015
 
 
   The parameter without value can be specified multiple times, like `?foo&foo=1&foo`.
   If we want to represent that. it has to be `{null, "1", null}`.
   I afraid nobody likes that, and there is no nice way to represent such values via the Servlet API.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] panchenko commented on issue #232: Fix handling of query parameters with no value, like `?foo`

Posted by GitBox <gi...@apache.org>.
panchenko commented on issue #232: Fix handling of query parameters with no value, like `?foo`
URL: https://github.com/apache/tomcat/pull/232#issuecomment-573090362
 
 
   `getParameterValues(String) != null` would do that

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org