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/12/02 15:21:44 UTC

[GitHub] [tomcat] shorinji opened a new pull request #383: Fix so ResponseUtil:addVaryFieldName() stop adding duplicate values

shorinji opened a new pull request #383:
URL: https://github.com/apache/tomcat/pull/383


   Currently addVaryFieldName() add a value twice when there already is named value (not asterisk).
   The existing tests did not catch this, since they assert using a HashMap thus containing no duplicates.
   I have updated one of the test case, inlining parts of the doTestAddVaryFieldName() that can be reused.
   
   Adding `System.out.println("adding " + name + " with " + adapter.getHeaders(VARY_HEADER) + " already present = " + varyHeader);` at the end of addVaryFieldName() we can track what happens when running the tests:
   
   `test-nio:
       [junit] Running org.apache.tomcat.util.http.TestResponseUtil
       [junit] adding too with [foo, bar] already present = too,bar,too,foo
       [junit] adding too with [{{{, bar] already present = too,bar,too
       [junit] adding foo with [foo, bar] already present = foo,bar,foo
       [junit] adding bar with [{{{, bar] already present = bar,bar
       [junit] adding too with [foo, bar] already present = too,bar,too,foo
       [junit] adding too with [foo, bar] already present = too,bar,too,foo
       [junit] adding foo with [foo, bar] already present = foo,bar,foo
   ...`
   
   After my patch this looks like:
   `test-nio:
       [junit] Running org.apache.tomcat.util.http.TestResponseUtil
       [junit] adding too with [foo, bar] already present = bar,too,foo
       [junit] adding too with [{{{, bar] already present = bar,too
       [junit] adding foo with [foo, bar] already present = bar,foo
       [junit] adding bar with [{{{, bar] already present = bar
       [junit] adding too with [foo, bar] already present = bar,too,foo
       [junit] adding too with [foo, bar] already present = bar,too,foo
       [junit] adding foo with [foo, bar] already present = bar,foo
   `


----------------------------------------------------------------
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



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

[GitHub] [tomcat] markt-asf closed pull request #383: Fix so ResponseUtil:addVaryFieldName() stop adding duplicate values

Posted by GitBox <gi...@apache.org>.
markt-asf closed pull request #383:
URL: https://github.com/apache/tomcat/pull/383


   


----------------------------------------------------------------
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



---------------------------------------------------------------------
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 pull request #383: Fix so ResponseUtil:addVaryFieldName() stop adding duplicate values

Posted by GitBox <gi...@apache.org>.
markt-asf commented on pull request #383:
URL: https://github.com/apache/tomcat/pull/383#issuecomment-737428376


   Thanks for the pull request. I've applied a slightly different version as:
   
   - we patch 10.0.x first and then back-port
   - I agree it is better if the tests don't use Set
   - On reflection, I think it is better not to modify the existing header order and add any new value to the end
   
   That last point isn't strictly necessary but it strikes me as what users will expect.


----------------------------------------------------------------
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



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